<div dir="ltr">On Thu, Mar 17, 2016 at 7:10 AM, Saleem Abdulrasool via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.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">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 division<br>
handling in MSVC mode.  They were applied to avoid an assertion being triggered<br>
in the block frequency analysis.  However, the underlying problem was simply<br>
being masked rather than solved properly.  Address the actual underlying problem<br>
and revert the changes.  Rather than analyze the cause of the assertion, 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 emission of<br>
the div-by-zero check (WIN__DBZCHK).  We did not construct the proper successor<br>
information in the basic blocks, nor did we update the PHIs associated with the<br>
basic block when we split them.  This would result in assertions being triggered<br>
in the block frequency analysis pass.<br>
<br>
Although the original tests are being removed, the tests themselves performed<br>
very little in terms of validation but merely tested that we did not assert when<br>
generating code.  Update this with new tests that actually ensure that we do not<br>
regress on the code generation.<br></blockquote><div><br></div><div>Can this be pulled into the 3.8.1 release?  Emission of division library calls on Windows ARM is broken when optimizations are enabled.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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: <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>
--- 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>
       { RTLIB::UINTTOFP_I64_F32, "__u64tos", CallingConv::ARM_AAPCS_VFP },<br>
       { RTLIB::UINTTOFP_I64_F64, "__u64tod", CallingConv::ARM_AAPCS_VFP },<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: <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>
--- 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 -print-machineinstrs=expand-isel-pseudos -o /dev/null %s 2>&1 | FileCheck %s -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 -print-machineinstrs=expand-isel-pseudos -o /dev/null %s 2>&1 | FileCheck %s -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: <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>
--- llvm/trunk/test/CodeGen/ARM/Windows/division.ll (original)<br>
+++ llvm/trunk/test/CodeGen/ARM/Windows/division.ll Thu Mar 17 09:10:49 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: <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>
--- 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>
</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>