[llvm] r299851 - [InstCombine] fix matching of or-of-icmps constants (PR32524)
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 11 09:12:00 PDT 2017
I have reverted at r299955, so we can see if the bots come back.
On Tue, Apr 11, 2017 at 9:56 AM, Diana Picus <diana.picus at linaro.org> wrote:
> Hi Sanjay,
>
> I think this is also breaking all the ARM/AArch64 selfhost bots, see e.g.
> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-lld/builds/1010
>
> I ruled out 299847 (it's green). We should probably revert.
>
> Thanks,
> Diana
>
> On 11 April 2017 at 17:50, Akira Hatanaka via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > I'm not sure which commit is the culprit yet, but this bot started
> failing
> > too:
> >
> > http://lab.llvm.org:8080/green/job/clang-stage2-configure-
> Rlto_check/14734/
> >
> > On Tue, Apr 11, 2017 at 8:39 AM, Sanjay Patel via llvm-commits
> > <llvm-commits at lists.llvm.org> wrote:
> >>
> >> Thank you for letting me know. I am curious to know how to debug this.
> >>
> >> On Tue, Apr 11, 2017 at 8:36 AM, NAKAMURA Takumi <geek4civic at gmail.com>
> >> wrote:
> >>>
> >>> Seems it causes miscompilation (or revealing a bug) in AMDGPUCodeGen,
> in
> >>> stage 2.
> >>> Investigating.
> >>>
> >>> http://bb.pgr.jp/builders/clang-3stage-x86_64-linux/builds/14928
> >>>
> >>> On Tue, Apr 11, 2017 at 2:08 AM Sanjay Patel via llvm-commits
> >>> <llvm-commits at lists.llvm.org> wrote:
> >>>>
> >>>> Author: spatel
> >>>> Date: Mon Apr 10 11:55:57 2017
> >>>> New Revision: 299851
> >>>>
> >>>> URL: http://llvm.org/viewvc/llvm-project?rev=299851&view=rev
> >>>> Log:
> >>>> [InstCombine] fix matching of or-of-icmps constants (PR32524)
> >>>>
> >>>> Also, make the same change in and-of-icmps and remove a hack for
> >>>> detecting that case.
> >>>>
> >>>> Finally, add some FIXME comments because the code duplication here is
> >>>> awful.
> >>>>
> >>>> This should fix the remaining IR problem noted in:
> >>>> https://bugs.llvm.org/show_bug.cgi?id=32524
> >>>>
> >>>>
> >>>> Modified:
> >>>> llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
> >>>> llvm/trunk/test/Transforms/InstCombine/or.ll
> >>>>
> >>>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.
> cpp
> >>>> URL:
> >>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transform
> s/InstCombine/InstCombineAndOrXor.cpp?rev=299851&r1=299850&
> r2=299851&view=diff
> >>>>
> >>>> ============================================================
> ==================
> >>>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
> >>>> (original)
> >>>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Mon
> >>>> Apr 10 11:55:57 2017
> >>>> @@ -807,6 +807,7 @@ Value *InstCombiner::FoldAndOfICmps(ICmp
> >>>> }
> >>>> }
> >>>>
> >>>> + // FIXME: The code below is duplicated in FoldOrOfICmps.
> >>>> // From here on, we only handle:
> >>>> // (icmp1 A, C1) & (icmp2 A, C2) --> something simpler.
> >>>> if (Val != Val2)
> >>>> @@ -825,11 +826,14 @@ Value *InstCombiner::FoldAndOfICmps(ICmp
> >>>>
> >>>> // Ensure that the larger constant is on the RHS.
> >>>> bool ShouldSwap;
> >>>> - if (CmpInst::isSigned(PredL) ||
> >>>> - (ICmpInst::isEquality(PredL) && CmpInst::isSigned(PredR)))
> >>>> - ShouldSwap = LHSC->getValue().sgt(RHSC->getValue());
> >>>> - else
> >>>> + if (CmpInst::isUnsigned(PredL) || CmpInst::isUnsigned(PredR)) {
> >>>> + // We have an unsigned compare (possibly with an equality
> compare),
> >>>> so treat
> >>>> + // the constants as unsigned.
> >>>> ShouldSwap = LHSC->getValue().ugt(RHSC->getValue());
> >>>> + } else {
> >>>> + // Equality transforms treat the constants as signed.
> >>>> + ShouldSwap = LHSC->getValue().sgt(RHSC->getValue());
> >>>> + }
> >>>>
> >>>> if (ShouldSwap) {
> >>>> std::swap(LHS, RHS);
> >>>> @@ -877,10 +881,6 @@ Value *InstCombiner::FoldAndOfICmps(ICmp
> >>>> case ICmpInst::ICMP_SGT: // (X != 13 & X s> 15) -> X s> 15
> >>>> return RHS;
> >>>> case ICmpInst::ICMP_NE:
> >>>> - // Special case to get the ordering right when the values wrap
> >>>> around
> >>>> - // zero.
> >>>> - if (LHSC->getValue() == 0 && RHSC->getValue().isAllOnesValu
> e())
> >>>> - std::swap(LHSC, RHSC);
> >>>> if (LHSC == SubOne(RHSC)) { // (X != 13 & X != 14) -> X-13 >u 1
> >>>> Constant *AddC = ConstantExpr::getNeg(LHSC);
> >>>> Value *Add = Builder->CreateAdd(Val, AddC, Val->getName() +
> >>>> ".off");
> >>>> @@ -1727,6 +1727,7 @@ Value *InstCombiner::FoldOrOfICmps(ICmpI
> >>>> return Builder->CreateICmpULE(Val, LHSC);
> >>>> }
> >>>>
> >>>> + // FIXME: The code below is duplicated in FoldAndOfICmps.
> >>>> // From here on, we only handle:
> >>>> // (icmp1 A, C1) | (icmp2 A, C2) --> something simpler.
> >>>> if (Val != Val2)
> >>>> @@ -1745,11 +1746,14 @@ Value *InstCombiner::FoldOrOfICmps(ICmpI
> >>>>
> >>>> // Ensure that the larger constant is on the RHS.
> >>>> bool ShouldSwap;
> >>>> - if (CmpInst::isSigned(PredL) ||
> >>>> - (ICmpInst::isEquality(PredL) && CmpInst::isSigned(PredR)))
> >>>> - ShouldSwap = LHSC->getValue().sgt(RHSC->getValue());
> >>>> - else
> >>>> + if (CmpInst::isUnsigned(PredL) || CmpInst::isUnsigned(PredR)) {
> >>>> + // We have an unsigned compare (possibly with an equality
> compare),
> >>>> so treat
> >>>> + // the constants as unsigned.
> >>>> ShouldSwap = LHSC->getValue().ugt(RHSC->getValue());
> >>>> + } else {
> >>>> + // Equality transforms treat the constants as signed.
> >>>> + ShouldSwap = LHSC->getValue().sgt(RHSC->getValue());
> >>>> + }
> >>>>
> >>>> if (ShouldSwap) {
> >>>> std::swap(LHS, RHS);
> >>>>
> >>>> Modified: llvm/trunk/test/Transforms/InstCombine/or.ll
> >>>> URL:
> >>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transfor
> ms/InstCombine/or.ll?rev=299851&r1=299850&r2=299851&view=diff
> >>>>
> >>>> ============================================================
> ==================
> >>>> --- llvm/trunk/test/Transforms/InstCombine/or.ll (original)
> >>>> +++ llvm/trunk/test/Transforms/InstCombine/or.ll Mon Apr 10 11:55:57
> >>>> 2017
> >>>> @@ -223,10 +223,9 @@ define i1 @test19(i32 %A) {
> >>>>
> >>>> define i1 @or_icmps_eq_diff1(i32 %x) {
> >>>> ; CHECK-LABEL: @or_icmps_eq_diff1(
> >>>> -; CHECK-NEXT: [[CMP1:%.*]] = icmp eq i32 %x, -1
> >>>> -; CHECK-NEXT: [[CMP2:%.*]] = icmp eq i32 %x, 0
> >>>> -; CHECK-NEXT: [[LOGIC:%.*]] = or i1 [[CMP1]], [[CMP2]]
> >>>> -; CHECK-NEXT: ret i1 [[LOGIC]]
> >>>> +; CHECK-NEXT: [[X_OFF:%.*]] = add i32 %x, 1
> >>>> +; CHECK-NEXT: [[TMP1:%.*]] = icmp ult i32 [[X_OFF]], 2
> >>>> +; CHECK-NEXT: ret i1 [[TMP1]]
> >>>> ;
> >>>> %cmp1 = icmp eq i32 %x, -1
> >>>> %cmp2 = icmp eq i32 %x, 0
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> llvm-commits mailing list
> >>>> llvm-commits at lists.llvm.org
> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >>
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >>
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170411/a80b53f5/attachment.html>
More information about the llvm-commits
mailing list