[Mesa-dev] [PATCH v2] clover/llvm: Drop support for LLVM < 3.9.

Francisco Jerez currojerez at riseup.net
Fri Oct 27 23:32:21 UTC 2017


Vedran Miletić <vedran at miletic.net> writes:

> v2: remove/inline compat stuff
>
> Reviewed-by: Francisco Jerez <currojerez at riseup.net>
> ---
>  .../state_trackers/clover/llvm/codegen/native.cpp  |   8 +-
>  src/gallium/state_trackers/clover/llvm/compat.hpp  | 109 +--------------------
>  .../state_trackers/clover/llvm/invocation.cpp      |  22 +++--
>  .../state_trackers/clover/llvm/metadata.hpp        |  30 +-----
>  4 files changed, 20 insertions(+), 149 deletions(-)
>
> diff --git a/src/gallium/state_trackers/clover/llvm/codegen/native.cpp b/src/gallium/state_trackers/clover/llvm/codegen/native.cpp
> index 12c83a92b6..38028597a3 100644
> --- a/src/gallium/state_trackers/clover/llvm/codegen/native.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/codegen/native.cpp
> @@ -114,7 +114,7 @@ namespace {
>  
>        std::unique_ptr<TargetMachine> tm {
>           t->createTargetMachine(target.triple, target.cpu, "", {},
> -                                compat::default_reloc_model,
> +                                ::llvm::None,
>                                  compat::default_code_model,
>                                  ::llvm::CodeGenOpt::Default) };
>        if (!tm)
> @@ -124,11 +124,11 @@ namespace {
>        ::llvm::SmallVector<char, 1024> data;
>  
>        {
> -         compat::pass_manager pm;
> +         ::llvm::legacy::PassManager pm;
>           ::llvm::raw_svector_ostream os { data };
> -         compat::raw_ostream_to_emit_file fos(os);
> +         ::llvm::raw_svector_ostream &fos(os);
>

This is just a reference to the other local variable above.  Mind
cleaning it up?

> -         mod.setDataLayout(compat::get_data_layout(*tm));
> +         mod.setDataLayout(tm->createDataLayout());
>           tm->Options.MCOptions.AsmVerbose =
>              (ft == TargetMachine::CGFT_AssemblyFile);
>  
> diff --git a/src/gallium/state_trackers/clover/llvm/compat.hpp b/src/gallium/state_trackers/clover/llvm/compat.hpp
> index f8b56516d5..ce3a29f7d5 100644
> --- a/src/gallium/state_trackers/clover/llvm/compat.hpp
> +++ b/src/gallium/state_trackers/clover/llvm/compat.hpp
> @@ -36,6 +36,8 @@
>  
>  #include "util/algorithm.hpp"
>  
> +#include <llvm/Analysis/TargetLibraryInfo.h>
> +#include <llvm/IR/LegacyPassManager.h>
>  #include <llvm/IR/LLVMContext.h>
>  #include <llvm/Linker/Linker.h>
>  #include <llvm/Transforms/IPO.h>
> @@ -46,16 +48,6 @@
>  #include <llvm/Support/ErrorOr.h>
>  #endif
>  
> -#if HAVE_LLVM >= 0x0307
> -#include <llvm/IR/LegacyPassManager.h>
> -#include <llvm/Analysis/TargetLibraryInfo.h>
> -#else
> -#include <llvm/PassManager.h>
> -#include <llvm/Target/TargetLibraryInfo.h>
> -#include <llvm/Target/TargetSubtargetInfo.h>
> -#include <llvm/Support/FormattedStream.h>
> -#endif
> -
>  #include <clang/Basic/TargetInfo.h>
>  #include <clang/Frontend/CodeGenOptions.h>
>  #include <clang/Frontend/CompilerInstance.h>
> @@ -63,12 +55,6 @@
>  namespace clover {
>     namespace llvm {
>        namespace compat {
> -#if HAVE_LLVM >= 0x0307
> -         typedef ::llvm::TargetLibraryInfoImpl target_library_info;
> -#else
> -         typedef ::llvm::TargetLibraryInfo target_library_info;
> -#endif
> -
>  #if HAVE_LLVM >= 0x0500
>           const auto lang_as_offset = 0;
>           const clang::InputKind ik_opencl = clang::InputKind::OpenCL;
> @@ -77,19 +63,6 @@ namespace clover {
>           const clang::InputKind ik_opencl = clang::IK_OpenCL;
>  #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) {
> @@ -100,78 +73,8 @@ namespace clover {
>              F.PropagateAttrs = true;
>              F.LinkFlags = ::llvm::Linker::Flags::None;
>              opts.LinkBitcodeFiles.emplace_back(F);
> -#elif 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<std::string> &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<const char *>(
> -                      map(std::mem_fn(&std::string::data), names))));
> -#endif
> -         }
> -
> -         inline std::unique_ptr< ::llvm::Linker>
> -         create_linker(::llvm::Module &mod) {
> -#if HAVE_LLVM >= 0x0308
> -            return std::unique_ptr< ::llvm::Linker>(new ::llvm::Linker(mod));
>  #else
> -            return std::unique_ptr< ::llvm::Linker>(new ::llvm::Linker(&mod));
> -#endif
> -         }
> -
> -         inline bool
> -         link_in_module(::llvm::Linker &linker,
> -                        std::unique_ptr< ::llvm::Module> mod) {
> -#if HAVE_LLVM >= 0x0308
> -            return linker.linkInModule(std::move(mod));
> -#else
> -            return linker.linkInModule(mod.get());
> -#endif
> -         }
> -
> -#if HAVE_LLVM >= 0x0307
> -         typedef ::llvm::raw_svector_ostream &raw_ostream_to_emit_file;
> -#else
> -         typedef ::llvm::formatted_raw_ostream raw_ostream_to_emit_file;
> -#endif
> -
> -#if HAVE_LLVM >= 0x0307
> -         typedef ::llvm::DataLayout data_layout;
> -#else
> -         typedef const ::llvm::DataLayout *data_layout;
> -#endif
> -
> -         inline data_layout
> -         get_data_layout(::llvm::TargetMachine &tm) {
> -#if HAVE_LLVM >= 0x0307
> -            return tm.createDataLayout();
> -#else
> -            return tm.getSubtargetImpl()->getDataLayout();
> +            opts.LinkBitcodeFiles.emplace_back(::llvm::Linker::Flags::None, path);
>  #endif
>           }
>  
> @@ -181,12 +84,6 @@ namespace clover {
>           const auto default_code_model = ::llvm::CodeModel::Default;
>  #endif
>  
> -#if HAVE_LLVM >= 0x0309
> -         const auto default_reloc_model = ::llvm::None;
> -#else
> -         const auto default_reloc_model = ::llvm::Reloc::Default;
> -#endif
> -
>           template<typename M, typename F> void
>           handle_module_error(M &mod, const F &f) {
>  #if HAVE_LLVM >= 0x0400
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> index a373df4eac..949a689d58 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> @@ -125,7 +125,7 @@ namespace {
>        // http://www.llvm.org/bugs/show_bug.cgi?id=19735
>        c->getDiagnosticOpts().ShowCarets = false;
>  
> -      compat::set_lang_defaults(c->getInvocation(), c->getLangOpts(),
> +      c->getInvocation().setLangDefaults(c->getLangOpts(),
>                                  compat::ik_opencl, ::llvm::Triple(target.triple),
>                                  c->getPreprocessorOpts(),
>                                  clang::LangStandard::lang_opencl11);
> @@ -223,9 +223,9 @@ namespace {
>     void
>     optimize(Module &mod, unsigned optimization_level,
>              bool internalize_symbols) {
> -      compat::pass_manager pm;
> -
> -      compat::add_data_layout_pass(pm);
> +      ::llvm::legacy::PassManager pm;
> +      std::vector<std::string> names = map(std::mem_fn(&Function::getName),
> +                                           get_kernels(mod));

Please, move this declaration into the 'if (internalize_symbols)'
conditional where it's used, and declare as const.

>  
>        // By default, the function internalizer pass will look for a function
>        // called "main" and then mark all other functions as internal.  Marking
> @@ -240,12 +240,15 @@ namespace {
>        // treat the functions in the list as "main" functions and internalize
>        // all of the other functions.
>        if (internalize_symbols)
> -         compat::add_internalize_pass(pm, map(std::mem_fn(&Function::getName),
> -                                              get_kernels(mod)));
> +         pm.add(::llvm::createInternalizePass(
> +                      [=](const ::llvm::GlobalValue &gv) {
> +                         return std::find(names.begin(), names.end(),
> +                                          gv.getName()) != names.end();
> +                      }));
>  
>        ::llvm::PassManagerBuilder pmb;
>        pmb.OptLevel = optimization_level;
> -      pmb.LibraryInfo = new compat::target_library_info(
> +      pmb.LibraryInfo = new ::llvm::TargetLibraryInfoImpl(
>           ::llvm::Triple(mod.getTargetTriple()));
>        pmb.populateModulePassManager(pm);
>        pm.run(mod);
> @@ -255,11 +258,10 @@ namespace {
>     link(LLVMContext &ctx, const clang::CompilerInstance &c,
>          const std::vector<module> &modules, std::string &r_log) {
>        std::unique_ptr<Module> mod { new Module("link", ctx) };
> -      auto linker = compat::create_linker(*mod);
> +      auto linker = std::unique_ptr< ::llvm::Linker>(new ::llvm::Linker(*mod));
>

You can declare the pointer like 'std::unique_ptr< ::llvm::Linker>
linker { ... }' as above.

>        for (auto &m : modules) {
> -         if (compat::link_in_module(*linker,
> -                                    parse_module_library(m, ctx, r_log)))
> +         if (linker->linkInModule(std::move(parse_module_library(m, ctx, r_log))))
>              throw build_error();
>        }
>  
> diff --git a/src/gallium/state_trackers/clover/llvm/metadata.hpp b/src/gallium/state_trackers/clover/llvm/metadata.hpp
> index 825008d497..bf8d9d5b06 100644
> --- a/src/gallium/state_trackers/clover/llvm/metadata.hpp
> +++ b/src/gallium/state_trackers/clover/llvm/metadata.hpp
> @@ -57,44 +57,16 @@ namespace clover {
>  
>           inline bool
>           is_kernel(const ::llvm::Function &f) {
> -#if HAVE_LLVM >= 0x0309
>              return f.getMetadata("kernel_arg_type");
> -#else
> -            return clover::any_of(is_kernel_node_for(f),
> -                                  get_kernel_nodes(*f.getParent()));
> -#endif
>           }
>  
>           inline iterator_range< ::llvm::MDNode::op_iterator>
>           get_kernel_metadata_operands(const ::llvm::Function &f,
>                                        const std::string &name) {
> -#if HAVE_LLVM >= 0x0309
> -            // On LLVM v3.9+ kernel argument attributes are stored as
> +            // LLVM stores kernel argument attributes as
>              // function metadata.

You can probably fit the whole comment into a single line now.  With
these nit-picks addressed:

Reviewed-by: Francisco Jerez <currojerez at riseup.net>

>              const auto data_node = f.getMetadata(name);
>              return range(data_node->op_begin(), data_node->op_end());
> -#else
> -            using ::llvm::cast;
> -            using ::llvm::dyn_cast;
> -            const auto kernel_node = find(is_kernel_node_for(f),
> -                                          get_kernel_nodes(*f.getParent()));
> -
> -            const auto data_node = cast< ::llvm::MDNode>(
> -               find([&](const ::llvm::MDOperand &op) {
> -                     if (auto m = dyn_cast< ::llvm::MDNode>(op))
> -                        if (m->getNumOperands())
> -                           if (auto m_name = dyn_cast< ::llvm::MDString>(
> -                                  m->getOperand(0).get()))
> -                              return m_name->getString() == name;
> -
> -                     return false;
> -                  },
> -                  kernel_node->operands()));
> -
> -            // Skip the first operand node which is just the metadata
> -            // attribute name.
> -            return range(data_node->op_begin() + 1, data_node->op_end());
> -#endif
>           }
>        }
>  
> -- 
> 2.14.2
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171027/361e1666/attachment-0001.sig>


More information about the mesa-dev mailing list