[ARM] Honour ABI for rem under -O0 for EABI, GNUEABI, Android and Musl

[rovka]

Summary

At higher optimization levels, we generate the libcall for DIVREM_Ix, which is
fine: aeabi_{u|i}divmod. At -O0 we generate the one for REM_Ix, which is the
default {u}mod{q|h|s|d}i3.

This commit makes sure that we don't generate REM_Ix calls for ABIs that
don't support them (i.e. where we need to use DIVREM_Ix instead). This is
achieved by bailing out of FastISel, which can't handle non-double multi-reg
returns, and letting the legalization infrastructure expand the REM_Ix calls.

It also updates the divmod-eabi.ll test to run under -O0 as well, and adds some
Windows checks to it to make sure we don't break things for it.

Fixes PR27068

[rovka]

I'm not sure this is the most principled way to test this. I think unit tests for getLibcallName might be a better idea, but I notice we don't have an awful lot of unit tests. Any suggestions?

[rengolin]

test/CodeGen/ARM/divmod-eabi.ll

20

Removing the ABI registers will make the test a lot weaker and PR28164 tells me that this can already be broken in some cases.

Can you expand on what happens to the registers and why the test fails? My gut feeling is that a CHECK-DAG could fix the problem, but worst case scenario, we'll need two sets of CHECKs for O0 and O1.

[rovka]

I'm not sure this is the most principled way to test this. I think unit tests for getLibcallName might be a better idea, but I notice we don't have an awful lot of unit tests. Any suggestions?

test/CodeGen/ARM/divmod-eabi.ll

20

I think it's just some spills getting in the way, I'll try CHECK-DAG. Thanks.

[rovka]

Changed the test to check the registers too.

For the first 2 run lines (eabi, eabihf) i got away by adding -optimize-regalloc, but for the other lines I wasn't so lucky. It seems androideabi, gnueabi and musleabi all fail to merge div and divmod into a single call at O0, so I had to add another set of checks for that. I also had to add another set of checks for Darwin, because the scheduling is a bit different at O0 so instead of an add right after one of the libcalls we get a mov and then the corresponding add at the end of the function (I might be able to fiddle with some llc scheduling options to get rid of this...).

I think there's a problem with the androideabi, gnueabi and musleabi tests, which seem to be using r0 instead of r1 to get the remainder from idivmod (either that or it's just Friday evening and I'm being clumsy).

Please let me know if this looks like a good approach for the test and I'll update the rest of it (currently only the first function in the test is updated for the new sets of checks).

[compnerd]

test/CodeGen/ARM/divmod-eabi.ll

7

I think naming the prefix something more pertinent to what is being tested is helpful. How about EABI-RAOPT ?

16

Please add a test for non-EABI targets as do not get the EABI library calls.. How about something like thumbv7-windows?

43

Am I mistaken or couldnt these cases be collapsed? The idea is that on darwin targets, the calls are identical across the two? The math would be different, but you could have a single check for the calls in both cases.

[rovka]

This patch is wrong, I need to work on it some more. Thanks for the comments, I'll incorporate them into the next version of the patch.

test/CodeGen/ARM/divmod-eabi.ll

48

+ ; DARWIN-O0: __modsi3

[rovka]

• Collapsed common checks between O0 and default
• Added Windows checks (would be great if someone could double-check these, the Microsoft docs that I've found are pretty vague).
• Fixed the patch to use the right register for the remainder. For this I had to basically bail out of FastISel, because it doesn't handle non-double multi-reg returns. I hope this hammer isn't too big for this problem, but getting FastISel to do the right thing here seemed like quite some work.

[rengolin]

Much simpler, great tests. Thanks! LGTM.

lib/Target/ARM/ARMISelLowering.cpp

if (Subtarget->isTargetAEABI() || Subtarget->isTargetAndroid() ||
     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");
 
+    setLibcallName(RTLIB::SREM_I8,  "__aeabi_idivmod");
+    setLibcallName(RTLIB::SREM_I16, "__aeabi_idivmod");
+    setLibcallName(RTLIB::SREM_I32, "__aeabi_idivmod");
+    setLibcallName(RTLIB::SREM_I64, "__aeabi_idivmod");
+    setLibcallName(RTLIB::UREM_I8,  "__aeabi_uidivmod");
+    setLibcallName(RTLIB::UREM_I16, "__aeabi_uidivmod");
+    setLibcallName(RTLIB::UREM_I32, "__aeabi_uidivmod");
+    setLibcallName(RTLIB::UREM_I64, "__aeabi_uidivmod");
+
     setLibcallCallingConv(RTLIB::SDIVREM_I8, CallingConv::ARM_AAPCS);
     setLibcallCallingConv(RTLIB::SDIVREM_I16, CallingConv::ARM_AAPCS);
     setLibcallCallingConv(RTLIB::SDIVREM_I32, CallingConv::ARM_AAPCS);
     setLibcallCallingConv(RTLIB::SDIVREM_I64, CallingConv::ARM_AAPCS);
     setLibcallCallingConv(RTLIB::UDIVREM_I8, CallingConv::ARM_AAPCS);
     setLibcallCallingConv(RTLIB::UDIVREM_I16, CallingConv::ARM_AAPCS);
     setLibcallCallingConv(RTLIB::UDIVREM_I32, CallingConv::ARM_AAPCS);
     setLibcallCallingConv(RTLIB::UDIVREM_I64, CallingConv::ARM_AAPCS);

test/CodeGen/ARM/divmod-eabi.ll

 ; RUN: llc -mtriple armv7-none-eabi %s -o - | FileCheck %s --check-prefix=EABI
+; RUN: llc -mtriple armv7-none-eabi %s -o - -O0 -optimize-regalloc | FileCheck %s --check-prefix=EABI
 ; RUN: llc -mtriple armv7-none-eabihf %s -o - | FileCheck %s --check-prefix=EABI
+; RUN: llc -mtriple armv7-none-eabihf %s -o - -O0 -optimize-regalloc | FileCheck %s --check-prefix=EABI
 ; All "eabi" (Bare, GNU and Android) must lower SREM/UREM to __aeabi_{u,i}divmod
 ; RUN: llc -mtriple armv7-linux-androideabi %s -o - | FileCheck %s --check-prefix=EABI
+; RUN: llc -mtriple armv7-linux-androideabi %s -o - -O0 -optimize-regalloc | FileCheck %s --check-prefix=EABI-2CALLS
 ; RUN: llc -mtriple armv7-linux-gnueabi %s -o - | FileCheck %s --check-prefix=EABI
+; RUN: llc -mtriple armv7-linux-gnueabi %s -o - -O0 -optimize-regalloc | FileCheck %s --check-prefix=EABI-2CALLS
 ; RUN: llc -mtriple armv7-linux-musleabi %s -o - | FileCheck %s --check-prefix=EABI
+; RUN: llc -mtriple armv7-linux-musleabi %s -o - -O0 -optimize-regalloc | FileCheck %s --check-prefix=EABI-2CALLS
 ; RUN: llc -mtriple armv7-apple-darwin %s -o - | FileCheck %s --check-prefix=DARWIN
+; RUN: llc -mtriple armv7-apple-darwin %s -o - -O0 -optimize-regalloc | FileCheck %s --check-prefix=DARWIN-O0
 ; FIXME: long-term, we will use "-apple-macho" and won't need this exception:
 ; RUN: llc -mtriple armv7-apple-darwin-eabi %s -o - | FileCheck %s --check-prefix=DARWIN
+; RUN: llc -mtriple armv7-apple-darwin-eabi %s -o - -O0 -optimize-regalloc | FileCheck %s --check-prefix=DARWIN-O0
 
 define signext i16 @f16(i16 signext %a, i16 signext %b) {
 ; EABI-LABEL: f16:
+; EABI-2CALLS-LABEL: f16:
 ; DARWIN-LABEL: f16:
+; DARWIN-O0-LABEL: f16:
 entry:
   %conv = sext i16 %a to i32
   %conv1 = sext i16 %b to i32
   %div = sdiv i32 %conv, %conv1
   %rem = srem i32 %conv, %conv1
 ; EABI: __aeabi_idivmod
 ; EABI: mov [[div:r[0-9]+]], r0
 ; EABI: mov [[rem:r[0-9]+]], r1
+; EABI-2CALLS: __aeabi_idiv
+; EABI-2CALLS: mov [[div:r[0-9]+]], r0
+; EABI-2CALLS: __aeabi_idivmod
+; TODO: Shouldn't the remainder be in r1? Is this another bug or am I missing something?
+; EABI-2CALLS: mov [[rem:r[0-9]+]], r0
 ; DARWIN: ___divsi3
 ; DARWIN: mov [[sum:r[0-9]+]], r0
 ; DARWIN: __modsi3
 ; DARWIN: add [[sum]]{{.*}}r0
+; DARWIN-O0: ___divsi3
+; DARWIN-O0: mov [[div:r[0-9]+]], r0
+; DARWIN-O0: __modsi3
+; DARWIN-O0: mov [[rem:r[0-9]+]], r0
   %rem8 = srem i32 %conv1, %conv
 ; EABI: __aeabi_idivmod
+; EABI-2CALLS: __aeabi_idivmod
 ; DARWIN: __modsi3
   %add = add nsw i32 %rem, %div
   %add13 = add nsw i32 %add, %rem8
   %conv14 = trunc i32 %add13 to i16
 ; EABI: add r0{{.*}}r1
 ; EABI: sxth r0, r0
+; EABI-2CALLS: add [[sum:r[0-9]+]], [[rem]], [[div]]
+; EABI-2CALLS: add [[res:r[0-9]+]], [[sum]], r0
+; EABI-2CALLS: sxth r0, [[res]]
 ; DARWIN: add r0{{.*}}[[sum]]
 ; DARWIN: sxth r0, r0
+; DARWIN-O0: add [[sum:r[0-9]+]], [[rem]], [[div]]
+; DARWIN-O0: add [[res:r[0-9]+]], [[sum]], r0
+; DARWIN-O0: sxth r0, [[res]]
   ret i16 %conv14
 }
 
 define i32 @f32(i32 %a, i32 %b) {
 ; EABI-LABEL: f32:
 ; DARWIN-LABEL: f32:
 entry:
   %div = sdiv i32 %a, %b