[builtins][ARM] Replace call_apsr.S with inline asm

[rprichard]

Summary

The %arm_call_apsr expansion doesn't work when config.clang is a clang
driver defaulting to a non-ARM arch. Rather than fix it, replace
call_apsr.S with inline asm in call_apsr.h, which also resolves the
FIXME added in D31259.

Maybe the __attribute__((noinline,pcs("aapcs"))) attributes are
unnecessary on the static functions, but I was unsure what liberty the
compiler had to insert instructions that modified the condition codes,
so it seemed helpful.

[rprichard]

I needed this change to run the builtins' unit tests in an arm32 NDK configuration, but I'm not very confident in it. Without the change, the tests failed because the arm -march flag didn't work with clang's default host architecture (e.g. x86_64).

[compnerd]

I dont think that there is a guarantee that target_cflags will include -target armv7-unknown-linux-androideabi. Can we explicitly set the target triple instead?

[rprichard]

@jmgao added the call_apsr functions in D12089. At that point, I think the builtins only had a test bash script for testing. D30802 and D31259 added lit support, and the second one added the lit.cfg file setting %arm_call_apsr.

It seems that lit.cfg would have only worked if config.clang's default arch was ARM.

AFAICT, target_arch is a string like "i686", "x86_64", "arm", or "aarch64", so it doesn't generally make sense to add that to -march. call_aprs.S is ARM code, so we correct target_arch from "arm" to "armv7" above.

D31259 also added this line:

# FIXME: move the call_apsr.s into call_apsr.h as inline-asm.

If we still want to do that, that ought to be easy.

In D82147#2123072, @compnerd wrote:

I dont think that there is a guarantee that target_cflags will include -target armv7-unknown-linux-androideabi. Can we explicitly set the target triple instead?

I think that's right, but we're using target_cflags to compile all the unit test C files (via %clang_builtins), so it ought to do the right thing for the asm file too? It won't matter if I remove this code, though.

[rprichard]

Replace call_apsr.S with inline asm in call_apsr.h.

[compnerd]

I really like this approach way better. Thank you!

compiler-rt/test/builtins/Unit/arm/aeabi_cdcmpeq_test.c

 // REQUIRES: arm-target-arch || armv6m-target-arch
-// RUN: %arm_call_apsr -o %t.aspr.o
-// RUN: %clang_builtins %s %t.aspr.o %librt -o %t && %run %t
+// RUN: %clang_builtins %s %librt -o %t && %run %t
 
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <math.h>
 
 #if __arm__
 #include "call_apsr.h"

compiler-rt/test/builtins/Unit/arm/aeabi_cdcmple_test.c

 // REQUIRES: arm-target-arch || armv6m-target-arch
-// RUN: %arm_call_apsr -o %t.aspr.o
-// RUN: %clang_builtins %s  %t.aspr.o %librt -o %t && %run %t
+// RUN: %clang_builtins %s %librt -o %t && %run %t
 
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <math.h>
 
 #include "call_apsr.h"
 

compiler-rt/test/builtins/Unit/arm/aeabi_cfcmpeq_test.c

 // REQUIRES: arm-target-arch || armv6m-target-arch
-// RUN: %arm_call_apsr -o %t.aspr.o
-// RUN: %clang_builtins %s  %t.aspr.o %librt -o %t && %run %t
+// RUN: %clang_builtins %s %librt -o %t && %run %t
 
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <math.h>
 
 #if __arm__
 #include "call_apsr.h"

compiler-rt/test/builtins/Unit/arm/aeabi_cfcmple_test.c

 // REQUIRES: arm-target-arch || armv6m-target-arch
-// RUN: %arm_call_apsr -o %t.aspr.o
-// RUN: %clang_builtins %s  %t.aspr.o %librt -o %t && %run %t
+// RUN: %clang_builtins %s %librt -o %t && %run %t
 
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <math.h>
 
 #include "call_apsr.h"
 

compiler-rt/test/builtins/Unit/arm/call_apsr.h

struct {
         uint32_t v: 1;
         uint32_t c: 1;
         uint32_t z: 1;
         uint32_t n: 1;
     } flags;
     uint32_t value;
 };
 
-extern __attribute__((pcs("aapcs")))
-uint32_t call_apsr_f(float a, float b, __attribute__((pcs("aapcs"))) void (*fn)(float, float));
+__attribute__((noinline, pcs("aapcs"))) static uint32_t call_apsr_f(float a, float b,
+                                                                    __attribute__((pcs("aapcs"))) void (*fn)(float, float)) {
+  uint32_t result;
+  fn(a, b);
+  asm volatile("mrs %0, apsr"
+               : "=r"(result));
+  return result;
+}
 
-extern __attribute__((pcs("aapcs")))
-uint32_t call_apsr_d(double a, double b, __attribute__((pcs("aapcs"))) void (*fn)(double, double));
+__attribute__((noinline, pcs("aapcs"))) static uint32_t call_apsr_d(double a, double b,
+                                                                    __attribute__((pcs("aapcs"))) void (*fn)(double, double)) {
+  uint32_t result;
+  fn(a, b);
+  asm volatile("mrs %0, apsr"
+               : "=r"(result));
+  return result;
+}
 
 #endif // CALL_APSR_H

compiler-rt/test/builtins/Unit/arm/call_apsr.S

-#include "../../../../lib/builtins/assembly.h"
-
-.syntax unified
-// __attribute__((pcs("aapcs")))
-// int32_t call_apsr_d(double a, double b, void(*fn)(double, double)) {
-//   fn(a, b);
-//   return apsr;
-// }
-
-DEFINE_COMPILERRT_PRIVATE_FUNCTION(call_apsr_d)
-    push {r7, lr}
-    ldr r7, [sp, #8]
-    blx r7
-    mrs r0, apsr
-    pop {r7, pc}
-END_COMPILERRT_FUNCTION(call_apsr_d)
-
-// __attribute__((pcs("aapcs")))
-// int32_t call_apsr_f(float a, float b, void(*fn)(float, float)) {
-//   fn(a, b);
-//   return apsr;
-// }
-
-DEFINE_COMPILERRT_PRIVATE_FUNCTION(call_apsr_f)
-    push {lr}
-    blx r2
-    mrs r0, apsr
-    pop {pc}
-END_COMPILERRT_FUNCTION(call_apsr_f)

compiler-rt/test/builtins/Unit/lit.cfg.py

if not builtins_is_msvc:
   config.available_features.add('int128')
 
 clang_wrapper = ""
 
 def build_invocation(compile_flags):
   return " " + " ".join([clang_wrapper, config.clang] + compile_flags) + " "
 
 
-target_arch = config.target_arch
-if (target_arch == "arm"):
-  target_arch = "armv7"
-
 config.substitutions.append( ("%clang ", build_invocation(target_cflags)) )
 config.substitutions.append( ("%clangxx ", build_invocation(target_cxxflags)) )
 config.substitutions.append( ("%clang_builtins ", \
                               build_invocation(clang_builtins_cflags)))
 config.substitutions.append( ("%clangxx_builtins ", \
                               build_invocation(clang_builtins_cxxflags)))
 
-# FIXME: move the call_apsr.s into call_apsr.h as inline-asm.
-# some ARM tests needs call_apsr.s
-call_apsr_source = os.path.join(builtins_lit_source_dir, 'arm', 'call_apsr.S')
-march_flag = '-march=' + target_arch
-call_apsr_flags = ['-c', march_flag, call_apsr_source]
-config.substitutions.append( ("%arm_call_apsr ", \
-                              build_invocation(call_apsr_flags)) )
-
 # Default test suffixes.
 config.suffixes = ['.c', '.cpp']
 
 if not config.emulator:
   config.available_features.add('native-run')
 
 # Add features for available sources
 builtins_source_features = config.builtins_lit_source_features.split(';')