[PATCH] ARM: sort out MachO target checks

Tim Northover t.p.northover at gmail.com
Thu Dec 12 12:37:23 PST 2013


  Hi Renato,

  Thanks for taking a look. I've got a few comments on yours, though they may or may not address the actual issues...


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:261
@@ -260,3 +260,3 @@
 
-  if (Subtarget->isAAPCS_ABI() && !Subtarget->isTargetDarwin()) {
+  if (Subtarget->isAAPCS_ABI() && !Subtarget->isTargetMachO()) {
     // Double-precision floating-point arithmetic helper functions
----------------
Renato Golin wrote:
> I'd *really* love to change this to "isTargetEABI()". Does MachO EABI also call __aeabi_ functions or not?
Sounds like a very good idea longer term. EABI seems to be the only environment that does use them. But I'd rather not do that in this patch because until I get the front-end sorted compilation will still be using -darwin-eabi which doesn't want them.

================
Comment at: lib/Target/ARM/ARMSubtarget.cpp:194
@@ -195,1 +193,3 @@
+  if (TargetTriple.getEnvironment() != Triple::GNU &&
+      (!isTargetDarwin() || isMClass()))
     // FIXME: We might want to separate AAPCS and EABI. Some systems, e.g.
----------------
Renato Golin wrote:
> This is a tricky change...
Another one that can be simplified when the front-end is updated. We can then drop the M-class exception.

================
Comment at: test/CodeGen/ARM/emit-big-cst.ll:9
@@ -9,1 +8,3 @@
+; CHECK-NEXT: .long 0
+; CHECK-NEXT: .size bigCst, 16
 
----------------
Renato Golin wrote:
> This is a weird change...
It comes from the move APCS/AAPCS change you commented on above. That one makes AAPCS the default for "thumbv7-unknown-unknown" which changes the alignment constraints of big types (an i82 in this case).

================
Comment at: test/CodeGen/ARM/fold-stack-adjust.ll:14
@@ -13,3 +13,3 @@
 ; CHECK-LABEL: check_simple:
-; CHECK: push.w {r7, r8, r9, r10, r11, lr}
+; CHECK: push {r3, r4, r5, r6, r7, lr}
 ; CHECK-NOT: sub sp, sp,
----------------
Renato Golin wrote:
> smells like this is going to break next time...
Hopefully not. The large part of the change was due to me forcing the iOS-style 2-area prologue (to use a Thumb1 16-bit push) on MachO bare-metal targets. I'd only expect it to change in future if LLVM decides it needs more than 16 bytes of stack.


http://llvm-reviews.chandlerc.com/D2395



More information about the llvm-commits mailing list