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

Renato Golin via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 18 04:29:33 PDT 2016


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.

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