<div dir="ltr">On Fri, Mar 18, 2016 at 4:29 AM, Renato Golin <span dir="ltr"><<a href="mailto:renato.golin@linaro.org" target="_blank">renato.golin@linaro.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Saleem,<br>
<br>
Makes sense. The change is not disruptive and fixes a clear bug on a<br>
specific target. I've included it in my list for 3.8.1 backports. When<br>
we start the cycle, I'll backport it.<br></blockquote><div><br></div><div>Thanks!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
cheers,<br>
--renato<br>
<br>
On 18 March 2016 at 02:16, Saleem Abdulrasool via llvm-commits<br>
<div class="HOEnZb"><div class="h5"><<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
> On Thu, Mar 17, 2016 at 7:10 AM, Saleem Abdulrasool via llvm-commits<br>
> <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>> Author: compnerd<br>
>> Date: Thu Mar 17 09:10:49 2016<br>
>> New Revision: 263714<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=263714&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=263714&view=rev</a><br>
>> Log:<br>
>> ARM: Revert SVN r253865, 254158, fix windows division<br>
>><br>
>> The two changes together weakened the test and caused a regression with<br>
>> division<br>
>> handling in MSVC mode.  They were applied to avoid an assertion being<br>
>> triggered<br>
>> in the block frequency analysis.  However, the underlying problem was<br>
>> simply<br>
>> being masked rather than solved properly.  Address the actual underlying<br>
>> problem<br>
>> and revert the changes.  Rather than analyze the cause of the assertion,<br>
>> the<br>
>> division failure was assumed to be an overflow.<br>
>><br>
>> The underlying issue was a subtle bug in the BB construction in the<br>
>> emission of<br>
>> the div-by-zero check (WIN__DBZCHK).  We did not construct the proper<br>
>> successor<br>
>> information in the basic blocks, nor did we update the PHIs associated<br>
>> with the<br>
>> basic block when we split them.  This would result in assertions being<br>
>> triggered<br>
>> in the block frequency analysis pass.<br>
>><br>
>> Although the original tests are being removed, the tests themselves<br>
>> performed<br>
>> very little in terms of validation but merely tested that we did not<br>
>> assert when<br>
>> generating code.  Update this with new tests that actually ensure that we<br>
>> do not<br>
>> regress on the code generation.<br>
><br>
><br>
> Can this be pulled into the 3.8.1 release?  Emission of division library<br>
> calls on Windows ARM is broken when optimizations are enabled.<br>
><br>
>><br>
>> Added:<br>
>>     llvm/trunk/test/CodeGen/ARM/Windows/dbzchk.ll<br>
>> Removed:<br>
>>     llvm/trunk/test/CodeGen/ARM/Windows/overflow.ll<br>
>> Modified:<br>
>>     llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp<br>
>>     llvm/trunk/test/CodeGen/ARM/Windows/division.ll<br>
>><br>
>> Modified: llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp?rev=263714&r1=263713&r2=263714&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp?rev=263714&r1=263713&r2=263714&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp (original)<br>
>> +++ llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp Thu Mar 17 09:10:49 2016<br>
>> @@ -390,10 +390,6 @@ ARMTargetLowering::ARMTargetLowering(con<br>
>>        { RTLIB::SINTTOFP_I64_F64, "__i64tod", CallingConv::ARM_AAPCS_VFP<br>
>> },<br>
>>        { RTLIB::UINTTOFP_I64_F32, "__u64tos", CallingConv::ARM_AAPCS_VFP<br>
>> },<br>
>>        { RTLIB::UINTTOFP_I64_F64, "__u64tod", CallingConv::ARM_AAPCS_VFP<br>
>> },<br>
>> -      { RTLIB::SDIV_I32, "__rt_sdiv",   CallingConv::ARM_AAPCS_VFP },<br>
>> -      { RTLIB::UDIV_I32, "__rt_udiv",   CallingConv::ARM_AAPCS_VFP },<br>
>> -      { RTLIB::SDIV_I64, "__rt_sdiv64", CallingConv::ARM_AAPCS_VFP },<br>
>> -      { RTLIB::UDIV_I64, "__rt_udiv64", CallingConv::ARM_AAPCS_VFP },<br>
>>      };<br>
>><br>
>>      for (const auto &LC : LibraryCalls) {<br>
>> @@ -781,6 +777,14 @@ ARMTargetLowering::ARMTargetLowering(con<br>
>>      setOperationAction(ISD::UDIV,  MVT::i32, LibCall);<br>
>>    }<br>
>><br>
>> +  if (Subtarget->isTargetWindows() && !Subtarget->hasDivide()) {<br>
>> +    setOperationAction(ISD::SDIV, MVT::i32, Custom);<br>
>> +    setOperationAction(ISD::UDIV, MVT::i32, Custom);<br>
>> +<br>
>> +    setOperationAction(ISD::SDIV, MVT::i64, Custom);<br>
>> +    setOperationAction(ISD::UDIV, MVT::i64, Custom);<br>
>> +  }<br>
>> +<br>
>>    setOperationAction(ISD::SREM,  MVT::i32, Expand);<br>
>>    setOperationAction(ISD::UREM,  MVT::i32, Expand);<br>
>>    // Register based DivRem for AEABI (RTABI 4.2)<br>
>> @@ -7010,8 +7014,14 @@ SDValue ARMTargetLowering::LowerOperatio<br>
>>    case ISD::CONCAT_VECTORS: return LowerCONCAT_VECTORS(Op, DAG);<br>
>>    case ISD::FLT_ROUNDS_:   return LowerFLT_ROUNDS_(Op, DAG);<br>
>>    case ISD::MUL:           return LowerMUL(Op, DAG);<br>
>> -  case ISD::SDIV:          return LowerSDIV(Op, DAG);<br>
>> -  case ISD::UDIV:          return LowerUDIV(Op, DAG);<br>
>> +  case ISD::SDIV:<br>
>> +    if (Subtarget->isTargetWindows())<br>
>> +      return LowerDIV_Windows(Op, DAG, /* Signed */ true);<br>
>> +    return LowerSDIV(Op, DAG);<br>
>> +  case ISD::UDIV:<br>
>> +    if (Subtarget->isTargetWindows())<br>
>> +      return LowerDIV_Windows(Op, DAG, /* Signed */ false);<br>
>> +    return LowerUDIV(Op, DAG);<br>
>>    case ISD::ADDC:<br>
>>    case ISD::ADDE:<br>
>>    case ISD::SUBC:<br>
>> @@ -8008,7 +8018,7 @@ ARMTargetLowering::EmitLowered__dbzchk(M<br>
>>    MF->push_back(ContBB);<br>
>>    ContBB->splice(ContBB->begin(), MBB,<br>
>>                   std::next(MachineBasicBlock::iterator(MI)), MBB->end());<br>
>> -  MBB->addSuccessor(ContBB);<br>
>> +  ContBB->transferSuccessorsAndUpdatePHIs(MBB);<br>
>><br>
>>    MachineBasicBlock *TrapBB = MF->CreateMachineBasicBlock();<br>
>>    MF->push_back(TrapBB);<br>
>> @@ -8018,6 +8028,7 @@ ARMTargetLowering::EmitLowered__dbzchk(M<br>
>>    BuildMI(*MBB, MI, DL, TII->get(ARM::tCBZ))<br>
>>        .addReg(MI->getOperand(0).getReg())<br>
>>        .addMBB(TrapBB);<br>
>> +  MBB->addSuccessor(ContBB);<br>
>><br>
>>    MI->eraseFromParent();<br>
>>    return ContBB;<br>
>><br>
>> Added: llvm/trunk/test/CodeGen/ARM/Windows/dbzchk.ll<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/Windows/dbzchk.ll?rev=263714&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/Windows/dbzchk.ll?rev=263714&view=auto</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/test/CodeGen/ARM/Windows/dbzchk.ll (added)<br>
>> +++ llvm/trunk/test/CodeGen/ARM/Windows/dbzchk.ll Thu Mar 17 09:10:49 2016<br>
>> @@ -0,0 +1,80 @@<br>
>> +; RUN: llc -mtriple thumbv7--windows-itanium<br>
>> -print-machineinstrs=expand-isel-pseudos -o /dev/null %s 2>&1 | FileCheck %s<br>
>> -check-prefix CHECK-DIV<br>
>> +<br>
>> +; int f(int n, int d) {<br>
>> +;   if (n / d)<br>
>> +;     return 1;<br>
>> +;   return 0;<br>
>> +; }<br>
>> +<br>
>> +define arm_aapcs_vfpcc i32 @f(i32 %n, i32 %d) {<br>
>> +entry:<br>
>> +  %retval = alloca i32, align 4<br>
>> +  %n.addr = alloca i32, align 4<br>
>> +  %d.addr = alloca i32, align 4<br>
>> +  store i32 %n, i32* %n.addr, align 4<br>
>> +  store i32 %d, i32* %d.addr, align 4<br>
>> +  %0 = load i32, i32* %n.addr, align 4<br>
>> +  %1 = load i32, i32* %d.addr, align 4<br>
>> +  %div = sdiv i32 %0, %1<br>
>> +  %tobool = icmp ne i32 %div, 0<br>
>> +  br i1 %tobool, label %if.then, label %if.end<br>
>> +<br>
>> +if.then:<br>
>> +  store i32 1, i32* %retval, align 4<br>
>> +  br label %return<br>
>> +<br>
>> +if.end:<br>
>> +  store i32 0, i32* %retval, align 4<br>
>> +  br label %return<br>
>> +<br>
>> +return:<br>
>> +  %2 = load i32, i32* %retval, align 4<br>
>> +  ret i32 %2<br>
>> +}<br>
>> +<br>
>> +; CHECK-DIV-DAG: BB#0<br>
>> +; CHECK-DIV-DAG: Successors according to CFG: BB#5({{.*}}) BB#4<br>
>> +; CHECK-DIV-DAG: BB#1<br>
>> +; CHECK-DIV-DAG: Successors according to CFG: BB#3<br>
>> +; CHECK-DIV-DAG: BB#2<br>
>> +; CHECK-DIV-DAG: Successors according to CFG: BB#3<br>
>> +; CHECK-DIV-DAG: BB#3<br>
>> +; CHECK-DIV-DAG: BB#4<br>
>> +; CHECK-DIV-DAG: Successors according to CFG: BB#1({{.*}}) BB#2<br>
>> +; CHECK-DIV-DAG: BB#5<br>
>> +<br>
>> +; RUN: llc -mtriple thumbv7--windows-itanium<br>
>> -print-machineinstrs=expand-isel-pseudos -o /dev/null %s 2>&1 | FileCheck %s<br>
>> -check-prefix CHECK-MOD<br>
>> +<br>
>> +; int r;<br>
>> +; int g(int l, int m) {<br>
>> +;   if (m <= 0)<br>
>> +;     return 0;<br>
>> +;   return (r = l % m);<br>
>> +; }<br>
>> +<br>
>> +@r = common global i32 0, align 4<br>
>> +<br>
>> +define arm_aapcs_vfpcc i32 @g(i32 %l, i32 %m) {<br>
>> +entry:<br>
>> +  %cmp = icmp eq i32 %m, 0<br>
>> +  br i1 %cmp, label %return, label %if.end<br>
>> +<br>
>> +if.end:<br>
>> +  %rem = urem i32 %l, %m<br>
>> +  store i32 %rem, i32* @r, align 4<br>
>> +  br label %return<br>
>> +<br>
>> +return:<br>
>> +  %retval.0 = phi i32 [ %rem, %if.end ], [ 0, %entry ]<br>
>> +  ret i32 %retval.0<br>
>> +}<br>
>> +<br>
>> +; CHECK-MOD-DAG: BB#0<br>
>> +; CHECK-MOD-DAG: Successors according to CFG: BB#2({{.*}}) BB#1<br>
>> +; CHECK-MOD-DAG: BB#1<br>
>> +; CHECK-MOD-DAG: Successors according to CFG: BB#4({{.*}}) BB#3<br>
>> +; CHECK-MOD-DAG: BB#2<br>
>> +; CHECK-MOD-DAG: BB#3<br>
>> +; CHECK-MOD-DAG: Successors according to CFG: BB#2<br>
>> +; CHECK-MOD-DAG: BB#4<br>
>> +<br>
>><br>
>> Modified: llvm/trunk/test/CodeGen/ARM/Windows/division.ll<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/Windows/division.ll?rev=263714&r1=263713&r2=263714&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/Windows/division.ll?rev=263714&r1=263713&r2=263714&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/test/CodeGen/ARM/Windows/division.ll (original)<br>
>> +++ llvm/trunk/test/CodeGen/ARM/Windows/division.ll Thu Mar 17 09:10:49<br>
>> 2016<br>
>> @@ -7,8 +7,10 @@ entry:<br>
>>    ret i32 %div<br>
>>  }<br>
>><br>
>> -; CHECK-LABEL: sdiv32<br>
>> -; CHECK: b __rt_sdiv<br>
>> +; CHECK-LABEL: sdiv32:<br>
>> +; CHECK: cbz r0<br>
>> +; CHECK: bl __rt_sdiv<br>
>> +; CHECK: udf.w #249<br>
>><br>
>>  define arm_aapcs_vfpcc i32 @udiv32(i32 %divisor, i32 %divident) {<br>
>>  entry:<br>
>> @@ -17,7 +19,9 @@ entry:<br>
>>  }<br>
>><br>
>>  ; CHECK-LABEL: udiv32:<br>
>> -; CHECK: b __rt_udiv<br>
>> +; CHECK: cbz r0<br>
>> +; CHECK: bl __rt_udiv<br>
>> +; CHECK: udf.w #249<br>
>><br>
>>  define arm_aapcs_vfpcc i64 @sdiv64(i64 %divisor, i64 %divident) {<br>
>>  entry:<br>
>> @@ -25,8 +29,11 @@ entry:<br>
>>    ret i64 %div<br>
>>  }<br>
>><br>
>> -; CHECK-LABEL: sdiv64<br>
>> +; CHECK-LABEL: sdiv64:<br>
>> +; CHECK: orr.w r12, r0, r1<br>
>> +; CHECK-NEXT: cbz r12<br>
>>  ; CHECK: bl __rt_sdiv64<br>
>> +; CHECK: udf.w #249<br>
>><br>
>>  define arm_aapcs_vfpcc i64 @udiv64(i64 %divisor, i64 %divident) {<br>
>>  entry:<br>
>> @@ -35,4 +42,8 @@ entry:<br>
>>  }<br>
>><br>
>>  ; CHECK-LABEL: udiv64:<br>
>> +; CHECK: orr.w r12, r0, r1<br>
>> +; CHECK-NEXT: cbz r12<br>
>>  ; CHECK: bl __rt_udiv64<br>
>> +; CHECK: udf.w #249<br>
>> +<br>
>><br>
>> Removed: llvm/trunk/test/CodeGen/ARM/Windows/overflow.ll<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/Windows/overflow.ll?rev=263713&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/Windows/overflow.ll?rev=263713&view=auto</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/test/CodeGen/ARM/Windows/overflow.ll (original)<br>
>> +++ llvm/trunk/test/CodeGen/ARM/Windows/overflow.ll (removed)<br>
>> @@ -1,77 +0,0 @@<br>
>> -; RUN: llc -mtriple thumbv7-windows-gnu -filetype asm -o - %s<br>
>> -<br>
>> -define i32 @divsoverflow32(i32 %a, i32 %b) {<br>
>> -  %1 = alloca i32, align 4<br>
>> -  %2 = alloca i32, align 4<br>
>> -  %3 = load i32, i32* %1, align 4<br>
>> -  %4 = load i32, i32* %2, align 4<br>
>> -  %5 = sub nsw i32 0, %4<br>
>> -  %6 = sdiv i32 -2147483647, %3<br>
>> -  %7 = icmp sgt i32 %5, %6<br>
>> -  br i1 %7, label %8, label %9<br>
>> -  call void (...) @abort_simpl32()<br>
>> -  unreachable<br>
>> -  %10 = load i32, i32* %1, align 4<br>
>> -  %11 = load i32, i32* %2, align 4<br>
>> -  %12 = mul nsw i32 %10, %11<br>
>> -  ret i32 %12<br>
>> -}<br>
>> -<br>
>> -declare void @abort_simpl32(...)<br>
>> -<br>
>> -define i64 @divsoverflow64(i64 %a, i64 %b) {<br>
>> -  %1 = alloca i64, align 8<br>
>> -  %2 = alloca i64, align 8<br>
>> -  %3 = load i64, i64* %1, align 8<br>
>> -  %4 = load i64, i64* %2, align 8<br>
>> -  %5 = sub nsw i64 0, %4<br>
>> -  %6 = sdiv i64 -9223372036854775808, %3<br>
>> -  %7 = icmp sgt i64 %5, %6<br>
>> -  br i1 %7, label %8, label %9<br>
>> -  call void (...) @abort_simpl64()<br>
>> -  unreachable<br>
>> -  %10 = load i64, i64* %1, align 8<br>
>> -  %11 = load i64, i64* %2, align 8<br>
>> -  %12 = mul nsw i64 %10, %11<br>
>> -  ret i64 %12<br>
>> -}<br>
>> -<br>
>> -declare void @abort_simpl64(...)<br>
>> -<br>
>> -define i32 @divuoverflow32(i32 %a, i32 %b) {<br>
>> -  %1 = alloca i32, align 4<br>
>> -  %2 = alloca i32, align 4<br>
>> -  %3 = load i32, i32* %1, align 4<br>
>> -  %4 = load i32, i32* %2, align 4<br>
>> -  %5 = sub nsw i32 0, %4<br>
>> -  %6 = udiv i32 4294967296, %3<br>
>> -  %7 = icmp sgt i32 %5, %6<br>
>> -  br i1 %7, label %8, label %9<br>
>> -  call void (...) @abort_uimpl32()<br>
>> -  unreachable<br>
>> -  %10 = load i32, i32* %1, align 4<br>
>> -  %11 = load i32, i32* %2, align 4<br>
>> -  %12 = mul nsw i32 %10, %11<br>
>> -  ret i32 %12<br>
>> -}<br>
>> -<br>
>> -declare void @abort_uimpl32(...)<br>
>> -<br>
>> -define i64 @divuoverflow64(i64 %a, i64 %b) {<br>
>> -  %1 = alloca i64, align 8<br>
>> -  %2 = alloca i64, align 8<br>
>> -  %3 = load i64, i64* %1, align 8<br>
>> -  %4 = load i64, i64* %2, align 8<br>
>> -  %5 = sub nsw i64 0, %4<br>
>> -  %6 = udiv i64 18446744073709551616, %3<br>
>> -  %7 = icmp sgt i64 %5, %6<br>
>> -  br i1 %7, label %8, label %9<br>
>> -  call void (...) @abort_uimpl64()<br>
>> -  unreachable<br>
>> -  %10 = load i64, i64* %1, align 8<br>
>> -  %11 = load i64, i64* %2, align 8<br>
>> -  %12 = mul nsw i64 %10, %11<br>
>> -  ret i64 %12<br>
>> -}<br>
>> -<br>
>> -declare void @abort_uimpl64(...)<br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@lists.llvm.org">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/mailman/listinfo/llvm-commits</a><br>
><br>
><br>
><br>
><br>
> --<br>
> Saleem Abdulrasool<br>
> compnerd (at) compnerd (dot) org<br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org">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/mailman/listinfo/llvm-commits</a><br>
><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature">Saleem Abdulrasool<br>compnerd (at) compnerd (dot) org</div>
</div></div>