[llvm] r263714 - ARM: Revert SVN r253865, 254158, fix windows division
Tom Stellard via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 1 12:22:49 PDT 2016
On Fri, Mar 18, 2016 at 12:29:33PM +0100, Renato Golin wrote:
> Hi Saleem,
>
> Makes sense. The change is not disruptive and fixes a clear bug on a
> specific target. I've included it in my list for 3.8.1 backports. When
> we start the cycle, I'll backport it.
>
You can go ahead and backport this now. If you want to also backport
r263118 and r263123, that would be great.
-Tom
> cheers,
> --renato
>
> On 18 March 2016 at 02:16, Saleem Abdulrasool via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > 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
> >
> > _______________________________________________
> > 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