[PATCH] D153883: [Clang][OpenMP] Enable use of __kmpc_alloc_shared for VLAs defined in AMD GPU offloaded regions

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Add the runtime test?



================
Comment at: clang/lib/CodeGen/CGDecl.cpp:587
+    std::pair<llvm::Value *, llvm::Value *> AddrSizePair;
+    KmpcAllocFree(std::pair<llvm::Value *, llvm::Value *> AddrSizePair)
+        : AddrSizePair(AddrSizePair) {}
----------------
Better to pass it as const reference


================
Comment at: clang/lib/CodeGen/CGDecl.cpp:589
+        : AddrSizePair(AddrSizePair) {}
+    void Emit(CodeGenFunction &CGF, Flags flags) override {
+      CGOpenMPRuntimeGPU &RT =
----------------
Wrong param name, use Camel


================
Comment at: clang/lib/CodeGen/CGDecl.cpp:1603
+    // deallocation call of __kmpc_free_shared() is emitted later.
+    if (getLangOpts().OpenMP && getTarget().getTriple().isAMDGCN()) {
+      // Emit call to __kmpc_alloc_shared() instead of the alloca.
----------------
OpenMPIsDevice?


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1085
   }
-  for (const auto *VD : I->getSecond().EscapedVariableLengthDecls) {
-    // Use actual memory size of the VLA object including the padding
----------------
Why this code is removed?


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1112
+
+  return std::pair<llvm::Value *, llvm::Value *>({VoidPtr, Size});
+}
----------------
Use `std::make_pair(VoidPtr, Size)`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153883/new/

https://reviews.llvm.org/D153883