[Openmp-commits] [PATCH] D109468: [OpenMP][FIX] Be more deliberate about invalidating the AAKernelInfo state

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:613
     if (KIS.KernelInitCB) {
-      if (KernelInitCB && KernelInitCB != KIS.KernelInitCB)
-        indicatePessimisticFixpoint();
+      if(KernelInitCB && KernelInitCB != KIS.KernelInitCB)
+        llvm_unreachable("Kernel that calls another kernel violates OpenMP-Opt assumptions.");
----------------
format


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3012
     if (!changeToSPMDMode(A))
-      buildCustomStateMachine(A);
+      return buildCustomStateMachine(A);
 
----------------
Don't you expect anything else between in the future?


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3310
     if (DisableOpenMPOptStateMachineRewrite)
-      return indicatePessimisticFixpoint();
+      return ChangeStatus::UNCHANGED;
 
----------------
>From `Attributor`'s perspective, they behave same, don't they?


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3848
       SPMDCompatibilityTracker.insert(&CB);
       ReachedUnknownParallelRegions.insert(&CB);
+      break;
----------------
IIRC, `ReachedUnknownParallelRegions` will be invalidated on insertion. With this change, this AA indicates optimistic fixed point?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109468