[PATCH] D84413: [DA][SDA] SyncDependenceAnalysis re-write

[Simon Moll via Phabricator via llvm-commits]

simoll planned changes to this revision.
simoll marked 10 inline comments as done.
simoll added inline comments.


================
Comment at: llvm/lib/Analysis/DivergenceAnalysis.cpp:220
 
-    assert(!DivLoop->contains(UserBlock) &&
+    assert(!OuterDivLoop.contains(UserBlock) &&
            "irreducible control flow detected");
----------------
sameerds wrote:
> Not necessarily binding on this change, but could this be an error instead? Or perhaps an early exit with a conservative approximation?
It is non-obvious which threads synchronize in irreducible loops.
The convergence semantics of D85603 should clarify this. Until that patch is in, i'd rather we leave this as an assertion.


================
Comment at: llvm/lib/Analysis/SyncDependenceAnalysis.cpp:116-120
+// The SDA algorithm operates on a modified CFG - we modify the edges leaving
+// loop headers as follows:
+//
+// * We remove all edges leaving all loop headers.
+// * We add additional edges from the loop headers to their exit blocks.
----------------
sameerds wrote:
> Is this intended to treat each child loop as a single node when traversing a parent loop? Could we just say that here?
We treat child loops as single nodes to speedup the analysis (all loops are reducible and all blocks of child loops will receive the same label as their loop headers).. This is not part of the changes to the CFG that are required for correctness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84413