On Tue, 2018-02-20 at 18:17 +0000, Emil Velikov wrote:
> On 28 October 2017 at 00:32, Francisco Jerez <curroje...@riseup.net> wrote:
> > Vedran Miletić <ved...@miletic.net> writes:
> >
> > > v2: remove/inline compat stuff
> > >
> > > Reviewed-by: Francisco Jerez <curroje...@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 <curroje...@riseup.net>
> >
>
> Is it me, or this seems to have fallen through the cracks?looks like it. Vedran, do you have a version with Francisco's comments addressed around? Jan > > -Emil
signature.asc
Description: This is a digitally signed message part
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
