[PATCH] D65143: [InstCombine] Fold '(-1 u/ %x) u< %y' to '@llvm.umul.with.overflow' + overflow bit extraction

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked 3 inline comments as done.
lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:3349
+    if (I.getOperand(1) != Y)
+      Pred = I.getSwappedPredicate();
+
----------------
nikic wrote:
> lebedev.ri wrote:
> > nikic wrote:
> > > Side note: This seems pretty awkward and potentially bug prone. Having looked through the relatively few uses of this matcher, most cases either don't care (because they handle equalities) or would benefit from returning the swapped predicate. I think we should change the semantics to remove this potential footgun...
> > ... i.e. you suggest that `m_c_ICmp()` should swap predicate as needed?
> Yes. `m_c_ICmp(Pred, X, Y)` should match `X Pred Y` or an equivalent commutative form, which is `Y SwappedPred X`, not `Y Pred X` (apart from the degenerate case of equality). At least that's what I would intuitively expect and what would be useful in practice (such as here).
Got it.
Looks like right now there is only 20 uses of `m_c_ICmp()` so it seems doable.
Filed https://bugs.llvm.org/show_bug.cgi?id=42801


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65143