[PATCH] D65017: [InstCombine] Teach foldOrOfICmps to allow icmp eq MIN_INT/MAX to be part of a range comparision. Similar for foldAndOfICmps

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Ok, LGTM, thank you.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2269-2272
+      // (X == 0 || X u> C) -> (X-1) u>= C
+      if (LHSC->isMinValue(false))
+        return insertRangeTest(LHS0, LHSC->getValue() + 1, RHSC->getValue() + 1,
+                               false, false);
----------------
https://rise4fun.com/Alive/3RLT ok


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2276-2279
+      // (X == INT_MIN || X s> C) -> (X-(INT_MIN+1)) u>= C-INT_MIN
+      if (LHSC->isMinValue(true))
+        return insertRangeTest(LHS0, LHSC->getValue() + 1, RHSC->getValue() + 1,
+                               true, false);
----------------
https://rise4fun.com/Alive/1ri ok


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2289-2292
+      // (X u< C || X == UINT_MAX) => (X-C) u>= UINT_MAX-C
+      if (RHSC->isMaxValue(false))
+        return insertRangeTest(LHS0, LHSC->getValue(), RHSC->getValue(),
+                               false, false);
----------------
https://rise4fun.com/Alive/zq4 ok


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2304-2308
+    case ICmpInst::ICMP_EQ:
+      // (X s< C || X == INT_MAX) => (X-C) u>= INT_MAX-C
+      if (RHSC->isMaxValue(true))
+        return insertRangeTest(LHS0, LHSC->getValue(), RHSC->getValue(),
+                               true, false);
----------------
https://rise4fun.com/Alive/xb2 ok


================
Comment at: llvm/test/Transforms/InstCombine/and-or-icmps.ll:316
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp ugt i32 [[X_OFF]], 2147483645
+; CHECK-NEXT:    ret i1 [[TMP1]]
 ;
----------------
lebedev.ri wrote:
> craig.topper wrote:
> > nikic wrote:
> > > This seems like a weird choice that the range check code is making. We could also generate
> > > 
> > > ```
> > > %x.off = add i32 %x, 1
> > > %c = icmp ult i32 %x.off, -2147483646
> > > ```
> > > 
> > > with smaller constants.
> > Is this something this patch should address?
> > Is this something this patch should address?
> 
> I'm going to go with no. That is a separate fold.
This might warrant filing an issue to track though.


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

https://reviews.llvm.org/D65017