[PATCH] D113510: [InstCombine] Strip offset when folding and/or of icmps

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1209-1221
+  // Look through addition of a constant offset. This allows us to interpret
+  // the V + C' < C'' range idiom into a proper range.
+  const APInt *Offset1 = nullptr, *Offset2 = nullptr;
+  if (V1 != V2) {
+    Value *X;
+    if (match(V1, m_Add(m_Value(X), m_APInt(Offset1))))
+      V1 = X;
----------------
lebedev.ri wrote:
> What about the case where both of them are add's,
> but we should have matched only one of them,
> i.e. `y = x + C0; q = ((y + C1) < C2) | (y < C3)` ?
Can this happen though? In canonical IR this will get folded to `(x + (C1 + C0) < C2) | (x + C0 < C3)`, at which point we have the expected pattern. (In non-canonical IR this is not the case, but that shouldn't be a problem as this is not a question of correctness -- the fold will apply only once it has become canonical.)


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2467
 
-  // (icmp ult (X + CA), C1) | (icmp eq X, C2) -> (icmp ule (X + CA), C1)
-  //   iff C2 + CA == C1.
----------------
lebedev.ri wrote:
> Hm, but that new code expects two add's.
It doesn't require both comparisons to have an add. We can have an add on neither, both or one of them (this pattern).


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

https://reviews.llvm.org/D113510