[PATCH] D32215: Updated PPCG Code Generation for OpenCL compatibility

[Philipp Schaad via Phabricator via llvm-commits]

PhilippSchaad added inline comments.


================
Comment at: lib/CodeGen/PPCGCodeGeneration.cpp:1306
     } else {
-      Args.push_back(Builder.getInt8PtrTy());
+      Args.push_back(Builder.getInt8Ty()->getPointerTo(1));
     }
----------------
bollu wrote:
> PhilippSchaad wrote:
> > bollu wrote:
> > > 1. Change the magic constant `1` to a `static const GlobalMemory = 1` or something of the sort.
> > > 2. This change seems unrelated to the initial change. Can this be split into two separate patches?
> > > 3. Please add a test case that makes sure that the correct code is generated.
> > 1. What's the issue exactly?
> > 2. The change should do exactly the same thing, just in one statement instead of two. Before, I used getInt8Ty, and then got a pointer to it, declaring address space 1 = globa, now I'm doing it in one step.
> > 3. Sure, can do
> From what I understand, the constant `1` is used to refer to the global address space in the NVPTX backend (please correct me if I am wrong). Could you change the invocation to:
> 
> ```lang=cpp
> static const int UseGlobalMemory = 1;
> Args.push_back(Builder.getInt8PtrTy(UseGlobalMemory));
> 
> ```
> 
> I personally dislike magic constants in code. Documenting it this way should save the next person a google search.
Oh that's what you mean, yes, in that case I agree with you. This is a better solution. I will update that, thanks for pointing it out.


Repository:
  rL LLVM

https://reviews.llvm.org/D32215