[PATCH] D105246: [AbstractAttributor] Fold function calls to `__kmpc_is_spmd_exec_mode` if possible

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Test missing. Other than that mostly nits.



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:460
+  /// Flag to indicate that we may reach a kernel entry that is not tracked.
+  bool MayBeReachedByUnknownKernel = false;
+
----------------
Unsure, I think this way is more natural.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:464
+  /// Note that it could be set to false if the function is already in SPMD
+  /// mode.
   bool IsSPMDCompatible = true;
----------------



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2738
+        A.deleteAfterManifest(*P.first);
+      }
+
----------------
use the `changeValueAfterManifest` interface.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3007-3008
 
   /// Returns true if value is assumed to be tracked.
   bool canBeExecutedInSPMDMode() const { return IsSPMDCompatible; }
 
----------------
Remove


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3025
       return "<invalid>";
-    return std::string(canBeExecutedInSPMDMode() ? "SPMD" : "generic") + std::string("#PRs: ") + std::to_string(ParallelRegions.size()) +
+    return std::string(canBeExecutedInSPMDMode() ? "SPMD" : "generic") +
+           std::string("#PRs: ") + std::to_string(ParallelRegions.size()) +
----------------



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3105
+        },
+        F);
+  }
----------------
Move this before the kernel handling, then you can exit if it is not a kernel. Alternatively, put the kernel handling in a helper.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3152-3153
+
+        // We might encounter an unknown kernel. We cannot fold the associated
+        // function here.
+        MayBeReachedByUnknownKernel = true;
----------------



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3202
+      }
+    }
+
----------------
Maybe put this into a helper as well. And the "reaching kernels" update. Will make it easier to read in the future.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3209
       if (Callee) {
+        A.getOrCreateAAFor<AAKernelInfo>(IRPosition::function(*Callee));
+
----------------
Add a comment why we would want this (here).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105246