[PATCH] D71578: [CodeMoverUtils] Improve IsControlFlowEquivalent.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

I'm generally fine with this. Some comments below.

I'll accept it tomorrow or so if no one else posts another comment.



================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:50
+  if (isa<LoadInst>(I1) || isa<CallInst>(I1))
+    return false;
+
----------------
`CallInst` -> `CallBase`


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:71
+  return true;
+}
+
----------------
Last modification here, make it Values, not Instructions. 


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:73
+
+// FIXME: Reuse GVN/CSE login to check for equivalence between Values.
+bool llvm::ControlConditionsComparator::isEquivalent(const Value &V1,
----------------
Typo: logic


================
Comment at: llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp:479
+}
+
+TEST(CodeMoverUtils, IsSafeToMoveTest) {
----------------
Maybe add my example from the last review just to make sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71578