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

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:3349
+    if (I.getOperand(1) != Y)
+      Pred = I.getSwappedPredicate();
+
----------------
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).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65143