[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
llvm-commits at lists.llvm.org
Wed Jul 24 00:29:40 PDT 2019
lebedev.ri requested changes to this revision.
lebedev.ri added inline comments.
This revision now requires changes to proceed.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1208-1211
+ // (X != INT_MIN & X s< 14) -> X-(INT_MIN+1) u< (14-(INT_MIN+1))
+ if (LHSC->isMinValue(true))
+ return insertRangeTest(LHS0, LHSC->getValue() + 1, RHSC->getValue(),
+ true, true);
----------------
Ok, https://rise4fun.com/Alive/VUp
Comment does not specify the precondition that `C != INT_MIN`
I'm guessing that case is handled elsewhere?
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1226-1229
+ // X u> 13 & X != UINT_MAX -> (X-14) u< UINT_MAX-14
+ if (RHSC->isMaxValue(false))
+ return insertRangeTest(LHS0, LHSC->getValue() + 1, RHSC->getValue(),
+ false, true);
----------------
Update comment please: `X u> 13 & X != UINT_MAX -> (X-(C+1)) u< UINT_MAX-(C+1)`
https://rise4fun.com/Alive/qt2, ok
Comment does not specify the precondition that `C != UINT_MAX`
I'm guessing that case is handled elsewhere?
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1244-1247
+ // X s> 13 & X != INT_MAX -> (X-14) u< INT_MAX-14
+ if (RHSC->isMaxValue(true))
+ return insertRangeTest(LHS0, LHSC->getValue() + 1, RHSC->getValue(),
+ true, true);
----------------
Fix comment please: `X s> 13 & X != INT_MAX -> (X-(C+1))) u< INT_MAX-(C+1)`
https://rise4fun.com/Alive/hZlO, ok
Comment does not specify the precondition that `C != INT_MAX`
I'm guessing that case is handled elsewhere?
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2269-2309
+ // We can treat equality of UINT_MIN the same as ULE UINT_MIN.
+ if (LHSC->isMinValue(false))
+ return insertRangeTest(LHS0, LHSC->getValue() + 1, RHSC->getValue() + 1,
+ false, false);
+ // (X == 13 | X u> 14) -> no change
+ break;
+ case ICmpInst::ICMP_SGT:
----------------
Can you please add actual formulas here as comments?
================
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]]
;
----------------
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65017/new/
https://reviews.llvm.org/D65017
More information about the llvm-commits
mailing list