[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:1283
+  } else {
+    Ret += "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:" \
+            "64-f32:32:32-f64:64:64-v16:16:16-v32:32:32-v64:64:" \
----------------
bollu wrote:
> Please add a test case to check that both code paths are taken. 
Not both branches are being taken at the moment. The only invocation of this function, at this time, is being done with constant true. The 32-bit part was already present pre-patch, I just adapted BOTH strings to fit the ones recommended by the NVPTX backend. (See [[ http://llvm.org/docs/NVPTXUsage.html#data-layout | NVPTX Usage ]])


================
Comment at: lib/CodeGen/PPCGCodeGeneration.cpp:1306
     } else {
-      Args.push_back(Builder.getInt8PtrTy());
+      Args.push_back(Builder.getInt8Ty()->getPointerTo(1));
     }
----------------
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


Repository:
  rL LLVM

https://reviews.llvm.org/D32215