<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"><base href="x-msg://22160/"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>Hello,</div><div><br></div><div>Come comments inline:</div><blockquote type="cite"><div><br></div><div>Added support for signed/unsigned integer division (SDIV and UDIV),</div><div>which are optional for ARMv7-A/ARMv7-R in ARM mode. By enabling</div><div>subtarget feature "hwdiv", sdiv/udiv will be emitted in ARM mode</div><div>to replace libcalls for integer division and modulo operations.</div></blockquote><div><br></div><div>Cool!</div><br><blockquote type="cite"><div><br></div><div>Note: Use LLVM integrated assembler or GNU AS 4.7. GNU AS 4.6 does not</div><div>support them.</div><div>---</div><div> lib/Target/ARM/ARMISelLowering.cpp | 6 ++++--</div><div> lib/Target/ARM/ARMInstrFormats.td | 12 ++++++++++++</div><div> lib/Target/ARM/ARMInstrInfo.td | 33 +++++++++++++++++++++++++++++++++</div><div> test/CodeGen/ARM/divhw.ll | 36 ++++++++++++++++++++++++++++++++++++</div><div> test/CodeGen/Thumb2/divhw.ll | 36 ++++++++++++++++++++++++++++++++++++</div><div> 5 files changed, 121 insertions(+), 2 deletions(-)</div><div> create mode 100755 test/CodeGen/ARM/divhw.ll</div><div> create mode 100755 test/CodeGen/Thumb2/divhw.ll</div><div><br></div><div>diff --git a/lib/Target/ARM/ARMISelLowering.cpp b/lib/Target/ARM/ARMISelLowering.cpp</div><div>index dce9246..351c558 100644</div><div>--- a/lib/Target/ARM/ARMISelLowering.cpp</div><div>+++ b/lib/Target/ARM/ARMISelLowering.cpp</div><div>@@ -636,8 +636,10 @@ ARMTargetLowering::ARMTargetLowering(TargetMachine &TM)</div><div> setOperationAction(ISD::BSWAP, MVT::i32, Expand);</div><div> </div><div> // These are expanded into libcalls.</div><div>- if (!Subtarget->hasDivide() || !Subtarget->isThumb2()) {</div><div>- // v7M has a hardware divider</div><div>+ if (!Subtarget->hasDivide() ||</div><div>+ (!Subtarget->isThumb2() && Subtarget->isMClass())) {</div><div>+ // v7M has a hardware divider in Thumb mode.</div><div>+ // v7A and v7R have optional hardware divider in ARM mode.</div><div> setOperationAction(ISD::SDIV, MVT::i32, Expand);</div><div> setOperationAction(ISD::UDIV, MVT::i32, Expand);</div><div> }</div><div>diff --git a/lib/Target/ARM/ARMInstrFormats.td b/lib/Target/ARM/ARMInstrFormats.td</div><div>index c8966fb..0a16dec 100644</div><div>--- a/lib/Target/ARM/ARMInstrFormats.td</div><div>+++ b/lib/Target/ARM/ARMInstrFormats.td</div><div>@@ -69,6 +69,7 @@ def NVExtFrm : Format<39>;</div><div> def NVMulSLFrm : Format<40>;</div><div> def NVTBLFrm : Format<41>;</div><div> def DPSoRegImmFrm : Format<42>;</div><div>+def DivFrm : Format<43>;</div></blockquote><div><br></div>Why do we need a new format?<div><br><blockquote type="cite"><div> </div><div> // Misc flags.</div><div> </div><div>@@ -735,6 +736,17 @@ class AXI4<dag oops, dag iops, IndexMode im, Format f, InstrItinClass itin,</div><div> let Inst{15-0} = regs;</div><div> }</div><div> </div><div>+</div><div>+// Signed and Unsigned division.</div><div>+class AsDiv1I<bits<3> opcod, dag oops, dag iops, InstrItinClass itin,</div><div>+ string opc, string asm, list<dag> pattern></div><div>+ : I<oops, iops, AddrModeNone, 4, IndexModeNone, DivFrm, itin,</div><div>+ opc, asm, "", pattern> {</div><div>+ let Inst{7-4} = 0b0001;</div><div>+ let Inst{22-20} = opcod;</div><div>+ let Inst{27-23} = 0b01110;</div><div>+}</div><div>+</div><div> // Unsigned multiply, multiply-accumulate instructions.</div><div> class AMul1I<bits<7> opcod, dag oops, dag iops, InstrItinClass itin,</div><div> string opc, string asm, list<dag> pattern></div><div>diff --git a/lib/Target/ARM/ARMInstrInfo.td b/lib/Target/ARM/ARMInstrInfo.td</div><div>index 20d7c1b..a2d0ef8 100644</div><div>--- a/lib/Target/ARM/ARMInstrInfo.td</div><div>+++ b/lib/Target/ARM/ARMInstrInfo.td</div><div>@@ -3389,6 +3389,39 @@ def MVNi : AsI1<0b1111, (outs GPR:$Rd), (ins so_imm:$imm), DPFrm,</div><div> def : ARMPat<(and GPR:$src, so_imm_not:$imm),</div><div> (BICri GPR:$src, so_imm_not:$imm)>;</div><div> </div><div>+</div><div>+//===----------------------------------------------------------------------===//</div><div>+// Division Instructions.</div><div>+// Signed and unsigned division, optional on v7a/r</div><div>+//</div><div>+</div><div>+class AsDiv1I32<bits<3> opcod, dag oops, dag iops, InstrItinClass itin,</div><div>+ string opc, string asm, list<dag> pattern></div><div>+ : AsDiv1I<opcod, oops, iops, itin, opc, asm, pattern> {</div><div>+ bits<4> Rd;</div><div>+ bits<4> Rm;</div><div>+ bits<4> Rn;</div><div>+ let Inst{19-16} = Rd;</div><div>+ let Inst{11-8} = Rm;</div><div>+ let Inst{3-0} = Rn;</div><div>+}</div><div>+</div></blockquote><div><br></div><div>Why have both AMul1I and AsDiv1I32? It seems they could be combined.</div><br><blockquote type="cite"><div>+def SDIV : AsDiv1I32<0b001, (outs GPRnopc:$Rd), (ins GPRnopc:$Rn, GPRnopc:$Rm),</div><div>+ IIC_iALUr, "sdiv", "\t$Rd, $Rn, $Rm",</div><div>+ [(set GPRnopc:$Rd, (sdiv GPRnopc:$Rn, GPRnopc:$Rm))]>,</div><div>+ Requires<[IsARM, HasDivide]> {</div><div>+ let Inst{15-12} = 0b1111;</div><div>+ let Unpredictable{15-12} = 0b1111;</div></blockquote><div><br></div><div>These bits are common for SDIV and UDIV, so can they be part of AMul1I rather than specified explicitly here?</div><div><br></div><div>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:</div><div><br></div><div>def UDIV : AMulI<"udiv", udiv>; </div><div><div>def UDIV : AMulI<"udiv", sdiv>; </div></div><div><br></div><br><blockquote type="cite"><div>+}</div><div>+</div><div>+def UDIV : AsDiv1I32<0b011, (outs GPRnopc:$Rd), (ins GPRnopc:$Rn, GPRnopc:$Rm),</div><div>+ IIC_iALUr, "udiv", "\t$Rd, $Rn, $Rm",</div><div>+ [(set GPRnopc:$Rd, (udiv GPRnopc:$Rn, GPRnopc:$Rm))]>,</div><div>+ Requires<[IsARM, HasDivide]> {</div><div>+ let Inst{15-12} = 0b1111;</div><div>+ let Unpredictable{15-12} = 0b1111;</div><div>+}</div><div>+</div><div> //===----------------------------------------------------------------------===//</div><div> // Multiply Instructions.</div><div> //</div><div>diff --git a/test/CodeGen/ARM/divhw.ll b/test/CodeGen/ARM/divhw.ll</div><div>new file mode 100755</div><div>index 0000000..dd21297</div><div>--- /dev/null</div><div>+++ b/test/CodeGen/ARM/divhw.ll</div><div>@@ -0,0 +1,36 @@</div><div>+; RUN: llc < %s -mtriple=armv7-none-linux-gnueabi -mattr=+hwdiv | FileCheck %s</div><div>+</div><div>+define i32 @f1(i32 %a, i32 %b) {</div><div>+entry:</div><div>+; CHECK: f1</div></blockquote><div><br></div><div>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').</div><br><blockquote type="cite"><div>+; CHECK: sdiv</div></blockquote><div><br></div><div>Please check the operands as well as the mnemonic.</div><div><br></div><br><blockquote type="cite"><div>+ %tmp1 = sdiv i32 %a, %b</div><div>+ ret i32 %tmp1</div><div>+}</div><div>+</div><div>+define i32 @f2(i32 %a, i32 %b) {</div><div>+entry:</div><div>+; CHECK: f2</div><div>+; CHECK: udiv</div><div>+ %tmp1 = udiv i32 %a, %b</div><div>+ ret i32 %tmp1</div><div>+}</div><div>+</div><div>+define i32 @f3(i32 %a, i32 %b) {</div><div>+entry:</div><div>+; CHECK: f3</div><div>+; CHECK: sdiv</div><div>+; CHECK-NEXT: mls</div><div>+ %tmp1 = srem i32 %a, %b</div><div>+ ret i32 %tmp1</div><div>+}</div><div>+</div><div>+define i32 @f4(i32 %a, i32 %b) {</div><div>+entry:</div><div>+; CHECK: f4</div><div>+; CHECK: udiv</div><div>+; CHECK-NEXT: mls</div><div>+ %tmp1 = urem i32 %a, %b</div><div>+ ret i32 %tmp1</div><div>+}</div><div>+</div><div>diff --git a/test/CodeGen/Thumb2/divhw.ll b/test/CodeGen/Thumb2/divhw.ll</div><div>new file mode 100755</div><div>index 0000000..f99efd8</div><div>--- /dev/null</div><div>+++ b/test/CodeGen/Thumb2/divhw.ll</div><div>@@ -0,0 +1,36 @@</div><div>+; RUN: llc < %s -mtriple=thumbv7-none-linux-gnueabi -mattr=+hwdiv | FileCheck %s</div><div>+</div><div>+define i32 @f1(i32 %a, i32 %b) {</div><div>+entry:</div><div>+; CHECK: f1</div><div>+; CHECK: sdiv</div><div>+ %tmp1 = sdiv i32 %a, %b</div><div>+ ret i32 %tmp1</div><div>+}</div><div>+</div><div>+define i32 @f2(i32 %a, i32 %b) {</div><div>+entry:</div><div>+; CHECK: f2</div><div>+; CHECK: udiv</div><div>+ %tmp1 = udiv i32 %a, %b</div><div>+ ret i32 %tmp1</div><div>+}</div><div>+</div><div>+define i32 @f3(i32 %a, i32 %b) {</div><div>+entry:</div><div>+; CHECK: f3</div><div>+; CHECK: sdiv</div><div>+; CHECK-NEXT: mls</div><div>+ %tmp1 = srem i32 %a, %b</div><div>+ ret i32 %tmp1</div><div>+}</div><div>+</div><div>+define i32 @f4(i32 %a, i32 %b) {</div><div>+entry:</div><div>+; CHECK: f4</div><div>+; CHECK: udiv</div><div>+; CHECK-NEXT: mls</div><div>+ %tmp1 = urem i32 %a, %b</div><div>+ ret i32 %tmp1</div><div>+}</div><div>+</div><div>-- </div><div>1.7.8.3</div><div><br></div></blockquote><div><br></div><div>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.</div><div><br></div><div><br></div><br><div><div>On Sep 27, 2012, at 12:08 PM, Weiming Zhao <<a href="mailto:weimingz@codeaurora.org">weimingz@codeaurora.org</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div lang="EN-US" link="blue" vlink="purple" style="font-family: Helvetica; font-size: medium; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; "><div class="WordSection1" style="page: WordSection1; "><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; ">Hi,<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; "><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; ">This patch fixes PR 13936 (<a href="http://llvm.org/bugs/show_bug.cgi?id=13936" style="color: purple; text-decoration: underline; ">http://llvm.org/bugs/show_bug.cgi?id=13936</a>)<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; "><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; ">According to ARM manual [1], ARM mode sdiv/udiv are optional in ARMv7-A and ARMv7-R on ARM mode.<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; ">So, this patch adds the ARM mode sdiv/udiv definition and encoding into<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; ">existing td files and use the existing target-feature: hwdiv flag to guard it.<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; ">So, we can avoid calls to RT lib if the hardware supports hw div.<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; "><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; ">Please review the attached patch.<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; "><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; ">Thanks,<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; ">Weiming<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; "><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; ">[1] Architecture Reference Manual ARMv7-A and ARMv7-R edition,<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; "><a href="https://silver.arm.com/download/ARM_and_AMBA_Architecture/AR570-DA-70000-r0p0-00rel1/DDI0406C_b_arm_architecture_reference_manual.pdf" style="color: purple; text-decoration: underline; ">https://silver.arm.com/download/ARM_and_AMBA_Architecture/AR570-DA-70000-r0p0-00rel1/DDI0406C_b_arm_architecture_reference_manual.pdf</a><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; ">, Page A8-600<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; "><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; "><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; ">Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; "><o:p> </o:p></div></div><span><0001-PR-13936-Added-hardware-integer-division-support-for.patch></span>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline; ">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" style="color: purple; text-decoration: underline; ">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></div></blockquote></div><br></div></body></html>