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