[llvm-commits] Fix PR13936: Add sdiv/udiv in ARM mode for targets that support them
Weiming Zhao
weimingz at codeaurora.org
Mon Oct 1 10:45:15 PDT 2012
Hi Wilson,
Yes, hwdiv-arm does the job, cool!
Thanks!
Weiming
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation
From: Bob Wilson [mailto:bob.wilson at apple.com]
Sent: Saturday, September 29, 2012 5:27 PM
To: weimingz at codeaurora.org
Cc: 'Jim Grosbach'; llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] Fix PR13936: Add sdiv/udiv in ARM mode for
targets that support them
I think most, if not all of this, is already handled by svn r164899. Can
you check if we're still missing anything?
On Sep 27, 2012, at 6:37 PM, Weiming Zhao <weimingz at codeaurora.org> wrote:
Hi Jim,
Thanks for your review and suggestions.
I addressed the issues that you mentioned.
I'm reusing the encoding of AMul, but I can't completely remove the ADivI32
class because the coding format of them are defined differently on Bit 7-4,
12-15 and bit 20.
Please help to review the attached patch.
Thanks,
Weiming
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation
From: Jim Grosbach [mailto:grosbach@ <http://apple.com> apple.com]
Sent: Thursday, September 27, 2012 1:52 PM
To: <mailto:weimingz at codeaurora.org> weimingz at codeaurora.org
Cc: <mailto:llvm-commits at cs.uiuc.edu> llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] Fix PR13936: Add sdiv/udiv in ARM mode for
targets that support them
Hello,
Come comments inline:
Added support for signed/unsigned integer division (SDIV and UDIV),
which are optional for ARMv7-A/ARMv7-R in ARM mode. By enabling
subtarget feature "hwdiv", sdiv/udiv will be emitted in ARM mode
to replace libcalls for integer division and modulo operations.
Cool!
Note: Use LLVM integrated assembler or GNU AS 4.7. GNU AS 4.6 does not
support them.
---
lib/Target/ARM/ARMISelLowering.cpp | 6 ++++--
lib/Target/ARM/ARMInstrFormats.td | 12 ++++++++++++
lib/Target/ARM/ARMInstrInfo.td | 33 +++++++++++++++++++++++++++++++++
test/CodeGen/ARM/divhw.ll | 36
++++++++++++++++++++++++++++++++++++
test/CodeGen/Thumb2/divhw.ll | 36
++++++++++++++++++++++++++++++++++++
5 files changed, 121 insertions(+), 2 deletions(-)
create mode 100755 test/CodeGen/ARM/divhw.ll
create mode 100755 test/CodeGen/Thumb2/divhw.ll
diff --git a/lib/Target/ARM/ARMISelLowering.cpp
b/lib/Target/ARM/ARMISelLowering.cpp
index dce9246..351c558 100644
--- a/lib/Target/ARM/ARMISelLowering.cpp
+++ b/lib/Target/ARM/ARMISelLowering.cpp
@@ -636,8 +636,10 @@ ARMTargetLowering::ARMTargetLowering(TargetMachine &TM)
setOperationAction(ISD::BSWAP, MVT::i32, Expand);
// These are expanded into libcalls.
- if (!Subtarget->hasDivide() || !Subtarget->isThumb2()) {
- // v7M has a hardware divider
+ if (!Subtarget->hasDivide() ||
+ (!Subtarget->isThumb2() && Subtarget->isMClass())) {
+ // v7M has a hardware divider in Thumb mode.
+ // v7A and v7R have optional hardware divider in ARM mode.
setOperationAction(ISD::SDIV, MVT::i32, Expand);
setOperationAction(ISD::UDIV, MVT::i32, Expand);
}
diff --git a/lib/Target/ARM/ARMInstrFormats.td
b/lib/Target/ARM/ARMInstrFormats.td
index c8966fb..0a16dec 100644
--- a/lib/Target/ARM/ARMInstrFormats.td
+++ b/lib/Target/ARM/ARMInstrFormats.td
@@ -69,6 +69,7 @@ def NVExtFrm : Format<39>;
def NVMulSLFrm : Format<40>;
def NVTBLFrm : Format<41>;
def DPSoRegImmFrm : Format<42>;
+def DivFrm : Format<43>;
Why do we need a new format?
// Misc flags.
@@ -735,6 +736,17 @@ class AXI4<dag oops, dag iops, IndexMode im, Format f,
InstrItinClass itin,
let Inst{15-0} = regs;
}
+
+// Signed and Unsigned division.
+class AsDiv1I<bits<3> opcod, dag oops, dag iops, InstrItinClass itin,
+ string opc, string asm, list<dag> pattern>
+ : I<oops, iops, AddrModeNone, 4, IndexModeNone, DivFrm, itin,
+ opc, asm, "", pattern> {
+ let Inst{7-4} = 0b0001;
+ let Inst{22-20} = opcod;
+ let Inst{27-23} = 0b01110;
+}
+
// Unsigned multiply, multiply-accumulate instructions.
class AMul1I<bits<7> opcod, dag oops, dag iops, InstrItinClass itin,
string opc, string asm, list<dag> pattern>
diff --git a/lib/Target/ARM/ARMInstrInfo.td b/lib/Target/ARM/ARMInstrInfo.td
index 20d7c1b..a2d0ef8 100644
--- a/lib/Target/ARM/ARMInstrInfo.td
+++ b/lib/Target/ARM/ARMInstrInfo.td
@@ -3389,6 +3389,39 @@ def MVNi : AsI1<0b1111, (outs GPR:$Rd), (ins
so_imm:$imm), DPFrm,
def : ARMPat<(and GPR:$src, so_imm_not:$imm),
(BICri GPR:$src, so_imm_not:$imm)>;
+
+//===----------------------------------------------------------------------
===//
+// Division Instructions.
+// Signed and unsigned division, optional on v7a/r
+//
+
+class AsDiv1I32<bits<3> opcod, dag oops, dag iops, InstrItinClass itin,
+ string opc, string asm, list<dag> pattern>
+ : AsDiv1I<opcod, oops, iops, itin, opc, asm, pattern> {
+ bits<4> Rd;
+ bits<4> Rm;
+ bits<4> Rn;
+ let Inst{19-16} = Rd;
+ let Inst{11-8} = Rm;
+ let Inst{3-0} = Rn;
+}
+
Why have both AMul1I and AsDiv1I32? It seems they could be combined.
+def SDIV : AsDiv1I32<0b001, (outs GPRnopc:$Rd), (ins GPRnopc:$Rn,
GPRnopc:$Rm),
+ IIC_iALUr, "sdiv", "\t$Rd, $Rn, $Rm",
+ [(set GPRnopc:$Rd, (sdiv GPRnopc:$Rn, GPRnopc:$Rm))]>,
+ Requires<[IsARM, HasDivide]> {
+ let Inst{15-12} = 0b1111;
+ let Unpredictable{15-12} = 0b1111;
These bits are common for SDIV and UDIV, so can they be part of AMul1I
rather than specified explicitly here?
Likewise, the (ins) and (outs), format, itinerary, and asm operand string
for SDIV and UDIV are identical, so can be shared in the format. We only
need to parameterize the parts that actually vary between the two. In this
case, that's the mnemonic and the operator for the pattern. These
definitions can look like:
def UDIV : AMulI<"udiv", udiv>;
def UDIV : AMulI<"udiv", sdiv>;
+}
+
+def UDIV : AsDiv1I32<0b011, (outs GPRnopc:$Rd), (ins GPRnopc:$Rn,
GPRnopc:$Rm),
+ IIC_iALUr, "udiv", "\t$Rd, $Rn, $Rm",
+ [(set GPRnopc:$Rd, (udiv GPRnopc:$Rn, GPRnopc:$Rm))]>,
+ Requires<[IsARM, HasDivide]> {
+ let Inst{15-12} = 0b1111;
+ let Unpredictable{15-12} = 0b1111;
+}
+
//===----------------------------------------------------------------------=
==//
// Multiply Instructions.
//
diff --git a/test/CodeGen/ARM/divhw.ll b/test/CodeGen/ARM/divhw.ll
new file mode 100755
index 0000000..dd21297
--- /dev/null
+++ b/test/CodeGen/ARM/divhw.ll
@@ -0,0 +1,36 @@
+; RUN: llc < %s -mtriple=armv7-none-linux-gnueabi -mattr=+hwdiv |
FileCheck %s
+
+define i32 @f1(i32 %a, i32 %b) {
+entry:
+; CHECK: f1
There should be a ':' following the function name on the check line to make
sure that's what we find (not some other reference to 'f1').
+; CHECK: sdiv
Please check the operands as well as the mnemonic.
+ %tmp1 = sdiv i32 %a, %b
+ ret i32 %tmp1
+}
+
+define i32 @f2(i32 %a, i32 %b) {
+entry:
+; CHECK: f2
+; CHECK: udiv
+ %tmp1 = udiv i32 %a, %b
+ ret i32 %tmp1
+}
+
+define i32 @f3(i32 %a, i32 %b) {
+entry:
+; CHECK: f3
+; CHECK: sdiv
+; CHECK-NEXT: mls
+ %tmp1 = srem i32 %a, %b
+ ret i32 %tmp1
+}
+
+define i32 @f4(i32 %a, i32 %b) {
+entry:
+; CHECK: f4
+; CHECK: udiv
+; CHECK-NEXT: mls
+ %tmp1 = urem i32 %a, %b
+ ret i32 %tmp1
+}
+
diff --git a/test/CodeGen/Thumb2/divhw.ll b/test/CodeGen/Thumb2/divhw.ll
new file mode 100755
index 0000000..f99efd8
--- /dev/null
+++ b/test/CodeGen/Thumb2/divhw.ll
@@ -0,0 +1,36 @@
+; RUN: llc < %s -mtriple=thumbv7-none-linux-gnueabi -mattr=+hwdiv |
FileCheck %s
+
+define i32 @f1(i32 %a, i32 %b) {
+entry:
+; CHECK: f1
+; CHECK: sdiv
+ %tmp1 = sdiv i32 %a, %b
+ ret i32 %tmp1
+}
+
+define i32 @f2(i32 %a, i32 %b) {
+entry:
+; CHECK: f2
+; CHECK: udiv
+ %tmp1 = udiv i32 %a, %b
+ ret i32 %tmp1
+}
+
+define i32 @f3(i32 %a, i32 %b) {
+entry:
+; CHECK: f3
+; CHECK: sdiv
+; CHECK-NEXT: mls
+ %tmp1 = srem i32 %a, %b
+ ret i32 %tmp1
+}
+
+define i32 @f4(i32 %a, i32 %b) {
+entry:
+; CHECK: f4
+; CHECK: udiv
+; CHECK-NEXT: mls
+ %tmp1 = urem i32 %a, %b
+ ret i32 %tmp1
+}
+
--
1.7.8.3
There need to be encoding tests in the MC/ARM directory for these
instructions so we test the assembler support for them, not just isel.
On Sep 27, 2012, at 12:08 PM, Weiming Zhao <
<mailto:weimingz at codeaurora.org> weimingz at codeaurora.org> wrote:
Hi,
This patch fixes PR 13936 ( <http://llvm.org/bugs/show_bug.cgi?id=13936>
http://llvm.org/bugs/show_bug.cgi?id=13936)
According to ARM manual [1], ARM mode sdiv/udiv are optional in ARMv7-A and
ARMv7-R on ARM mode.
So, this patch adds the ARM mode sdiv/udiv definition and encoding into
existing td files and use the existing target-feature: hwdiv flag to guard
it.
So, we can avoid calls to RT lib if the hardware supports hw div.
Please review the attached patch.
Thanks,
Weiming
[1] Architecture Reference Manual ARMv7-A and ARMv7-R edition,
<https://silver.arm.com/download/ARM_and_AMBA_Architecture/AR570-DA-70000-r0
p0-00rel1/DDI0406C_b_arm_architecture_reference_manual.pdf>
https://silver.arm.com/download/ARM_and_AMBA_Architecture/AR570-DA-70000-r0p
0-00rel1/DDI0406C_b_arm_architecture_reference_manual.pdf
, Page A8-600
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation
<0001-PR-13936-Added-hardware-integer-division-support-for.patch>___________
____________________________________
llvm-commits mailing list
<mailto:llvm-commits at cs.uiuc.edu> llvm-commits at cs.uiuc.edu
<http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
<0002-PR-13936-Added-hardware-integer-division-support-for.patch>___________
____________________________________
llvm-commits mailing list
<mailto:llvm-commits at cs.uiuc.edu> llvm-commits at cs.uiuc.edu
<http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121001/4f676c9d/attachment.html>
More information about the llvm-commits
mailing list