[Mesa-dev] [PATCH v2] clover: Fix compilation after clang r315871
From: Jan Vesely v2: use a more generic compat function Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103388 Signed-off-by: Jan Vesely --- src/gallium/state_trackers/clover/llvm/codegen/common.cpp | 5 ++--- src/gallium/state_trackers/clover/llvm/compat.hpp | 12 ++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp index 075183400a..dd9d02ab11 100644 --- a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp +++ b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp @@ -70,7 +70,6 @@ namespace { make_kernel_args(const Module &mod, const std::string &kernel_name, const clang::CompilerInstance &c) { std::vector args; - const auto address_spaces = c.getTarget().getAddressSpaceMap(); const Function &f = *mod.getFunction(kernel_name); ::llvm::DataLayout dl(&mod); const auto size_type = @@ -128,8 +127,8 @@ namespace { const unsigned address_space = cast< ::llvm::PointerType>(actual_type)->getAddressSpace(); - if (address_space == address_spaces[clang::LangAS::opencl_local - - compat::lang_as_offset]) { + if (address_space == compat::target_lang_address_space( + c.getTarget(), clang::LangAS::opencl_local)) { args.emplace_back(module::argument::local, arg_api_size, target_size, target_align, module::argument::zero_ext); diff --git a/src/gallium/state_trackers/clover/llvm/compat.hpp b/src/gallium/state_trackers/clover/llvm/compat.hpp index f8b56516d5..3e34f0dd94 100644 --- a/src/gallium/state_trackers/clover/llvm/compat.hpp +++ b/src/gallium/state_trackers/clover/llvm/compat.hpp @@ -69,11 +69,19 @@ namespace clover { typedef ::llvm::TargetLibraryInfo target_library_info; #endif + template + unsigned target_lang_address_space(const T& target, const AS lang_as) { +const auto &map = target.getAddressSpaceMap(); +#if HAVE_LLVM >= 0x0500 +return map[static_cast(lang_as)]; +#else +return map[lang_as - clang::LangAS::Offset]; +#endif + } + #if HAVE_LLVM >= 0x0500 - const auto lang_as_offset = 0; const clang::InputKind ik_opencl = clang::InputKind::OpenCL; #else - const auto lang_as_offset = clang::LangAS::Offset; const clang::InputKind ik_opencl = clang::IK_OpenCL; #endif -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] clover: Fix compilation after clang r315871
Jan Vesely writes: > From: Jan Vesely > > v2: use a more generic compat function > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103388 > Signed-off-by: Jan Vesely > --- > src/gallium/state_trackers/clover/llvm/codegen/common.cpp | 5 ++--- > src/gallium/state_trackers/clover/llvm/compat.hpp | 12 ++-- > 2 files changed, 12 insertions(+), 5 deletions(-) > > diff --git a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp > b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp > index 075183400a..dd9d02ab11 100644 > --- a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp > +++ b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp > @@ -70,7 +70,6 @@ namespace { > make_kernel_args(const Module &mod, const std::string &kernel_name, > const clang::CompilerInstance &c) { >std::vector args; > - const auto address_spaces = c.getTarget().getAddressSpaceMap(); >const Function &f = *mod.getFunction(kernel_name); >::llvm::DataLayout dl(&mod); >const auto size_type = > @@ -128,8 +127,8 @@ namespace { > const unsigned address_space = >cast< ::llvm::PointerType>(actual_type)->getAddressSpace(); > > - if (address_space == > address_spaces[clang::LangAS::opencl_local > - - > compat::lang_as_offset]) { > + if (address_space == compat::target_lang_address_space( > + c.getTarget(), > clang::LangAS::opencl_local)) { >args.emplace_back(module::argument::local, arg_api_size, > target_size, target_align, > module::argument::zero_ext); > diff --git a/src/gallium/state_trackers/clover/llvm/compat.hpp > b/src/gallium/state_trackers/clover/llvm/compat.hpp > index f8b56516d5..3e34f0dd94 100644 > --- a/src/gallium/state_trackers/clover/llvm/compat.hpp > +++ b/src/gallium/state_trackers/clover/llvm/compat.hpp > @@ -69,11 +69,19 @@ namespace clover { > typedef ::llvm::TargetLibraryInfo target_library_info; > #endif > > + template > + unsigned target_lang_address_space(const T& target, const AS > lang_as) { Can you name this "target_address_space" (to me lang address space means the LangAS enum, i.e. the non-target-dependent representation, which is not what you get as result from this function), and place the '&' sign consistently in the argument declaration? With that cleaned up patch is: Reviewed-by: Francisco Jerez Thanks! > +const auto &map = target.getAddressSpaceMap(); > +#if HAVE_LLVM >= 0x0500 > +return map[static_cast(lang_as)]; > +#else > +return map[lang_as - clang::LangAS::Offset]; > +#endif > + } > + > #if HAVE_LLVM >= 0x0500 > - const auto lang_as_offset = 0; > const clang::InputKind ik_opencl = clang::InputKind::OpenCL; > #else > - const auto lang_as_offset = clang::LangAS::Offset; > const clang::InputKind ik_opencl = clang::IK_OpenCL; > #endif > > -- > 2.13.6 signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] clover: Fix compilation after clang r315871
On Sun, 2017-10-22 at 20:40 -0700, Francisco Jerez wrote: > Jan Vesely writes: > > > From: Jan Vesely > > > > v2: use a more generic compat function > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103388 > > Signed-off-by: Jan Vesely > > --- > > src/gallium/state_trackers/clover/llvm/codegen/common.cpp | 5 ++--- > > src/gallium/state_trackers/clover/llvm/compat.hpp | 12 ++-- > > 2 files changed, 12 insertions(+), 5 deletions(-) > > > > diff --git a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp > > b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp > > index 075183400a..dd9d02ab11 100644 > > --- a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp > > +++ b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp > > @@ -70,7 +70,6 @@ namespace { > > make_kernel_args(const Module &mod, const std::string &kernel_name, > > const clang::CompilerInstance &c) { > >std::vector args; > > - const auto address_spaces = c.getTarget().getAddressSpaceMap(); > >const Function &f = *mod.getFunction(kernel_name); > >::llvm::DataLayout dl(&mod); > >const auto size_type = > > @@ -128,8 +127,8 @@ namespace { > > const unsigned address_space = > >cast< > > ::llvm::PointerType>(actual_type)->getAddressSpace(); > > > > - if (address_space == > > address_spaces[clang::LangAS::opencl_local > > - - > > compat::lang_as_offset]) { > > + if (address_space == compat::target_lang_address_space( > > + c.getTarget(), > > clang::LangAS::opencl_local)) { > >args.emplace_back(module::argument::local, arg_api_size, > > target_size, target_align, > > module::argument::zero_ext); > > diff --git a/src/gallium/state_trackers/clover/llvm/compat.hpp > > b/src/gallium/state_trackers/clover/llvm/compat.hpp > > index f8b56516d5..3e34f0dd94 100644 > > --- a/src/gallium/state_trackers/clover/llvm/compat.hpp > > +++ b/src/gallium/state_trackers/clover/llvm/compat.hpp > > @@ -69,11 +69,19 @@ namespace clover { > > typedef ::llvm::TargetLibraryInfo target_library_info; > > #endif > > > > + template > > + unsigned target_lang_address_space(const T& target, const AS > > lang_as) { > > Can you name this "target_address_space" (to me lang address space means > the LangAS enum, i.e. the non-target-dependent representation, which is > not what you get as result from this function), and place the '&' sign > consistently in the argument declaration? Does this refer only to whitespace around "T& target", or would you prefer to pass lang_as as reference as well? (it's an integral type so I did not consider it necessary). Jan > With that cleaned up patch > is: > > Reviewed-by: Francisco Jerez > > Thanks! > > > +const auto &map = target.getAddressSpaceMap(); > > +#if HAVE_LLVM >= 0x0500 > > +return map[static_cast(lang_as)]; > > +#else > > +return map[lang_as - clang::LangAS::Offset]; > > +#endif > > + } > > + > > #if HAVE_LLVM >= 0x0500 > > - const auto lang_as_offset = 0; > > const clang::InputKind ik_opencl = clang::InputKind::OpenCL; > > #else > > - const auto lang_as_offset = clang::LangAS::Offset; > > const clang::InputKind ik_opencl = clang::IK_OpenCL; > > #endif > > > > -- > > 2.13.6 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
Re: [Mesa-dev] [PATCH v2] clover: Fix compilation after clang r315871
On 10/23/2017 05:24 AM, Jan Vesely wrote: > From: Jan Vesely > > v2: use a more generic compat function > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103388 > Signed-off-by: Jan Vesely > --- > src/gallium/state_trackers/clover/llvm/codegen/common.cpp | 5 ++--- > src/gallium/state_trackers/clover/llvm/compat.hpp | 12 ++-- > 2 files changed, 12 insertions(+), 5 deletions(-) > > diff --git a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp > b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp > index 075183400a..dd9d02ab11 100644 > --- a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp > +++ b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp > @@ -70,7 +70,6 @@ namespace { > make_kernel_args(const Module &mod, const std::string &kernel_name, > const clang::CompilerInstance &c) { >std::vector args; > - const auto address_spaces = c.getTarget().getAddressSpaceMap(); >const Function &f = *mod.getFunction(kernel_name); >::llvm::DataLayout dl(&mod); >const auto size_type = > @@ -128,8 +127,8 @@ namespace { > const unsigned address_space = >cast< ::llvm::PointerType>(actual_type)->getAddressSpace(); > > - if (address_space == > address_spaces[clang::LangAS::opencl_local > - - > compat::lang_as_offset]) { > + if (address_space == compat::target_lang_address_space( > + c.getTarget(), > clang::LangAS::opencl_local)) { >args.emplace_back(module::argument::local, arg_api_size, > target_size, target_align, > module::argument::zero_ext); > diff --git a/src/gallium/state_trackers/clover/llvm/compat.hpp > b/src/gallium/state_trackers/clover/llvm/compat.hpp > index f8b56516d5..3e34f0dd94 100644 > --- a/src/gallium/state_trackers/clover/llvm/compat.hpp > +++ b/src/gallium/state_trackers/clover/llvm/compat.hpp > @@ -69,11 +69,19 @@ namespace clover { > typedef ::llvm::TargetLibraryInfo target_library_info; > #endif > > + template > + unsigned target_lang_address_space(const T& target, const AS > lang_as) { > +const auto &map = target.getAddressSpaceMap(); > +#if HAVE_LLVM >= 0x0500 > +return map[static_cast(lang_as)]; > +#else > +return map[lang_as - clang::LangAS::Offset]; > +#endif > + } > + > #if HAVE_LLVM >= 0x0500 > - const auto lang_as_offset = 0; > const clang::InputKind ik_opencl = clang::InputKind::OpenCL; > #else > - const auto lang_as_offset = clang::LangAS::Offset; > const clang::InputKind ik_opencl = clang::IK_OpenCL; > #endif > > Thanks for improving the patch. Future-proof thinking: what if the value of clang::LangAS::Default changes from 0 to some other constant? Other than that, this patch is: Reviewed-by: Vedran Miletić Regards, Vedran -- Vedran Miletić vedran.miletic.net ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] clover: Fix compilation after clang r315871
Jan Vesely writes: > On Sun, 2017-10-22 at 20:40 -0700, Francisco Jerez wrote: >> Jan Vesely writes: >> >> > From: Jan Vesely >> > >> > v2: use a more generic compat function >> > >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103388 >> > Signed-off-by: Jan Vesely >> > --- >> > src/gallium/state_trackers/clover/llvm/codegen/common.cpp | 5 ++--- >> > src/gallium/state_trackers/clover/llvm/compat.hpp | 12 >> > ++-- >> > 2 files changed, 12 insertions(+), 5 deletions(-) >> > >> > diff --git a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp >> > b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp >> > index 075183400a..dd9d02ab11 100644 >> > --- a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp >> > +++ b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp >> > @@ -70,7 +70,6 @@ namespace { >> > make_kernel_args(const Module &mod, const std::string &kernel_name, >> > const clang::CompilerInstance &c) { >> >std::vector args; >> > - const auto address_spaces = c.getTarget().getAddressSpaceMap(); >> >const Function &f = *mod.getFunction(kernel_name); >> >::llvm::DataLayout dl(&mod); >> >const auto size_type = >> > @@ -128,8 +127,8 @@ namespace { >> > const unsigned address_space = >> >cast< >> > ::llvm::PointerType>(actual_type)->getAddressSpace(); >> > >> > - if (address_space == >> > address_spaces[clang::LangAS::opencl_local >> > - - >> > compat::lang_as_offset]) { >> > + if (address_space == compat::target_lang_address_space( >> > + c.getTarget(), >> > clang::LangAS::opencl_local)) { >> >args.emplace_back(module::argument::local, arg_api_size, >> > target_size, target_align, >> > module::argument::zero_ext); >> > diff --git a/src/gallium/state_trackers/clover/llvm/compat.hpp >> > b/src/gallium/state_trackers/clover/llvm/compat.hpp >> > index f8b56516d5..3e34f0dd94 100644 >> > --- a/src/gallium/state_trackers/clover/llvm/compat.hpp >> > +++ b/src/gallium/state_trackers/clover/llvm/compat.hpp >> > @@ -69,11 +69,19 @@ namespace clover { >> > typedef ::llvm::TargetLibraryInfo target_library_info; >> > #endif >> > >> > + template >> > + unsigned target_lang_address_space(const T& target, const AS >> > lang_as) { >> >> Can you name this "target_address_space" (to me lang address space means >> the LangAS enum, i.e. the non-target-dependent representation, which is >> not what you get as result from this function), and place the '&' sign >> consistently in the argument declaration? > > Does this refer only to whitespace around "T& target", or would you > prefer to pass lang_as as reference as well? (it's an integral type so > I did not consider it necessary). > Nope, I was referring to the target argument only, there's no use in passing lang_as by reference. Thanks. > Jan > >> With that cleaned up patch >> is: >> >> Reviewed-by: Francisco Jerez >> >> Thanks! >> >> > +const auto &map = target.getAddressSpaceMap(); >> > +#if HAVE_LLVM >= 0x0500 >> > +return map[static_cast(lang_as)]; >> > +#else >> > +return map[lang_as - clang::LangAS::Offset]; >> > +#endif >> > + } >> > + >> > #if HAVE_LLVM >= 0x0500 >> > - const auto lang_as_offset = 0; >> > const clang::InputKind ik_opencl = clang::InputKind::OpenCL; >> > #else >> > - const auto lang_as_offset = clang::LangAS::Offset; >> > const clang::InputKind ik_opencl = clang::IK_OpenCL; >> > #endif >> > >> > -- >> > 2.13.6 signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] clover: Fix compilation after clang r315871
On Tue, 2017-10-24 at 15:32 +0200, Vedran Miletić wrote: > On 10/23/2017 05:24 AM, Jan Vesely wrote: > > From: Jan Vesely > > > > v2: use a more generic compat function > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103388 > > Signed-off-by: Jan Vesely > > --- > > src/gallium/state_trackers/clover/llvm/codegen/common.cpp | 5 ++--- > > src/gallium/state_trackers/clover/llvm/compat.hpp | 12 ++-- > > 2 files changed, 12 insertions(+), 5 deletions(-) > > > > diff --git a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp > > b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp > > index 075183400a..dd9d02ab11 100644 > > --- a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp > > +++ b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp > > @@ -70,7 +70,6 @@ namespace { > > make_kernel_args(const Module &mod, const std::string &kernel_name, > > const clang::CompilerInstance &c) { > >std::vector args; > > - const auto address_spaces = c.getTarget().getAddressSpaceMap(); > >const Function &f = *mod.getFunction(kernel_name); > >::llvm::DataLayout dl(&mod); > >const auto size_type = > > @@ -128,8 +127,8 @@ namespace { > > const unsigned address_space = > >cast< > > ::llvm::PointerType>(actual_type)->getAddressSpace(); > > > > - if (address_space == > > address_spaces[clang::LangAS::opencl_local > > - - > > compat::lang_as_offset]) { > > + if (address_space == compat::target_lang_address_space( > > + c.getTarget(), > > clang::LangAS::opencl_local)) { > >args.emplace_back(module::argument::local, arg_api_size, > > target_size, target_align, > > module::argument::zero_ext); > > diff --git a/src/gallium/state_trackers/clover/llvm/compat.hpp > > b/src/gallium/state_trackers/clover/llvm/compat.hpp > > index f8b56516d5..3e34f0dd94 100644 > > --- a/src/gallium/state_trackers/clover/llvm/compat.hpp > > +++ b/src/gallium/state_trackers/clover/llvm/compat.hpp > > @@ -69,11 +69,19 @@ namespace clover { > > typedef ::llvm::TargetLibraryInfo target_library_info; > > #endif > > > > + template > > + unsigned target_lang_address_space(const T& target, const AS > > lang_as) { > > +const auto &map = target.getAddressSpaceMap(); > > +#if HAVE_LLVM >= 0x0500 > > +return map[static_cast(lang_as)]; > > +#else > > +return map[lang_as - clang::LangAS::Offset]; > > +#endif > > + } > > + > > #if HAVE_LLVM >= 0x0500 > > - const auto lang_as_offset = 0; > > const clang::InputKind ik_opencl = clang::InputKind::OpenCL; > > #else > > - const auto lang_as_offset = clang::LangAS::Offset; > > const clang::InputKind ik_opencl = clang::IK_OpenCL; > > #endif > > > > > > Thanks for improving the patch. Future-proof thinking: what if the value > of clang::LangAS::Default changes from 0 to some other constant? Hi Vedran, you're right that it'd be more future proof, but I liked the one line simplicity of the current version. Future clang changes will require adaptations, but I don't expect clang to go back to non-0 lang AS indices. Feel free to add "I told you so" if they prove me wrong :) > > Other than that, this patch is: > > Reviewed-by: Vedran Miletić I was not sure if this applied even without the change so I pushed it only with francisco's rb. thanks, Jan > > Regards, > Vedran > 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
Re: [Mesa-dev] [PATCH v2] clover: Fix compilation after clang r315871
Jan Vesely writes: > On Tue, 2017-10-24 at 15:32 +0200, Vedran Miletić wrote: >> On 10/23/2017 05:24 AM, Jan Vesely wrote: >> > From: Jan Vesely >> > >> > v2: use a more generic compat function >> > >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103388 >> > Signed-off-by: Jan Vesely >> > --- >> > src/gallium/state_trackers/clover/llvm/codegen/common.cpp | 5 ++--- >> > src/gallium/state_trackers/clover/llvm/compat.hpp | 12 >> > ++-- >> > 2 files changed, 12 insertions(+), 5 deletions(-) >> > >> > diff --git a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp >> > b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp >> > index 075183400a..dd9d02ab11 100644 >> > --- a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp >> > +++ b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp >> > @@ -70,7 +70,6 @@ namespace { >> > make_kernel_args(const Module &mod, const std::string &kernel_name, >> > const clang::CompilerInstance &c) { >> >std::vector args; >> > - const auto address_spaces = c.getTarget().getAddressSpaceMap(); >> >const Function &f = *mod.getFunction(kernel_name); >> >::llvm::DataLayout dl(&mod); >> >const auto size_type = >> > @@ -128,8 +127,8 @@ namespace { >> > const unsigned address_space = >> >cast< >> > ::llvm::PointerType>(actual_type)->getAddressSpace(); >> > >> > - if (address_space == >> > address_spaces[clang::LangAS::opencl_local >> > - - >> > compat::lang_as_offset]) { >> > + if (address_space == compat::target_lang_address_space( >> > + c.getTarget(), >> > clang::LangAS::opencl_local)) { >> >args.emplace_back(module::argument::local, arg_api_size, >> > target_size, target_align, >> > module::argument::zero_ext); >> > diff --git a/src/gallium/state_trackers/clover/llvm/compat.hpp >> > b/src/gallium/state_trackers/clover/llvm/compat.hpp >> > index f8b56516d5..3e34f0dd94 100644 >> > --- a/src/gallium/state_trackers/clover/llvm/compat.hpp >> > +++ b/src/gallium/state_trackers/clover/llvm/compat.hpp >> > @@ -69,11 +69,19 @@ namespace clover { >> > typedef ::llvm::TargetLibraryInfo target_library_info; >> > #endif >> > >> > + template >> > + unsigned target_lang_address_space(const T& target, const AS >> > lang_as) { >> > +const auto &map = target.getAddressSpaceMap(); >> > +#if HAVE_LLVM >= 0x0500 >> > +return map[static_cast(lang_as)]; >> > +#else >> > +return map[lang_as - clang::LangAS::Offset]; >> > +#endif >> > + } >> > + >> > #if HAVE_LLVM >= 0x0500 >> > - const auto lang_as_offset = 0; >> > const clang::InputKind ik_opencl = clang::InputKind::OpenCL; >> > #else >> > - const auto lang_as_offset = clang::LangAS::Offset; >> > const clang::InputKind ik_opencl = clang::IK_OpenCL; >> > #endif >> > >> > >> >> Thanks for improving the patch. Future-proof thinking: what if the value >> of clang::LangAS::Default changes from 0 to some other constant? > > Hi Vedran, > > you're right that it'd be more future proof, but I liked the one line > simplicity of the current version. Future clang changes will require > adaptations, but I don't expect clang to go back to non-0 lang AS > indices. Feel free to add "I told you so" if they prove me wrong :) > I think it will be trivial to extend Jan's current approach to non-zero LangAS offset... Just add a new #elif directive to compat::target_address_space() in the unlikely case that it's necessary in the future. >> >> Other than that, this patch is: >> >> Reviewed-by: Vedran Miletić > > I was not sure if this applied even without the change so I pushed it > only with francisco's rb. > > thanks, > Jan > >> >> Regards, >> Vedran >> > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] clover: Fix compilation after clang r315871
26.10.2017 u 01:09, Jan Vesely je napisao/la: > On Tue, 2017-10-24 at 15:32 +0200, Vedran Miletić wrote: >> On 10/23/2017 05:24 AM, Jan Vesely wrote: >>> From: Jan Vesely >>> >>> v2: use a more generic compat function >>> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103388 >>> Signed-off-by: Jan Vesely >>> --- >>> src/gallium/state_trackers/clover/llvm/codegen/common.cpp | 5 ++--- >>> src/gallium/state_trackers/clover/llvm/compat.hpp | 12 ++-- >>> 2 files changed, 12 insertions(+), 5 deletions(-) >>> >>> diff --git a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp >>> b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp >>> index 075183400a..dd9d02ab11 100644 >>> --- a/src/gallium/state_trackers/clover/llvm/codegen/common.cpp >>> +++ b/src/gallium/state_trackers/clover/llvm/codegen/common.cpp >>> @@ -70,7 +70,6 @@ namespace { >>> make_kernel_args(const Module &mod, const std::string &kernel_name, >>> const clang::CompilerInstance &c) { >>>std::vector args; >>> - const auto address_spaces = c.getTarget().getAddressSpaceMap(); >>>const Function &f = *mod.getFunction(kernel_name); >>>::llvm::DataLayout dl(&mod); >>>const auto size_type = >>> @@ -128,8 +127,8 @@ namespace { >>> const unsigned address_space = >>>cast< >>> ::llvm::PointerType>(actual_type)->getAddressSpace(); >>> >>> - if (address_space == >>> address_spaces[clang::LangAS::opencl_local >>> - - >>> compat::lang_as_offset]) { >>> + if (address_space == compat::target_lang_address_space( >>> + c.getTarget(), >>> clang::LangAS::opencl_local)) { >>>args.emplace_back(module::argument::local, arg_api_size, >>> target_size, target_align, >>> module::argument::zero_ext); >>> diff --git a/src/gallium/state_trackers/clover/llvm/compat.hpp >>> b/src/gallium/state_trackers/clover/llvm/compat.hpp >>> index f8b56516d5..3e34f0dd94 100644 >>> --- a/src/gallium/state_trackers/clover/llvm/compat.hpp >>> +++ b/src/gallium/state_trackers/clover/llvm/compat.hpp >>> @@ -69,11 +69,19 @@ namespace clover { >>> typedef ::llvm::TargetLibraryInfo target_library_info; >>> #endif >>> >>> + template >>> + unsigned target_lang_address_space(const T& target, const AS >>> lang_as) { >>> +const auto &map = target.getAddressSpaceMap(); >>> +#if HAVE_LLVM >= 0x0500 >>> +return map[static_cast(lang_as)]; >>> +#else >>> +return map[lang_as - clang::LangAS::Offset]; >>> +#endif >>> + } >>> + >>> #if HAVE_LLVM >= 0x0500 >>> - const auto lang_as_offset = 0; >>> const clang::InputKind ik_opencl = clang::InputKind::OpenCL; >>> #else >>> - const auto lang_as_offset = clang::LangAS::Offset; >>> const clang::InputKind ik_opencl = clang::IK_OpenCL; >>> #endif >>> >>> >> >> Thanks for improving the patch. Future-proof thinking: what if the value >> of clang::LangAS::Default changes from 0 to some other constant? > > Hi Vedran, > > you're right that it'd be more future proof, but I liked the one line > simplicity of the current version. Future clang changes will require > adaptations, but I don't expect clang to go back to non-0 lang AS > indices. Feel free to add "I told you so" if they prove me wrong :) > >> >> Other than that, this patch is: >> >> Reviewed-by: Vedran Miletić > > I was not sure if this applied even without the change so I pushed it > only with francisco's rb. > > thanks, > Jan > It did, I'll make sure to say it more clearly in the future. Regards, Vedran -- Vedran Miletić vedran.miletic.net ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev