[PATCH] D37822: [OpenCL] Clean up and add missing fields for block struct

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 4 inline comments as done.
yaxunl added inline comments.


================
Comment at: lib/CodeGen/CGBlocks.cpp:311
+    // The header is basically 'struct { int; int; generic void *;
+    // custom_fields; }'. Assert that that struct is packed.
+    auto GenPtrAlign = CharUnits::fromQuantity(
----------------
Anastasia wrote:
> remove one "that".
will do.


================
Comment at: lib/CodeGen/CGBlocks.cpp:312
+    // custom_fields; }'. Assert that that struct is packed.
+    auto GenPtrAlign = CharUnits::fromQuantity(
+        CGM.getTarget().getPointerAlign(LangAS::opencl_generic) / 8);
----------------
Anastasia wrote:
> I think the alignment might not be computed correctly now if there will be custom fields that might have a bigger size than a pointer? Also what happens if we have captures as well?
Will fix.

The captures will be accounted for by computeBlockInfo and BlockSize and BlockAlign will be updated.


================
Comment at: lib/CodeGen/CGBlocks.cpp:850
+                           CGM.getDataLayout().getTypeAllocSize(I->getType())),
+                       "block.custom");
+      }
----------------
Anastasia wrote:
> do we need to add numeration to each item name?
yes. will add it.


================
Comment at: lib/CodeGen/CGBlocks.cpp:1250
   // Function
   fields.add(blockFn);
 
----------------
Anastasia wrote:
> If we reorder fields and put this on top we can merge the if statements above and below this point.
By convention the size of the whole struct is the first field so that the library function reads the first integer and knows how many bytes to copy.


https://reviews.llvm.org/D37822