[llvm-branch-commits] [llvm] [SimplifyCFG] Avoid threading loop-header branches in convergent functions (PR #204958)

[Eli Friedman via llvm-branch-commits]

================
@@ -3713,7 +3713,10 @@ bool SimplifyCFGOpt::foldCondBranchOnValueKnownInPredecessor(CondBrInst *BI) {
   // Note: If BB is a loop header then there is a risk that threading introduces
   // a non-canonical loop by moving a back edge. So we avoid this optimization
   // for loop headers if NeedCanonicalLoop is set.
-  if (Options.NeedCanonicalLoop && is_contained(LoopHeaders, BI->getParent()))
+  // Also avoid threading loop headers in convergent functions, since changing
+  // the branch structure can change the dynamic instances of convergent ops.
+  if ((Options.NeedCanonicalLoop || BI->getFunction()->isConvergent()) &&
+      is_contained(LoopHeaders, BI->getParent()))
----------------
efriedma-quic wrote:

I guess I'd break it down into two problems:

One is whether the set of "headers" is sufficiently complete for your purposes.  It's currently computed using FindFunctionBackedges.  I guess this is actually slightly different from what we'd consider a "loop header" in other places for irreducible CFGs.  It's a bit weird because it's actually sensitive to the order of the successors: it tracks down some "backedges", but not others, depending on which block it visits first.  But in any case, I don't think it contains every block you'd care about if an irreducible CFG is involved.  We can probably fix this by changing around the algorithm a bit.  Not sure what algorithm we need off the top of my head.

The other problem is that we don't keep the LoopHeaders up to date; it was primarily intended as an optimization barrier, so it didn't really matter if some weird cases got out of sync, so it's just using WeakVH.  But if you're depending on it for correctness, it's important that we keep it up to date.  We probably want to add updating code, then assert that the updated list is correct after SimplifyCFG runs, then maybe try to do a bit of fuzzing.

https://github.com/llvm/llvm-project/pull/204958