[PATCH] D71578: [CodeMoverUtils] Improve IsControlFlowEquivalent.

[Whitney Tsang via Phabricator via llvm-commits]

Whitney added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:130
+      Conditions.addControlCondition(*BI->getCondition(), false);
+    }
+
----------------
jdoerfert wrote:
> The dom tree queries (`DT.dominates(BI->getSuccessor(1), CurBlock)`) confuse me. The block that contains BI is the immediate dominator of CurBlock, right?
> I might be mistaken here but doesn't this mean these queries are only true if the successor and cur block are the same?
> 
> If I'm right, code like the one below and a query %from -> %to should fail with the assertion above.:
> ```
> from:
>   br label %idom
> idom:
>   br i1 %c0, label %lvl0a, label %lvl0b
> lvl0a:
>   br i1 %c1, label %lvl1a, label %lvl1b
> lvl0b:
>   br i1 %c1, label %lvl1b, label %lvl1c
> lvl1a:
>   br label %to
> lvl1b:
>   ret void
> lvl1c:
>   br label %to
> to:
>   ret void
> ```
> 
> For now you can give up if the idom is not post dominated by the current block and not the predecessor block of the current one.
Good catch, `DT.dominates(BI->getSuccessor(1), CurBlock)` only true when `BI->getSuccessor(1)` is `CurBlock`. 
Cannot give up if the Idom is not post dominated by the CurBlock, as when CurBlock post dominates Idom, then CurBlock is executed unconditionally from Idom, i.e. no control condition is required. 
I am changing to `PDT.dominates(CurBlock, BI->getSuccessor(1))`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71578