[PATCH] D140850: [InstCombine] Add optimizations for icmp eq/ne (mul(X, Y), 0)

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:1311
+  if (match(Cmp.getOperand(0), m_Mul(m_Value(X), m_Value(Y))) &&
+      ICmpInst::isEquality(Pred)) {
+    KnownBits XKnown = computeKnownBits(X, 0, &Cmp);
----------------
Something of a style nit, but I think it would be more elegant to write the contents of this if as follows:
```
KnownBits XKnown = computeKnownBits(X, 0, &Cmp);
if (XKnown.countMaxTrailingZeros() != 0)
  return new ICmpInst(Pred, Y, Cmp.getOperand(1));

KnownBits YKnown = computeKnownBits(Y, 0, &Cmp);
if (YKnown.countMaxTrailingZeros() != 0)
  return new ICmpInst(Pred, X, Cmp.getOperand(1));

if (BO0->hasNoUnsignedWrap() || BO0->hasNoSignedWrap()) {
  const SimplifyQuery Q = SQ.getWithInstruction(&Cmp);
  if (isKnownNonZero(X, DL, 0, Q.AC, Q.CxtI, Q.DT))
    return new ICmpInst(Pred, Y, Cmp.getOperand(1));
  if (isKnownNonZero(Y, DL, 0, Q.AC, Q.CxtI, Q.DT))
    return new ICmpInst(Pred, X, Cmp.getOperand(1));
}
```
We don't really need to handle the case where both are unneeded explicitly (it will be picked up when simplifying the icmp, though in practice it will even be simplified before it reaches here), and then I think the code is simpler if you just forgo the XUnneeeded etc variables entirely.

Could also move the comments above this whole if to the respective clauses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140850