[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/Analysis/ValueTracking.cpp:3153
+}
+
 /// Return true if it is known that V1 != V2.
----------------
This at the very least should be a seperate patch and should probably fit into:
`IsCondKnownTrue` not be its own API.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:5167
 
+  // (A / -B) == (A / B) -> (A / B) == 0
+  {
----------------
This comment should include 'if B != INT_MIN'.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:5173
+    if (match(Op0, m_OneUse(m_SDiv(m_Value(A), m_OneUse(m_Neg(m_Value(B)))))) &&
+        match(Op1, m_SDiv(m_Deferred(A), m_Deferred(B))) &&
+        isKnownNonEqual(B, MinVal, DL, &AC, &I, &DT, true)) {
----------------
`m_Deferred` -> `m_Specific`. You aren't rematching `A`/`B` in the same `match` statement.


================
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()));
----------------
Can you split the `isKnownNonEqual` to a new patch and use either `IsCondKnownTrue` here or probably more direct in this case just use `computeKnownBits`.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:5175
+        isKnownNonEqual(B, MinVal, DL, &AC, &I, &DT, true)) {
+      return new ICmpInst(Pred, Op1, Constant::getNullValue(A->getType()));
+    }
----------------
You can strengthen this as it also works if `A != INT_MAX`: https://alive2.llvm.org/ce/z/MFAjAs so if check for B fails you can tryout A.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159203