[ARM] Save option "arm-long-calls" to the IR as a function attribute

[ahatanak]

Summary

This patch is part of the work to make LTO and function multi-versioning work correctly.

Currently, -mlong-calls, which is converted to cc1 option -arm-long-calls, is ignored when building with LTO because the option isn't passed to the linker or libLTO. This patch saves the option in the IR as a function attribute to fix this problem.

The corresponding llvm patch is here:
http://reviews.llvm.org/D9364

There are a few things to discuss:

1. Should "arm-long-calls" be a binary attribute or a tri-state like "unsafe-fp-math" that takes a value "true" or "false"? I made it a binary attribute because it simplifies the backend and frontend without breaking backward compatibility, but might be use cases that I'm unaware of in which this approach wouldn't work.

2. Since we are saving the option in the IR, should we stop passing it as a cc1 backend option and stop passing it to llvm:🆑:ParseCommandLineOptions? It's not needed if this attribute is a tri-state, but is needed if it's a binary to preserve backward compatibility. By "backward compatibility", I mean the following commands should produce the same result before and after this patch is committed:

1. clang -target armv7-apple-ios -static -mlong-calls old.bc -o old
2. clang -target armv7-apple-ios -static old.bc -o old

Here, old.bc is generated by an older version of clang that doesn't save "arm-long-calls" in the IR.

[ahatanak]

Made changes based on feedback comments.

In the new patch, a new cc1 option "function-attribute" is defined and "arm-long-calls" is passed as a function-attribute instead of a backend-option. I think this is better than my previous patch as it doesn't have to check "arm-long-calls", which is a target-specific attribute, in CompilerInvocation.cpp, and it no longer passes the string to cl::ParseCommandLineOptions to toggle the cl::opt option in the backend.

[echristo]

Hi Akira,

I'm not sure how we want to do calling convention type things. I don't know that we want it as a generic attribute on the function or as a subtarget feature. What are your thoughts here?

-eric

[ahatanak]

It seems to me that turning arm-long-calls into a subtarget feature would simplify both clang and llvm.

[ahatanak]

Save "long-calls" as a subtarget feature of ARM instead of a function attribute.

[echristo]

Couple of inline comments, go ahead and commit when you've resolved them.

Thanks!

-eric

lib/Driver/Tools.cpp

715–716

Can you add a plain english comment above this explaining what we're supposed to be checking here? :)

test/CodeGen/arm-long-calls.c

4

Test for -mno-long-calls?

[ahatanak]

Add comments to explain how the decision to pass feature "+long-calls" is made.

[ahatanak]

test/CodeGen/arm-long-calls.c

4

"+long-calls" (or "-long-calls") is never passed if -mno-calls is specified on the command line. The driver test (arm-long-calls.c) checks that "+long-calls" gets passed based on occurrence of "-mlong-calls" or "-mno-long-calls".

lib/Driver/Tools.cpp

StringRef tools::arm::getARMFloatABI(const Driver &D, const ArgList &Args,
 
   return FloatABI;
 }
 
 static void getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
                                  const ArgList &Args,
                                  std::vector<const char *> &Features,
                                  bool ForAS) {
+  bool KernelOrKext =
+      Args.hasArg(options::OPT_mkernel, options::OPT_fapple_kext);
   StringRef FloatABI = tools::arm::getARMFloatABI(D, Args, Triple);
   if (!ForAS) {
     // FIXME: Note, this is a hack, the LLVM backend doesn't actually use these
     // yet (it uses the -mfloat-abi and -msoft-float options), and it is
     // stripped out by the ARM target. We should probably pass this a new
     // -target-option, which is handled by the -cc1/-cc1as invocation.
     //
     // FIXME2:  For consistency, it would be ideal if we set up the target
if (A->getOption().matches(options::OPT_mcrc))
       Features.push_back("+crc");
     else
       Features.push_back("-crc");
   }
 
   if (Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v8_1a) {
     Features.insert(Features.begin(), "+v8.1a");
   }
+
+  // Look for the last occurrence of -mlong-calls or -mno-long-calls. If
+  // neither options are specified, see if we are compiling for kernel/kext and
+  // decide whether to pass "+long-calls" based on the OS and its version.
+  if (Arg *A = Args.getLastArg(options::OPT_mlong_calls,
+                               options::OPT_mno_long_calls)) {
+    if (A->getOption().matches(options::OPT_mlong_calls))
+      Features.push_back("+long-calls");
+  } else if (KernelOrKext && (!Triple.isiOS() || Triple.isOSVersionLT(6))) {
+      Features.push_back("+long-calls");
+  }
 }
 
 void Clang::AddARMTargetArgs(const ArgList &Args, ArgStringList &CmdArgs,
                              bool KernelOrKext) const {
   const Driver &D = getToolChain().getDriver();
   // Get the effective triple, which takes into account the deployment target.
   std::string TripleStr = getToolChain().ComputeEffectiveClangTriple(Args);
   llvm::Triple Triple(TripleStr);
if (FloatABI == "soft") {
     // Floating point operations and argument passing are hard.
     assert(FloatABI == "hard" && "Invalid float abi!");
     CmdArgs.push_back("-mfloat-abi");
     CmdArgs.push_back("hard");
   }
 
   // Kernel code has more strict alignment requirements.
   if (KernelOrKext) {
-    if (!Triple.isiOS() || Triple.isOSVersionLT(6)) {
-      CmdArgs.push_back("-backend-option");
-      CmdArgs.push_back("-arm-long-calls");
-    }
-
     CmdArgs.push_back("-backend-option");
     CmdArgs.push_back("-arm-strict-align");
 
     // The kext linker doesn't know how to deal with movw/movt.
     CmdArgs.push_back("-backend-option");
     CmdArgs.push_back("-arm-use-movt=0");
   }
 
if (Arg *A = Args.getLastArg(options::OPT_mrestrict_it,
     }
   } else if (TT.isOSWindows() && (TT.getArch() == llvm::Triple::arm ||
                                   TT.getArch() == llvm::Triple::thumb)) {
     // Windows on ARM expects restricted IT blocks
     CmdArgs.push_back("-backend-option");
     CmdArgs.push_back("-arm-restrict-it");
   }
 
-  if (TT.getArch() == llvm::Triple::arm ||
-      TT.getArch() == llvm::Triple::thumb) {
-    if (Arg *A = Args.getLastArg(options::OPT_mlong_calls,
-                                 options::OPT_mno_long_calls)) {
-      if (A->getOption().matches(options::OPT_mlong_calls)) {
-        CmdArgs.push_back("-backend-option");
-        CmdArgs.push_back("-arm-long-calls");
-      }
-    }
-  }
-
   // Forward -f options with positive and negative forms; we translate
   // these by hand.
   if (Arg *A = Args.getLastArg(options::OPT_fprofile_sample_use_EQ)) {
     StringRef fname = A->getValue();
     if (!llvm::sys::fs::exists(fname))
       D.Diag(diag::err_drv_no_such_file) << fname;
     else
       A->render(Args, CmdArgs);

test/CodeGen/arm-long-calls.c

+// RUN: %clang_cc1 -triple thumbv7-apple-ios5  -target-feature +long-calls -emit-llvm -o - %s | FileCheck -check-prefix=LONGCALL %s
+// RUN: %clang_cc1 -triple thumbv7-apple-ios5 -emit-llvm -o - %s | FileCheck -check-prefix=NOLONGCALL %s
+
+// LONGCALL: attributes #0 = { {{.*}} "target-features"="+long-calls"
+// NOLONGCALL-NOT: attributes #0 = { {{.*}} "target-features"="+long-calls"
+
+int foo1(int a) { return a; }

test/Driver/apple-kext-mkernel.c

 // RUN: %clang -target x86_64-apple-darwin10 \
 // RUN:   -mkernel -### -fsyntax-only %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-X86 < %t %s
 
 // CHECK-X86: "-disable-red-zone"
 // CHECK-X86: "-fno-builtin"
 // CHECK-X86: "-fno-rtti"
 // CHECK-X86: "-fno-common"
 
 // RUN: %clang -target x86_64-apple-darwin10 \
 // RUN:   -arch armv7 -mkernel -mstrict-align -### -fsyntax-only %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-ARM < %t %s
 
-// CHECK-ARM: "-backend-option" "-arm-long-calls"
+// CHECK-ARM: "-target-feature" "+long-calls"
 // CHECK-ARM: "-backend-option" "-arm-strict-align"
 // CHECK-ARM-NOT: "-backend-option" "-arm-strict-align"
 // CHECK-ARM: "-fno-builtin"
 // CHECK-ARM: "-fno-rtti"
 // CHECK-ARM: "-fno-common"
 
 // RUN: %clang -target x86_64-apple-darwin10 \
 // RUN:   -Werror -fno-builtin -fno-exceptions -fno-common -fno-rtti \
 // RUN:   -mkernel -fsyntax-only %s

test/Driver/arm-long-calls.c

 // RUN: %clang -target armv7-eabi -### %s 2>&1 \
 // RUN:    | FileCheck %s -check-prefix CHECK-DEFAULT
 
 // RUN: %clang -target armv7-eabi -### -mlong-calls %s 2>&1 \
 // RUN:    | FileCheck %s -check-prefix CHECK-LONG-CALLS
 
 // RUN: %clang -target armv7-eabi -### -mlong-calls -mno-long-calls %s 2>&1 \
 // RUN:    | FileCheck %s -check-prefix CHECK-NO-LONG-CALLS
 
-// CHECK-DEFAULT-NOT: "-backend-option" "-arm-long-calls"
+// CHECK-DEFAULT-NOT: "-target-feature" "+long-calls"
 
-// CHECK-LONG-CALLS: "-backend-option" "-arm-long-calls"
+// CHECK-LONG-CALLS: "-target-feature" "+long-calls"
 
-// CHECK-NO-LONG-CALLS-NOT: "-backend-option" "-arm-long-calls"
+// CHECK-NO-LONG-CALLS-NOT: "-target-feature" "+long-calls"