[PATCH] D24076: [ARM] Use __rt_div functions for DIVREM on Windows

[Martin Storsjö via llvm-commits]

mstorsjo added a comment.

I tested to extend this by removing the current custom code for SDIV/UDIV for windows, and divmod-eabi.ll now looks better (it produces two calls to __rt_sdiv instead of three).

Previously, all divisions included a check for whether the divisor is zero (with an udf.w #349 instruction in case of a division by zero), while when I made it work in the same way as __aeabi_idivmod I don't get those.

I'll upload the updated patch for further comments and clarifications.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:805
@@ -803,9 +804,3 @@
 
-    setLibcallName(RTLIB::SDIVREM_I8,  "__aeabi_idivmod");
-    setLibcallName(RTLIB::SDIVREM_I16, "__aeabi_idivmod");
-    setLibcallName(RTLIB::SDIVREM_I32, "__aeabi_idivmod");
-    setLibcallName(RTLIB::SDIVREM_I64, "__aeabi_ldivmod");
-    setLibcallName(RTLIB::UDIVREM_I8,  "__aeabi_uidivmod");
-    setLibcallName(RTLIB::UDIVREM_I16, "__aeabi_uidivmod");
-    setLibcallName(RTLIB::UDIVREM_I32, "__aeabi_uidivmod");
-    setLibcallName(RTLIB::UDIVREM_I64, "__aeabi_uldivmod");
+    if (Subtarget->isTargetWindows()) {
+      setLibcallName(RTLIB::SDIVREM_I8,  "__rt_sdiv");
----------------
rengolin wrote:
> I'd have put this in a separate if block, since the function names are *all* different.
I chose putting it here since it shares the code for all the setLibcallCallingConv below anyway, instead of duplicating all the 15 lines of shared code above and below. But you'd still prefer me to duplicate all of it including setOperationAction(), HasStandaloneRem and setLibcallCallingConv?

================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:12090
@@ -12077,2 +12089,3 @@
   for (unsigned i = 0, e = N->getNumOperands(); i != e; ++i) {
-    EVT ArgVT = N->getOperand(i).getValueType();
+    unsigned src = Subtarget->isTargetWindows() ? e - 1 - i : i;
+    EVT ArgVT = N->getOperand(src).getValueType();
----------------
rengolin wrote:
> This is not about windows or not, it's about rt_div instead of eabi_divmod. Please, make it dependent on the RT type (not OS).
> 
> Also, this induction fiddling is confusing. The easiest thing to do is to swap the two first arguments before the loop. It's clear and precise.
How do I find the RT type here - I didn't find it within ARMSubtarget on a quick look at least.

And how would I go about to swap the two arguments? N is a const SDNode here - is it ok to make it mutable and swap the arguments there?


https://reviews.llvm.org/D24076