[llvm] r263714 - ARM: Revert SVN r253865, 254158, fix windows division

Saleem Abdulrasool via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 17 18:16:10 PDT 2016


On Thu, Mar 17, 2016 at 7:10 AM, Saleem Abdulrasool via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: compnerd
> Date: Thu Mar 17 09:10:49 2016
> New Revision: 263714
>
> URL: http://llvm.org/viewvc/llvm-project?rev=263714&view=rev
> Log:
> ARM: Revert SVN r253865, 254158, fix windows division
>
> The two changes together weakened the test and caused a regression with
> division
> handling in MSVC mode.  They were applied to avoid an assertion being
> triggered
> in the block frequency analysis.  However, the underlying problem was
> simply
> being masked rather than solved properly.  Address the actual underlying
> problem
> and revert the changes.  Rather than analyze the cause of the assertion,
> the
> division failure was assumed to be an overflow.
>
> The underlying issue was a subtle bug in the BB construction in the
> emission of
> the div-by-zero check (WIN__DBZCHK).  We did not construct the proper
> successor
> information in the basic blocks, nor did we update the PHIs associated
> with the
> basic block when we split them.  This would result in assertions being
> triggered
> in the block frequency analysis pass.
>
> Although the original tests are being removed, the tests themselves
> performed
> very little in terms of validation but merely tested that we did not
> assert when
> generating code.  Update this with new tests that actually ensure that we
> do not
> regress on the code generation.
>

Can this be pulled into the 3.8.1 release?  Emission of division library
calls on Windows ARM is broken when optimizations are enabled.


> Added:
>     llvm/trunk/test/CodeGen/ARM/Windows/dbzchk.ll
> Removed:
>     llvm/trunk/test/CodeGen/ARM/Windows/overflow.ll
> Modified:
>     llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
>     llvm/trunk/test/CodeGen/ARM/Windows/division.ll
>
> Modified: llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp?rev=263714&r1=263713&r2=263714&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp Thu Mar 17 09:10:49 2016
> @@ -390,10 +390,6 @@ ARMTargetLowering::ARMTargetLowering(con
>        { RTLIB::SINTTOFP_I64_F64, "__i64tod", CallingConv::ARM_AAPCS_VFP },
>        { RTLIB::UINTTOFP_I64_F32, "__u64tos", CallingConv::ARM_AAPCS_VFP },
>        { RTLIB::UINTTOFP_I64_F64, "__u64tod", CallingConv::ARM_AAPCS_VFP },
> -      { RTLIB::SDIV_I32, "__rt_sdiv",   CallingConv::ARM_AAPCS_VFP },
> -      { RTLIB::UDIV_I32, "__rt_udiv",   CallingConv::ARM_AAPCS_VFP },
> -      { RTLIB::SDIV_I64, "__rt_sdiv64", CallingConv::ARM_AAPCS_VFP },
> -      { RTLIB::UDIV_I64, "__rt_udiv64", CallingConv::ARM_AAPCS_VFP },
>      };
>
>      for (const auto &LC : LibraryCalls) {
> @@ -781,6 +777,14 @@ ARMTargetLowering::ARMTargetLowering(con
>      setOperationAction(ISD::UDIV,  MVT::i32, LibCall);
>    }
>
> +  if (Subtarget->isTargetWindows() && !Subtarget->hasDivide()) {
> +    setOperationAction(ISD::SDIV, MVT::i32, Custom);
> +    setOperationAction(ISD::UDIV, MVT::i32, Custom);
> +
> +    setOperationAction(ISD::SDIV, MVT::i64, Custom);
> +    setOperationAction(ISD::UDIV, MVT::i64, Custom);
> +  }
> +
>    setOperationAction(ISD::SREM,  MVT::i32, Expand);
>    setOperationAction(ISD::UREM,  MVT::i32, Expand);
>    // Register based DivRem for AEABI (RTABI 4.2)
> @@ -7010,8 +7014,14 @@ SDValue ARMTargetLowering::LowerOperatio
>    case ISD::CONCAT_VECTORS: return LowerCONCAT_VECTORS(Op, DAG);
>    case ISD::FLT_ROUNDS_:   return LowerFLT_ROUNDS_(Op, DAG);
>    case ISD::MUL:           return LowerMUL(Op, DAG);
> -  case ISD::SDIV:          return LowerSDIV(Op, DAG);
> -  case ISD::UDIV:          return LowerUDIV(Op, DAG);
> +  case ISD::SDIV:
> +    if (Subtarget->isTargetWindows())
> +      return LowerDIV_Windows(Op, DAG, /* Signed */ true);
> +    return LowerSDIV(Op, DAG);
> +  case ISD::UDIV:
> +    if (Subtarget->isTargetWindows())
> +      return LowerDIV_Windows(Op, DAG, /* Signed */ false);
> +    return LowerUDIV(Op, DAG);
>    case ISD::ADDC:
>    case ISD::ADDE:
>    case ISD::SUBC:
> @@ -8008,7 +8018,7 @@ ARMTargetLowering::EmitLowered__dbzchk(M
>    MF->push_back(ContBB);
>    ContBB->splice(ContBB->begin(), MBB,
>                   std::next(MachineBasicBlock::iterator(MI)), MBB->end());
> -  MBB->addSuccessor(ContBB);
> +  ContBB->transferSuccessorsAndUpdatePHIs(MBB);
>
>    MachineBasicBlock *TrapBB = MF->CreateMachineBasicBlock();
>    MF->push_back(TrapBB);
> @@ -8018,6 +8028,7 @@ ARMTargetLowering::EmitLowered__dbzchk(M
>    BuildMI(*MBB, MI, DL, TII->get(ARM::tCBZ))
>        .addReg(MI->getOperand(0).getReg())
>        .addMBB(TrapBB);
> +  MBB->addSuccessor(ContBB);
>
>    MI->eraseFromParent();
>    return ContBB;
>
> Added: llvm/trunk/test/CodeGen/ARM/Windows/dbzchk.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/Windows/dbzchk.ll?rev=263714&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/ARM/Windows/dbzchk.ll (added)
> +++ llvm/trunk/test/CodeGen/ARM/Windows/dbzchk.ll Thu Mar 17 09:10:49 2016
> @@ -0,0 +1,80 @@
> +; RUN: llc -mtriple thumbv7--windows-itanium
> -print-machineinstrs=expand-isel-pseudos -o /dev/null %s 2>&1 | FileCheck
> %s -check-prefix CHECK-DIV
> +
> +; int f(int n, int d) {
> +;   if (n / d)
> +;     return 1;
> +;   return 0;
> +; }
> +
> +define arm_aapcs_vfpcc i32 @f(i32 %n, i32 %d) {
> +entry:
> +  %retval = alloca i32, align 4
> +  %n.addr = alloca i32, align 4
> +  %d.addr = alloca i32, align 4
> +  store i32 %n, i32* %n.addr, align 4
> +  store i32 %d, i32* %d.addr, align 4
> +  %0 = load i32, i32* %n.addr, align 4
> +  %1 = load i32, i32* %d.addr, align 4
> +  %div = sdiv i32 %0, %1
> +  %tobool = icmp ne i32 %div, 0
> +  br i1 %tobool, label %if.then, label %if.end
> +
> +if.then:
> +  store i32 1, i32* %retval, align 4
> +  br label %return
> +
> +if.end:
> +  store i32 0, i32* %retval, align 4
> +  br label %return
> +
> +return:
> +  %2 = load i32, i32* %retval, align 4
> +  ret i32 %2
> +}
> +
> +; CHECK-DIV-DAG: BB#0
> +; CHECK-DIV-DAG: Successors according to CFG: BB#5({{.*}}) BB#4
> +; CHECK-DIV-DAG: BB#1
> +; CHECK-DIV-DAG: Successors according to CFG: BB#3
> +; CHECK-DIV-DAG: BB#2
> +; CHECK-DIV-DAG: Successors according to CFG: BB#3
> +; CHECK-DIV-DAG: BB#3
> +; CHECK-DIV-DAG: BB#4
> +; CHECK-DIV-DAG: Successors according to CFG: BB#1({{.*}}) BB#2
> +; CHECK-DIV-DAG: BB#5
> +
> +; RUN: llc -mtriple thumbv7--windows-itanium
> -print-machineinstrs=expand-isel-pseudos -o /dev/null %s 2>&1 | FileCheck
> %s -check-prefix CHECK-MOD
> +
> +; int r;
> +; int g(int l, int m) {
> +;   if (m <= 0)
> +;     return 0;
> +;   return (r = l % m);
> +; }
> +
> + at r = common global i32 0, align 4
> +
> +define arm_aapcs_vfpcc i32 @g(i32 %l, i32 %m) {
> +entry:
> +  %cmp = icmp eq i32 %m, 0
> +  br i1 %cmp, label %return, label %if.end
> +
> +if.end:
> +  %rem = urem i32 %l, %m
> +  store i32 %rem, i32* @r, align 4
> +  br label %return
> +
> +return:
> +  %retval.0 = phi i32 [ %rem, %if.end ], [ 0, %entry ]
> +  ret i32 %retval.0
> +}
> +
> +; CHECK-MOD-DAG: BB#0
> +; CHECK-MOD-DAG: Successors according to CFG: BB#2({{.*}}) BB#1
> +; CHECK-MOD-DAG: BB#1
> +; CHECK-MOD-DAG: Successors according to CFG: BB#4({{.*}}) BB#3
> +; CHECK-MOD-DAG: BB#2
> +; CHECK-MOD-DAG: BB#3
> +; CHECK-MOD-DAG: Successors according to CFG: BB#2
> +; CHECK-MOD-DAG: BB#4
> +
>
> Modified: llvm/trunk/test/CodeGen/ARM/Windows/division.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/Windows/division.ll?rev=263714&r1=263713&r2=263714&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/ARM/Windows/division.ll (original)
> +++ llvm/trunk/test/CodeGen/ARM/Windows/division.ll Thu Mar 17 09:10:49
> 2016
> @@ -7,8 +7,10 @@ entry:
>    ret i32 %div
>  }
>
> -; CHECK-LABEL: sdiv32
> -; CHECK: b __rt_sdiv
> +; CHECK-LABEL: sdiv32:
> +; CHECK: cbz r0
> +; CHECK: bl __rt_sdiv
> +; CHECK: udf.w #249
>
>  define arm_aapcs_vfpcc i32 @udiv32(i32 %divisor, i32 %divident) {
>  entry:
> @@ -17,7 +19,9 @@ entry:
>  }
>
>  ; CHECK-LABEL: udiv32:
> -; CHECK: b __rt_udiv
> +; CHECK: cbz r0
> +; CHECK: bl __rt_udiv
> +; CHECK: udf.w #249
>
>  define arm_aapcs_vfpcc i64 @sdiv64(i64 %divisor, i64 %divident) {
>  entry:
> @@ -25,8 +29,11 @@ entry:
>    ret i64 %div
>  }
>
> -; CHECK-LABEL: sdiv64
> +; CHECK-LABEL: sdiv64:
> +; CHECK: orr.w r12, r0, r1
> +; CHECK-NEXT: cbz r12
>  ; CHECK: bl __rt_sdiv64
> +; CHECK: udf.w #249
>
>  define arm_aapcs_vfpcc i64 @udiv64(i64 %divisor, i64 %divident) {
>  entry:
> @@ -35,4 +42,8 @@ entry:
>  }
>
>  ; CHECK-LABEL: udiv64:
> +; CHECK: orr.w r12, r0, r1
> +; CHECK-NEXT: cbz r12
>  ; CHECK: bl __rt_udiv64
> +; CHECK: udf.w #249
> +
>
> Removed: llvm/trunk/test/CodeGen/ARM/Windows/overflow.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/Windows/overflow.ll?rev=263713&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/ARM/Windows/overflow.ll (original)
> +++ llvm/trunk/test/CodeGen/ARM/Windows/overflow.ll (removed)
> @@ -1,77 +0,0 @@
> -; RUN: llc -mtriple thumbv7-windows-gnu -filetype asm -o - %s
> -
> -define i32 @divsoverflow32(i32 %a, i32 %b) {
> -  %1 = alloca i32, align 4
> -  %2 = alloca i32, align 4
> -  %3 = load i32, i32* %1, align 4
> -  %4 = load i32, i32* %2, align 4
> -  %5 = sub nsw i32 0, %4
> -  %6 = sdiv i32 -2147483647, %3
> -  %7 = icmp sgt i32 %5, %6
> -  br i1 %7, label %8, label %9
> -  call void (...) @abort_simpl32()
> -  unreachable
> -  %10 = load i32, i32* %1, align 4
> -  %11 = load i32, i32* %2, align 4
> -  %12 = mul nsw i32 %10, %11
> -  ret i32 %12
> -}
> -
> -declare void @abort_simpl32(...)
> -
> -define i64 @divsoverflow64(i64 %a, i64 %b) {
> -  %1 = alloca i64, align 8
> -  %2 = alloca i64, align 8
> -  %3 = load i64, i64* %1, align 8
> -  %4 = load i64, i64* %2, align 8
> -  %5 = sub nsw i64 0, %4
> -  %6 = sdiv i64 -9223372036854775808, %3
> -  %7 = icmp sgt i64 %5, %6
> -  br i1 %7, label %8, label %9
> -  call void (...) @abort_simpl64()
> -  unreachable
> -  %10 = load i64, i64* %1, align 8
> -  %11 = load i64, i64* %2, align 8
> -  %12 = mul nsw i64 %10, %11
> -  ret i64 %12
> -}
> -
> -declare void @abort_simpl64(...)
> -
> -define i32 @divuoverflow32(i32 %a, i32 %b) {
> -  %1 = alloca i32, align 4
> -  %2 = alloca i32, align 4
> -  %3 = load i32, i32* %1, align 4
> -  %4 = load i32, i32* %2, align 4
> -  %5 = sub nsw i32 0, %4
> -  %6 = udiv i32 4294967296, %3
> -  %7 = icmp sgt i32 %5, %6
> -  br i1 %7, label %8, label %9
> -  call void (...) @abort_uimpl32()
> -  unreachable
> -  %10 = load i32, i32* %1, align 4
> -  %11 = load i32, i32* %2, align 4
> -  %12 = mul nsw i32 %10, %11
> -  ret i32 %12
> -}
> -
> -declare void @abort_uimpl32(...)
> -
> -define i64 @divuoverflow64(i64 %a, i64 %b) {
> -  %1 = alloca i64, align 8
> -  %2 = alloca i64, align 8
> -  %3 = load i64, i64* %1, align 8
> -  %4 = load i64, i64* %2, align 8
> -  %5 = sub nsw i64 0, %4
> -  %6 = udiv i64 18446744073709551616, %3
> -  %7 = icmp sgt i64 %5, %6
> -  br i1 %7, label %8, label %9
> -  call void (...) @abort_uimpl64()
> -  unreachable
> -  %10 = load i64, i64* %1, align 8
> -  %11 = load i64, i64* %2, align 8
> -  %12 = mul nsw i64 %10, %11
> -  ret i64 %12
> -}
> -
> -declare void @abort_uimpl64(...)
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>



-- 
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160317/5b98b741/attachment.html>


More information about the llvm-commits mailing list