[PATCH] D105634: [OpenMP] Use AAHeapToStack/AAHeapToShared analysis in SPMDization

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

Some minor things to address but LGTM.



================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:5718
+  bool isAssumedHeapToStackRemovedFree(CallBase &CB) const override {
+    if (isValidState())
+      for (auto &It : AllocationInfos) {
----------------
Nit: early exit please.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2543
 
+  void findPotentialRemovedFreeCalls(Attributor &A) {
+    auto &OMPInfoCache = static_cast<OMPInformationCache &>(A.getInfoCache());
----------------
Add brief doxygen description please.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2556
+      }
+      // TODO: Do we need to assume there is one, unique free user?
+      if (FreeCalls.size() != 1)
----------------
Remove the todo, keep the check though.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2576-2580
+    if (isValidState())
+      if (MallocCalls.count(&CB))
+        return true;
+
+    return false;
----------------



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2587
+        return true;
+    return false;
   }
----------------
same as above.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3476
+    assert(It->getSecond() == OMPRTL___kmpc_alloc_shared ||
+           It->getSecond() == OMPRTL___kmpc_free_shared);
+
----------------
Nit: Add a message.


================
Comment at: llvm/test/Transforms/OpenMP/spmdization.ll:68
+; CHECK: @[[__OMP_OFFLOADING_50_29CC7474_SEQUENTIAL_LOOP_TO_STACK_VAR_L20_EXEC_MODE:[a-zA-Z0-9_$"\\.-]+]] = weak constant i8 0
+; CHECK: @[[__OMP_OFFLOADING_50_29CC7474_SEQUENTIAL_LOOP_TO_SHARED_VAR_L35_EXEC_MODE:[a-zA-Z0-9_$"\\.-]+]] = weak constant i8 0
+; CHECK: @[[LLVM_COMPILER_USED:[a-zA-Z0-9_$"\\.-]+]] = appending global [3 x i8*] [i8* @__omp_offloading_50_29cc7474_sequential_loop_l5_exec_mode, i8* @__omp_offloading_50_29cc7474_sequential_loop_to_stack_var_l20_exec_mode, i8* @__omp_offloading_50_29cc7474_sequential_loop_to_shared_var_l35_exec_mode], section "llvm.metadata"
----------------
Rebase and this is probably going to be a 2 not a 0 anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105634