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

Jan Vesely jan.vesely at rutgers.edu
Fri Jul 8 22:10:34 UTC 2016


On Fri, 2016-07-08 at 14:39 -0700, Francisco Jerez wrote:
> 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?

we still get 'macro redefined' complains from the compiler. and if
anyone adds ifdef DEBUG code in the future it will cause unexpected
behaviour.
this goes for every file that includes llvm headers. it'd be nicer to
get mesa (and llvm) to stop using generic macro names, but I don't
think that's on anybody's todo list.
it was just a suggestion, feel free to ignore it.

Jan

> 
> > 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::F
> > > lags::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::lan
> > > g_opencl11);
> > > +
> > > +      compat::set_lang_defaults(c.getInvocation(),
> > > c.getLangOpts(),
> > > +                                clang::IK_OpenCL,
> > > ::llvm::Triple(triple),
> > > +                                c.getPreprocessorOpts(),
> > > +                                clang::LangStandard::lang_opencl
> > > 11);
> > > +
> > >        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::Lin
> > > ker::Flags::None,
> > > -                                                       libclc_pa
> > > th);
> > > -#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>
-- 

Jan Vesely <jan.vesely at rutgers.edu>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160708/696ee575/attachment-0001.sig>


More information about the mesa-dev mailing list