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

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.

LGTM



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1209-1210
     IRBuilderBase &Builder, bool IsAnd) {
+  // 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;
----------------
nikic wrote:
> 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.)
On first look, I also didn't catch that this block will match either 1 or 2 offsets, so we should make that clear in the code comment.
Something like:
  // Look through add of a constant offset on V1, V2, or both operands...


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

https://reviews.llvm.org/D113510