[PATCH] D97818: [OpenMP] Replace GPU globalization calls with shared memory in the middle-end

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:68
+                                   "threshold in shared memory"),
+                          cl::init(8));
+
----------------
I don't think we need this after all.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:1010
+      if (!CB)
+        return false;
+
----------------
For now, check `isKernel(F)` and only do this for kernel functions. Later we can be more aggressive but for now that should limit it properly, also with regards to the lifetime of those allocations.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:1068
+      OMPInfoCache.recollectUsesForFunction(OMPRTL___kmpc_alloc_shared);
+      OMPInfoCache.recollectUsesForFunction(OMPRTL___kmpc_free_shared);
+      NumBytesMovedToSharedMemory += AllocSize->getZExtValue();
----------------
You cannot recollect while looping.
Move the recollect for free after the loop, delete the other one. Return `true` from this function when it deleted the use, that will update the alloc use list.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:1072
+
+      return false;
+    };
----------------
`return true;`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97818