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

[Marc Auberer via Phabricator via llvm-commits]

marcauberer marked an inline comment as done.
marcauberer 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()));
----------------
goldstein.w.n wrote:
> 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()`
> 
Done.
I had to adjust the tests a bit, because with the original precondition (A != MIN_INT) we are not able to compute any known bits. If we have A != NOT_MIN_INT, we are.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159203