[PATCH] D71578: [CodeMoverUtils] Improve IsControlFlowEquivalent.

[Whitney Tsang via Phabricator via llvm-commits]

Whitney added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:208
+      if (Cmp1->getPredicate() ==
+              CmpInst::getSwappedPredicate(Cmp2->getInversePredicate()) &&
+          Cmp1->getOperand(0) == Cmp2->getOperand(1) &&
----------------
fhahn wrote:
> nit: should we cover the swapped case in isEquivalent?
Let's keep isEquivalent simple, and see if other passes (e.g. GVN) can handle those cases properly first. 


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:320
+  DT.updateDFSNumbers();
+  const bool MoveForward = OI.dfsBefore(&I, &InsertPoint);
   if (MoveForward) {
----------------
fhahn wrote:
> nit: I think it would be clearer to use OI.dominates, like t line 333. I think with OI.dominates, the DFS numbers should be updated on demand, automatically.
I intensionally use OI.dfsBefore instead of OI.dominates, as they are not equivalent.  Unit test IsSafeToMoveTest2 illustrate the need. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71578