[Mesa-dev] [PATCH 04/47] clover/llvm: Collect #ifdef mess into a separate file.

Francisco Jerez currojerez at riseup.net
Fri Jul 8 21:39:52 UTC 2016


Jan Vesely <jan.vesely at rutgers.edu> writes:

> On Sun, 2016-07-03 at 17:51 -0700, Francisco Jerez wrote:
>> This gets rid of most ifdef's from the invocation.cpp code -- Only a
>> couple of them are left which will be removed differently in the
>> following commits.
>> 
>> Reviewed-by: Serge Martin <edb+mesa at sigluy.net>
>> ---
>>  src/gallium/state_trackers/clover/Makefile.sources |   1 +
>>  src/gallium/state_trackers/clover/llvm/compat.hpp  | 116 +++++++++++++++++++++
>>  .../state_trackers/clover/llvm/invocation.cpp      | 115 +++++++-------------
>>  3 files changed, 157 insertions(+), 75 deletions(-)
>>  create mode 100644 src/gallium/state_trackers/clover/llvm/compat.hpp
>> 
>> diff --git a/src/gallium/state_trackers/clover/Makefile.sources b/src/gallium/state_trackers/clover/Makefile.sources
>> index 10bbda0..c4a692b 100644
>> --- a/src/gallium/state_trackers/clover/Makefile.sources
>> +++ b/src/gallium/state_trackers/clover/Makefile.sources
>> @@ -54,6 +54,7 @@ CPP_SOURCES := \
>>  	util/tuple.hpp
>>  
>>  LLVM_SOURCES := \
>> +	llvm/compat.hpp \
>>  	llvm/invocation.cpp
>>  
>>  TGSI_SOURCES := \
>> diff --git a/src/gallium/state_trackers/clover/llvm/compat.hpp b/src/gallium/state_trackers/clover/llvm/compat.hpp
>> new file mode 100644
>> index 0000000..3206f77c
>> --- /dev/null
>> +++ b/src/gallium/state_trackers/clover/llvm/compat.hpp
>> @@ -0,0 +1,116 @@
>> +//
>> +// Copyright 2016 Francisco Jerez
>> +//
>> +// Permission is hereby granted, free of charge, to any person obtaining a
>> +// copy of this software and associated documentation files (the "Software"),
>> +// to deal in the Software without restriction, including without limitation
>> +// the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> +// and/or sell copies of the Software, and to permit persons to whom the
>> +// Software is furnished to do so, subject to the following conditions:
>> +//
>> +// The above copyright notice and this permission notice shall be included in
>> +// all copies or substantial portions of the Software.
>> +//
>> +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> +// THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> +// OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> +// ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> +// OTHER DEALINGS IN THE SOFTWARE.
>> +//
>> +
>> +///
>> +/// \file
>> +/// Some thin wrappers around the Clang/LLVM API used to preserve
>> +/// compatibility with older API versions while keeping the ifdef clutter low
>> +/// in the rest of the clover::llvm subtree.  In case of an API break please
>> +/// consider whether it's possible to preserve backwards compatibility by
>> +/// introducing a new one-liner inline function or typedef here under the
>> +/// compat namespace in order to keep the running code free from preprocessor
>> +/// conditionals.
>> +///
>> +
>> +#ifndef CLOVER_LLVM_COMPAT_HPP
>> +#define CLOVER_LLVM_COMPAT_HPP
>> +
>> +#include "util/algorithm.hpp"
>> +
>> +#include 
>> +#include 
>> +
>> +#if HAVE_LLVM >= 0x0307
>> +#include 
>> +#include 
>> +#else
>> +#include 
>> +#include 
>> +#endif
>> +
>> +#include 
>> +#include <clang/Frontend/CompilerInstance.h>
>
> Have you considered using add push/pop_macro for DEBUG (like srw). TO
> avoid conflict between llvm's and mesa's DEBUG ?
>
Nope, I haven't.  I don't think any of the LLVM invocation code below
relies on the DEBUG macro?

> Jan
>
>> +
>> +namespace clover {
>> +   namespace llvm {
>> +      namespace compat {
>> +#if HAVE_LLVM >= 0x0307
>> +         typedef ::llvm::TargetLibraryInfoImpl target_library_info;
>> +#else
>> +         typedef ::llvm::TargetLibraryInfo target_library_info;
>> +#endif
>> +
>> +         inline void
>> +         set_lang_defaults(clang::CompilerInvocation &inv,
>> +                           clang::LangOptions &lopts, clang::InputKind ik,
>> +                           const ::llvm::Triple &t,
>> +                           clang::PreprocessorOptions &ppopts,
>> +                           clang::LangStandard::Kind std) {
>> +#if HAVE_LLVM >= 0x0309
>> +            inv.setLangDefaults(lopts, ik, t, ppopts, std);
>> +#else
>> +            inv.setLangDefaults(lopts, ik, std);
>> +#endif
>> +         }
>> +
>> +         inline void
>> +         add_link_bitcode_file(clang::CodeGenOptions &opts,
>> +                               const std::string &path) {
>> +#if HAVE_LLVM >= 0x0308
>> +            opts.LinkBitcodeFiles.emplace_back(::llvm::Linker::Flags::None, path);
>> +#else
>> +            opts.LinkBitcodeFile = path;
>> +#endif
>> +         }
>> +
>> +#if HAVE_LLVM >= 0x0307
>> +         typedef ::llvm::legacy::PassManager pass_manager;
>> +#else
>> +         typedef ::llvm::PassManager pass_manager;
>> +#endif
>> +
>> +         inline void
>> +         add_data_layout_pass(pass_manager &pm) {
>> +#if HAVE_LLVM < 0x0307
>> +            pm.add(new ::llvm::DataLayoutPass());
>> +#endif
>> +         }
>> +
>> +         inline void
>> +         add_internalize_pass(pass_manager &pm,
>> +                              const std::vector &names) {
>> +#if HAVE_LLVM >= 0x0309
>> +            pm.add(::llvm::createInternalizePass(
>> +                      [=](const ::llvm::GlobalValue &gv) {
>> +                         return std::find(names.begin(), names.end(),
>> +                                          gv.getName()) != names.end();
>> +                      }));
>> +#else
>> +            pm.add(::llvm::createInternalizePass(std::vector(
>> +                      map(std::mem_fn(&std::string::data), names))));
>> +#endif
>> +         }
>> +      }
>> +   }
>> +}
>> +
>> +#endif
>> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> index b9255a8..2f5c8aa 100644
>> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> @@ -24,6 +24,7 @@
>>  // OTHER DEALINGS IN THE SOFTWARE.
>>  //
>>  
>> +#include "llvm/compat.hpp"
>>  #include "core/compiler.hpp"
>>  
>>  #include 
>> @@ -41,11 +42,6 @@
>>  #include 
>>  #include 
>>  #include 
>> -#if HAVE_LLVM >= 0x0307
>> -#include 
>> -#else
>> -#include 
>> -#endif
>>  #include 
>>  #include 
>>  #include 
>> @@ -57,11 +53,6 @@
>>  
>>  
>>  #include 
>> -#if HAVE_LLVM >= 0x0307
>> -#include 
>> -#else
>> -#include 
>> -#endif
>>  #include 
>>  #include 
>>  
>> @@ -82,8 +73,19 @@
>>  #include 
>>  
>>  using namespace clover;
>> +using namespace clover::llvm;
>> +
>> +using ::llvm::Function;
>> +using ::llvm::Module;
>> +using ::llvm::raw_string_ostream;
>>  
>>  namespace {
>> +   // XXX - Temporary hack to avoid breaking the build for the moment, will
>> +   //       get rid of this later.
>> +   namespace llvm {
>> +      using namespace ::llvm;
>> +   }
>> +
>>     void debug_log(const std::string &msg, const std::string &suffix) {
>>        const char *dbg_file = debug_get_option("CLOVER_DEBUG_FILE", "stderr");
>>        if (!strcmp("stderr", dbg_file)) {
>> @@ -106,8 +108,6 @@ namespace {
>>        clang::EmitLLVMOnlyAction act(&llvm_ctx);
>>        std::string log;
>>        llvm::raw_string_ostream s_log(log);
>> -      std::string libclc_path = LIBCLC_LIBEXECDIR + processor + "-"
>> -                                                  + triple + ".bc";
>>  
>>        // Parse the compiler options:
>>        std::vector opts_array;
>> @@ -169,11 +169,12 @@ namespace {
>>        // of warnings and errors to be printed to stderr.
>>        // http://www.llvm.org/bugs/show_bug.cgi?id=19735
>>        c.getDiagnosticOpts().ShowCarets = false;
>> -      c.getInvocation().setLangDefaults(c.getLangOpts(), clang::IK_OpenCL,
>> -#if HAVE_LLVM >= 0x0309
>> -                                        llvm::Triple(triple), c.getPreprocessorOpts(),
>> -#endif
>> -                                        clang::LangStandard::lang_opencl11);
>> +
>> +      compat::set_lang_defaults(c.getInvocation(), c.getLangOpts(),
>> +                                clang::IK_OpenCL, ::llvm::Triple(triple),
>> +                                c.getPreprocessorOpts(),
>> +                                clang::LangStandard::lang_opencl11);
>> +
>>        c.createDiagnostics(
>>                            new clang::TextDiagnosticPrinter(
>>                                   s_log,
>> @@ -198,19 +199,17 @@ namespace {
>>           }
>>        }
>>  
>> -      // Setting this attribute tells clang to link this file before
>> -      // performing any optimizations.  This is required so that
>> -      // we can replace calls to the OpenCL C barrier() builtin
>> -      // with calls to target intrinsics that have the noduplicate
>> -      // attribute.  This attribute will prevent Clang from creating
>> -      // illegal uses of barrier() (e.g. Moving barrier() inside a conditional
>> -      // that is no executed by all threads) during its optimizaton passes.
>> -#if HAVE_LLVM >= 0x0308
>> -      c.getCodeGenOpts().LinkBitcodeFiles.emplace_back(llvm::Linker::Flags::None,
>> -                                                       libclc_path);
>> -#else
>> -      c.getCodeGenOpts().LinkBitcodeFile = libclc_path;
>> -#endif
>> +      // Tell clang to link this file before performing any
>> +      // optimizations.  This is required so that we can replace calls
>> +      // to the OpenCL C barrier() builtin with calls to target
>> +      // intrinsics that have the noduplicate attribute.  This
>> +      // attribute will prevent Clang from creating illegal uses of
>> +      // barrier() (e.g. Moving barrier() inside a conditional that is
>> +      // no executed by all threads) during its optimizaton passes.
>> +      const std::string libclc_path = LIBCLC_LIBEXECDIR + processor + "-"
>> +                                      + triple + ".bc";
>> +      compat::add_link_bitcode_file(c.getCodeGenOpts(), libclc_path);
>> +
>>        optimization_level = c.getCodeGenOpts().OptimizationLevel;
>>  
>>        // Compile the code
>> @@ -257,17 +256,10 @@ namespace {
>>  
>>     void
>>     optimize(llvm::Module *mod, unsigned optimization_level) {
>> +      compat::pass_manager PM;
>>  
>> -#if HAVE_LLVM >= 0x0307
>> -      llvm::legacy::PassManager PM;
>> -#else
>> -      llvm::PassManager PM;
>> -#endif
>> +      compat::add_data_layout_pass(PM);
>>  
>> -      const std::vector kernels = find_kernels(mod);
>> -
>> -      // Add a function internalizer pass.
>> -      //
>>        // By default, the function internalizer pass will look for a function
>>        // called "main" and then mark all other functions as internal.  Marking
>>        // functions as internal enables the optimizer to perform optimizations
>> @@ -280,40 +272,13 @@ namespace {
>>        // list of kernel functions to the internalizer.  The internalizer will
>>        // treat the functions in the list as "main" functions and internalize
>>        // all of the other functions.
>> -#if HAVE_LLVM >= 0x0309
>> -      auto preserve_kernels = [=](const llvm::GlobalValue &GV) {
>> -         for (const auto &kernel : kernels) {
>> -            if (GV.getName() == kernel->getName())
>> -               return true;
>> -         }
>> -         return false;
>> -      };
>> -#else
>> -      std::vector export_list;
>> -      for (std::vector::const_iterator I = kernels.begin(),
>> -                                                         E = kernels.end();
>> -                                                         I != E; ++I) {
>> -         llvm::Function *kernel = *I;
>> -         export_list.push_back(kernel->getName().data());
>> -      }
>> -#endif
>> -#if HAVE_LLVM < 0x0307
>> -      PM.add(new llvm::DataLayoutPass());
>> -#endif
>> -#if HAVE_LLVM >= 0x0309
>> -      PM.add(llvm::createInternalizePass(preserve_kernels));
>> -#else
>> -      PM.add(llvm::createInternalizePass(export_list));
>> -#endif
>> +      compat::add_internalize_pass(PM, map(std::mem_fn(&Function::getName),
>> +                                           find_kernels(mod)));
>>  
>>        llvm::PassManagerBuilder PMB;
>>        PMB.OptLevel = optimization_level;
>> -#if HAVE_LLVM < 0x0307
>> -      PMB.LibraryInfo = new llvm::TargetLibraryInfo(
>> -#else
>> -      PMB.LibraryInfo = new llvm::TargetLibraryInfoImpl(
>> -#endif
>> -            llvm::Triple(mod->getTargetTriple()));
>> +      PMB.LibraryInfo = new compat::target_library_info(
>> +         llvm::Triple(mod->getTargetTriple()));
>>        PMB.populateModulePassManager(PM);
>>        PM.run(*mod);
>>     }
>> @@ -828,7 +793,7 @@ clover::compile_program_llvm(const std::string &source,
>>     std::string triple(target, processor_str_len + 1,
>>                        target.size() - processor_str_len - 1);
>>     clang::LangAS::Map address_spaces;
>> -   llvm::LLVMContext llvm_ctx;
>> +   ::llvm::LLVMContext llvm_ctx;
>>     unsigned optimization_level;
>>  
>>     llvm_ctx.setDiagnosticHandler(diagnostic_handler, &r_log);
>> @@ -838,15 +803,15 @@ clover::compile_program_llvm(const std::string &source,
>>  
>>     // The input file name must have the .cl extension in order for the
>>     // CompilerInvocation class to recognize it as an OpenCL source file.
>> -   llvm::Module *mod = compile_llvm(llvm_ctx, source, headers, "input.cl",
>> -                                    triple, processor, opts, address_spaces,
>> -                                    optimization_level, r_log);
>> +   Module *mod = compile_llvm(llvm_ctx, source, headers, "input.cl",
>> +                              triple, processor, opts, address_spaces,
>> +                              optimization_level, r_log);
>>  
>>     optimize(mod, optimization_level);
>>  
>>     if (get_debug_flags() & DBG_LLVM) {
>>        std::string log;
>> -      llvm::raw_string_ostream s_log(log);
>> +      raw_string_ostream s_log(log);
>>        mod->print(s_log, NULL);
>>        s_log.flush();
>>        debug_log(log, ".ll");
> -- 
>
> Jan Vesely <jan.vesely at rutgers.edu>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160708/9943bd8f/attachment.sig>


More information about the mesa-dev mailing list