[llvm] r299851 - [InstCombine] fix matching of or-of-icmps constants (PR32524)
Diana Picus via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 11 08:56:30 PDT 2017
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/Transforms/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/Transforms/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
>
More information about the llvm-commits
mailing list