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

Siddharth via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 9 00:45:24 PDT 2017


bollu added a comment.

This can be done in another patch, but I think the `Index++` calls inside `for` loops can be hoisted as `for(..; ..; i++, Index++)`. I personally find this clearer, but I'm not sure what the others think.



================
Comment at: lib/CodeGen/PPCGCodeGeneration.cpp:1199
+  int NumArgs = std::distance(F->arg_begin(), F->arg_end());
+  int *ArgSizes = new int[NumArgs];
+
----------------
bollu wrote:
> why not `std::vector<int>(NumArgs)`? That removes the explicit memory allocation/deallocation. I wish `std::dynarray` existed, since that's exactly the use case for this. 
Have you considered `std::vector<int> ArgSizes(NumArgs);`? I find this more idiomatic.


================
Comment at: lib/CodeGen/PPCGCodeGeneration.cpp:146
+/// Given a LLVM Type, compute its size in bytes,
+static int computeSizeInBytes(Type *T) {
+  int bytes = T->getPrimitiveSizeInBits() / 8;
----------------
`getPrimitiveSizeInBits` and `getScalarSizeInBits` accept `const Type *T`. Could you please change the `Type *T` to `const Type *T`?


https://reviews.llvm.org/D32961





More information about the llvm-commits mailing list