[PATCH] D97818: [OpenMP] Remove GPU globalization calls in the middle-end

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

some initial comments.



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:988
+
+    auto removeAllocCalls = [&](Use &U, Function &F) {
+      auto &FreeCall = OMPInfoCache.RFIs[OMPRTL___kmpc_free_shared];
----------------



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:991
+      CallBase *CB =
+          dyn_cast<CallBase>(OpenMPOpt::getCallIfRegularCall(U, &RFI));
+      if (!CB)
----------------
No cast needed


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:996
+      IRPosition AllocPos = IRPosition::callsite_returned(*CB);
+      if (A.lookupAAFor<AANoCapture>(AllocPos)->isKnownNoCapture()) {
+        Constant *AllocSize = dyn_cast<Constant>(CB->getArgOperand(0));
----------------
prefer early exit, `if (!...) return false;`


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:999
+        if (!AllocSize)
+          return false;
+
----------------
No need for it to be a constant


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:1003
+        if (!FC || FC->getCalledFunction() != FreeCall.Declaration)
+          return false;
+
----------------
Don't assume an order. Check all users, one should be a free, others can be whatever. If you find bitcast users, remember the type, if they all agree, use that for the alloca.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:1645
+                                                 OMPRTL___kmpc_alloc_shared};
+    for (const auto GlobalizationCallID : GlobalizationRuntimeIDs) {
+      auto &RFI = OMPInfoCache.RFIs[GlobalizationCallID];
----------------
no need to go over the free calls. they need to be users of the alloc and we remove them with the alloc.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:1652
+
+        auto &CB = cast<CallBase>(*CI);
+
----------------
No need, use *CI below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97818