[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 a comment.

Thank you for the review.



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


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:3370
+  if (NeedNegation)
+    Res = Builder.CreateNot(Res, "umul.not.ov");
+
----------------
nikic wrote:
> Technically increases instruction count, but that seems justified here, especially as not's will usually be folded.
Yes, i think this is a very rare case where this is ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65143