[PATCH] D71578: [CodeMoverUtils] Improve IsControlFlowEquivalent.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Thanks for working on this. I was hoping we go in this direction eventually.

I haven't looked at everything in details but I have some high-level comments we should probably address/discuss first.

We should make `collectDependingConditions` visible from the outside.
Can we rename `Conditions` into `ControlConditions` (also use that term instead of "depending conditions")?
I would store the information about the range (from which block to which) in the ControlCondition object so it becomes self contained.

Further comments inlined.



================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:41
+  SmallVector<const Value *, 2> TrueConditions;
+  SmallVector<const Value *, 2> FalseConditions;
+
----------------
The class, members and methods need documentation.


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:101
+            !IsDifferent(Other.getFalseConditions(), FalseConditions,
+                         TrueConditions));
+  }
----------------
What if you would only track positive conditions (negate the others) and you put them into a set. The is equivalent check will then be imple equality check on the sets. You can then also check "implication" through subset relation, and missing condition, through set difference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71578