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

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 7 10:49:08 PDT 2021


jdoerfert added a comment.

Test missing.



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:538
       return false;
-    if (ReachedUnknownParallelRegions != RHS.ReachedUnknownParallelRegions)
       return false;
----------------
?


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2876
 
-    return ChangeStatus::CHANGED;
+    return Change;
   }
----------------
here we need to return changed or we need to set changed to CHANGED if changeToSPMDMode returned true.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3244
+      updateSPMDFolding(A);
+
     // Callback to check a call instruction.
----------------
Why `!assumed()` ? Wouldn't that mean we only update if we have unknown reaching kernels?


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3304
+    }
+  }
+
----------------
Early exit please.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3313
+                                          DepClassTy::REQUIRED);
+      assert(AA.isValidState() && "AA should be valid here");
+      if (AA.SPMDCompatibilityTracker.isAssumed() ||
----------------
I'm not so sure about this assertion. Maybe handle the case conservatively instead.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3314-3315
+      assert(AA.isValidState() && "AA should be valid here");
+      if (AA.SPMDCompatibilityTracker.isAssumed() ||
+          AA.SPMDCompatibilityTracker.isKnown())
+        ++Count;
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105246



More information about the llvm-commits mailing list