[PATCH] D24133: [ARM] Lower UDIV+UREM to UDIV+MLS (and the same for SREM)

[Renato Golin via llvm-commits]

rengolin added a comment.

So, I think that's a worthy optimisation, but I worry about the multi-cycle that the selection dag takes to get there.

The validation mechanism keeps cycling between divs and mods, merging and extending them until, quite by accident, things fall into shape. For instance, another patch to add "rt_div" reports that library call being called three times.

One thing we don't do, for example, is to merge divs into divmods that only need the mods, so you'll end up with a call to idiv + idivmod. In here, you'll end up with DIV+DIV+MUL(MOD) with the two first divs being identical.

The only comfort is that pure mods need the div anyway, so in the simple case, it's ok.

cheers,
--renato


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:12109
@@ +12108,3 @@
+  if (Subtarget->hasDivide()) {
+    unsigned DivOpcode = isSigned ? ISD::SDIV : ISD::UDIV;
+    const SDValue Dividend = Op->getOperand(0);
----------------
compnerd wrote:
> Why not inline this into the `Div`?
Because the merge of divs+mods happens elsewhere. I believe divs are already covered by instructions where possible, but not mods. However, mods fallback here, so this is the right place, I think.

================
Comment at: test/CodeGen/ARM/urem-opt-size.ll:43
@@ -36,1 +42,3 @@
+; V7M: mls
+; V7M-NOT: __aeabi_uidivmod
   %call = tail call i32 bitcast (i32 (...)* @GetValue to i32 ()*)()
----------------
Please, add tests for signed/unsigned, mod and div+mod, div+div+mod and see if they merge (if not, leave a FIXME comment). Some of those tests will probably need to be in divmod-eabi.ll.

Also, please make sure to include regex for the registers, to make sure that the result from udiv gets passed correctly to the mul + add.


https://reviews.llvm.org/D24133