[PATCH] D32961: [Polly][PPCGCodeGen] OpenCL now gets kernel argument size from PPCG CodeGen

[Philipp Schaad via Phabricator via llvm-commits]

PhilippSchaad marked 3 inline comments as done.
PhilippSchaad added inline comments.


================
Comment at: lib/CodeGen/PPCGCodeGeneration.cpp:1198
                                        SetVector<Value *> SubtreeValues) {
-  Type *ArrayTy = ArrayType::get(Builder.getInt8PtrTy(),
-                                 std::distance(F->arg_begin(), F->arg_end()));
+  int NumArgs = std::distance(F->arg_begin(), F->arg_end());
+  int *ArgSizes = new int[NumArgs];
----------------
bollu wrote:
> bollu wrote:
> > I believe `int NumArgs = F->getArgumentList().size()` will also work? seems a little clearer to me.
> `NumArgs` could be `const`? It doesn't seem to be mutated anywhere.
The type llvm::Function does not seem to have a member getArgumentList()? Also, I have just adapted that from before, but I agree that something like that would be clearer.


================
Comment at: lib/CodeGen/PPCGCodeGeneration.cpp:1347
+    Builder.CreateStore(ParamTyped, Slot);
+    Index++;
+  }
----------------
bollu wrote:
> Could the pattern of
> 
> ```lang=cpp, name=to-split.cpp
> Value *Slot = Builder.CreateGEP(
>         Parameters, {Builder.getInt64(0), Builder.getInt64(Index)});
>     Value *ParamTyped =
>         Builder.CreatePointerCast(Param, Builder.getInt8PtrTy());
>     Builder.CreateStore(ParamTyped, Slot);
> ```
> 
> be refactored into a separate function if it is not too much trouble? It occurs thrice in this function (`GPUNodeBuilder::createLaunchParameters`)
Looking into it.


Repository:
  rL LLVM

https://reviews.llvm.org/D32961