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

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:1314
+    KnownBits YKnown = computeKnownBits(Y, 0, &Cmp);
+    bool XExtra = XKnown.countMaxTrailingZeros() == 0;
+    bool YExtra = YKnown.countMaxTrailingZeros() == 0;
----------------
nikic wrote:
> YExtra -> YOdd if I understand the intention correctly.
> 
> Edit: Hm, or is the naming like this because it's odd in one case and non-zero in the other?
> YExtra -> YOdd if I understand the intention correctly.
> 
> Edit: Hm, or is the naming like this because it's odd in one case and non-zero in the other?

Yeah it was originally `YOdd` but to avoid having the rewrite the cases for `YNonZero` I renamed to extra. I can add a comment in V2 or do something like:
```
bool YOdd, YNonZero;
YNonZero = false;
YOdd = XKnown.countMaxTrailingZeros() == 0;
...
if(!YOdd && !XOdd) {
YNonZero = sKnownNonZero(X, DL, 0, Q.AC, Q.CxtI, Q.DT);
...
}
XExtra = YOdd | YNonZero;
...
```

Let me know what you prefer.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:1319
+      // transformation using overflow information and zero/non-status
+      BinaryOperator *BO0 = dyn_cast<BinaryOperator>(Cmp.getOperand(0));
+      if (BO0 && (BO0->hasNoUnsignedWrap() || BO0->hasNoSignedWrap())) {
----------------
nikic wrote:
> `cast<OverflowingBinaryOperator>` would always work (including for mul constant expressions).
> `cast<OverflowingBinaryOperator>` would always work (including for mul constant expressions).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140850