[PATCH] ARM: sort out MachO target checks

Renato Golin renato.golin at linaro.org
Thu Dec 12 06:36:25 PST 2013


  Hi Tim,

  I really like this change! Thanks for doing this! I think overall is good, some comments below, mostly harmless.

  Your new tests are good, but the changed ones look very fragile in general (specific sequence, registers by name, etc). I do understand that changing them is not part of this patch, however.


================
Comment at: lib/Target/ARM/ARMAsmPrinter.cpp:151
@@ -152,1 +150,3 @@
+                                             ? MCSymbolRefExpr::VK_ARM_TARGET1
+                                             : MCSymbolRefExpr::VK_None),
                                             OutContext);
----------------
I guess this is fine until we support a third object format...

================
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.
----------------
This is a tricky change...

================
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
----------------
I'd *really* love to change this to "isTargetEABI()". Does MachO EABI also call __aeabi_ functions or not?

================
Comment at: test/CodeGen/ARM/emit-big-cst.ll:9
@@ -9,1 +8,3 @@
+; CHECK-NEXT: .long 0
+; CHECK-NEXT: .size bigCst, 16
 
----------------
This is a weird change...

================
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,
----------------
smells like this is going to break next time...


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



More information about the llvm-commits mailing list