[llvm] r299851 - [InstCombine] fix matching of or-of-icmps constants (PR32524)

Tue Apr 11 08:50:53 PDT 2017

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().isAllOnesValue())
>>> -        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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170411/14b5ff25/attachment.html>