[PATCH] D159203: [InstCombine] Fold (A/-B)==(A/B) to (A/B)==0

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:5174
+        match(Op1, m_SDiv(m_Deferred(A), m_Deferred(B))) &&
+        isKnownNonEqual(B, MinVal, DL, &AC, &I, &DT, true)) {
+      return new ICmpInst(Pred, Op1, Constant::getNullValue(A->getType()));
----------------
marcauberer wrote:
> marcauberer wrote:
> > goldstein.w.n wrote:
> > > Can you split the `isKnownNonEqual` to a new patch and use either `IsCondKnownTrue` here or probably more direct in this case just use `computeKnownBits`.
> > How would I do that with `computeKnownBits`?
> > 
> > The following code does not seem to work, probably due to the fact that the assumption is not made for value A or B, but rather the icmp ne of A/B with MIN_INT:
> > 
> > 
> > ```
> > // Check if A is known to be != INT_MIN
> > KnownBits KnownA = computeKnownBits(A, 0, &I);
> > if (KnownA.isConstant() && KnownA.getConstant() == MinInt)
> >   return new ICmpInst(Pred, Op1, Constant::getNullValue(A->getType()));
> > ```
> @goldstein.w.n ping? Or is this resolved with extracting the `isKnownNonEqual` part to a new patch? This is done now.
Ah sorry didn't see your earlier message.

You don't really need to/want to be testing `isKnownNonEqual` here. You want to be testing this particular fold.
which doesn't inherently rely on `isKnownNonEqual`, it just relies on A/B != `INT_MIN`.

The latter can sometimes be proven with knownbits, for example `(a | 1) == INT_MAX` is known always false.

Since the isKnownNonEqual patch will probably take longer to get in, I would suggest doing something like:
`!computeKnownBits(A, ...).getSignedMinValue().isMinSignedValue()`



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159203