Skip to content

Commit

Permalink
ARM: Revert SVN r253865, 254158, fix windows division
Browse files Browse the repository at this point in the history
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.

llvm-svn: 263714
  • Loading branch information
compnerd committed Mar 17, 2016
1 parent 9743992 commit 071a099
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 88 deletions.
25 changes: 18 additions & 7 deletions llvm/lib/Target/ARM/ARMISelLowering.cpp
Expand Up @@ -390,10 +390,6 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
{ 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) {
Expand Down Expand Up @@ -781,6 +777,14 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
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)
Expand Down Expand Up @@ -7010,8 +7014,14 @@ SDValue ARMTargetLowering::LowerOperation(SDValue Op, SelectionDAG &DAG) const {
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:
Expand Down Expand Up @@ -8008,7 +8018,7 @@ ARMTargetLowering::EmitLowered__dbzchk(MachineInstr *MI,
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);
Expand All @@ -8018,6 +8028,7 @@ ARMTargetLowering::EmitLowered__dbzchk(MachineInstr *MI,
BuildMI(*MBB, MI, DL, TII->get(ARM::tCBZ))
.addReg(MI->getOperand(0).getReg())
.addMBB(TrapBB);
MBB->addSuccessor(ContBB);

MI->eraseFromParent();
return ContBB;
Expand Down
80 changes: 80 additions & 0 deletions llvm/test/CodeGen/ARM/Windows/dbzchk.ll
@@ -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);
; }

@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

19 changes: 15 additions & 4 deletions llvm/test/CodeGen/ARM/Windows/division.ll
Expand Up @@ -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:
Expand All @@ -17,16 +19,21 @@ 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:
%div = sdiv i64 %divident, %divisor
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:
Expand All @@ -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

77 changes: 0 additions & 77 deletions llvm/test/CodeGen/ARM/Windows/overflow.ll

This file was deleted.

0 comments on commit 071a099

Please sign in to comment.