[PATCH] D147597: [InstCombine] Fold icmp(bin(X, Y) | LHS, RHS) --> icmp(bin(X, Y)) iff LHS > RHS s>= 0

[Jun Zhang via Phabricator via llvm-commits]

junaire added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:1959
+  // icmp(bin(X, Y) | LHS, C) --> icmp(bin(X, Y)) iff LHS > C s>= 0
+  if (match(Or, m_c_Or(m_BinOp(BO), m_APInt(LHS)))) {
+    if (LHS->isStrictlyPositive() && C.isNonNegative() && LHS->sgt(C)) {
----------------
goldstein.w.n wrote:
> I think the BinOp aspect of the transform is meaningless:
> https://alive2.llvm.org/ce/z/cbWvv7
> 
> Also you're alive2 links still don't match the logic (you are only do `C == 0`).
> You can create the constraints you need with `@llvm.assume(i1)`
> I tried to implement here: https://alive2.llvm.org/ce/z/rmBJXx
> but it doesn't verify.
I agree we should remove the binop match, but the alive2 link you provided is not what the patch trying to transform. It should be https://alive2.llvm.org/ce/z/ybKpxy (in tgt we compare with 0, not RHS)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147597