[Mesa-dev] [PATCH v2] clover: Fix compilation after clang r315871

[Francisco Jerez]

Jan Vesely <jan.vesely at rutgers.edu> 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 <jan.vesely at rutgers.edu>
>> > 
>> > v2: use a more generic compat function
>> > 
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103388
>> > Signed-off-by: Jan Vesely <jan.vesely at rutgers.edu>
>> > ---
>> >  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<module::argument> 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<typename T, typename AS>
>> > +         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<unsigned>(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ć <vedran at miletic.net>
>
> 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- 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/20171025/86e28df9/attachment.sig>