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

[Marc Auberer via Phabricator via llvm-commits]

marcauberer added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:3153
+}
+
 /// Return true if it is known that V1 != V2.
----------------
goldstein.w.n wrote:
> marcauberer wrote:
> > goldstein.w.n wrote:
> > > This at the very least should be a seperate patch and should probably fit into:
> > > `IsCondKnownTrue` not be its own API.
> > I cannot find `IsCondKnownTrue`. Where can I find it?
> Crap I pasted the wrong thing. Mean: `simplifyICmpInst`
I think I don't quite understand ... This newly added function is essentially the same as `computeKnownBitsFromAssume` or `isKnownNonZeroFromAssume` just for `isKnownNonEqual`. Why should this fit into `simplifyICmpInst`. Isn't this relevant overall for ValueTracking?
I agree regarding the split into a new patch.


================
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:
> 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()));
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159203