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

[Duncan P. N. Exon Smith]

> On 2015-Apr-30, at 13:00, Akira Hatanaka <ahatanak at gmail.com> wrote:
> 
> Hi echristo, dexonsmith,
> 
> 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.

Binary SGTM.

> 
> 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::cl::ParseCommandLineOptions?

IMO, we should create a proper -cc1 option, stop using
`-backend-option`, and stop passing anything to
`cl::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.

So in this example, #2 would behave the same, but #1 would stop
enforcing -arm-long-calls (since old.bc doesn't have arm-long-calls
encoded, and -mlong-calls only adds attributes to functions in IRGen).

  - AFAICT, there wasn't a supported way to pass -arm-long-calls to
    LTO until now (via attributes), so we won't regress behaviour
    there.
  - Outside of LTO, archiving bitcode and later codegen'ing from
    clang isn't supported, unless I'm missing something?

IMO we don't need to worry about backwards compatibility for #1, so
I think: make it a binary option and stop using the `cl::opt`.

> 
> http://reviews.llvm.org/D9414
> 
> Files:
>  include/clang/Frontend/CodeGenOptions.h
>  lib/CodeGen/CGCall.cpp
>  lib/Frontend/CompilerInvocation.cpp
>  test/CodeGen/fn-attr.c
> 
> Index: include/clang/Frontend/CodeGenOptions.h
> ===================================================================
> --- include/clang/Frontend/CodeGenOptions.h
> +++ include/clang/Frontend/CodeGenOptions.h
> @@ -151,6 +151,9 @@
>   /// A list of command-line options to forward to the LLVM backend.
>   std::vector<std::string> BackendOptions;
> 
> +  /// A list of function attributes to save to the IR.
> +  std::vector<std::pair<std::string, std::string>> FunctionAttributes;
> +
>   /// A list of dependent libraries.
>   std::vector<std::string> DependentLibraries;
> 
> Index: lib/CodeGen/CGCall.cpp
> ===================================================================
> --- lib/CodeGen/CGCall.cpp
> +++ lib/CodeGen/CGCall.cpp
> @@ -1455,6 +1455,9 @@
>       FuncAttrs.addAttribute(llvm::Attribute::NoBuiltin);
>   } else {
>     // Attributes that should go on the function, but not the call site.
> +    for (auto &KV : CodeGenOpts.FunctionAttributes)
> +      FuncAttrs.addAttribute(KV.first, KV.second);
> +
>     if (!CodeGenOpts.DisableFPElim) {
>       FuncAttrs.addAttribute("no-frame-pointer-elim", "false");
>     } else if (CodeGenOpts.OmitLeafFramePointer) {
> Index: lib/Frontend/CompilerInvocation.cpp
> ===================================================================
> --- lib/Frontend/CompilerInvocation.cpp
> +++ lib/Frontend/CompilerInvocation.cpp
> @@ -340,6 +340,14 @@
>   }
> }
> 
> +static void getFunctionAttributes(CodeGenOptions &Opts) {
> +  StringRef Opt = "-arm-long-calls", Key = Opt.drop_front(1);
> +
> +  if (std::find(Opts.BackendOptions.begin(), Opts.BackendOptions.end(),
> +                Opt) != Opts.BackendOptions.end())
> +    Opts.FunctionAttributes.push_back(std::make_pair(Key, ""));
> +}
> +
> static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
>                              DiagnosticsEngine &Diags,
>                              const TargetOptions &TargetOpts) {
> @@ -643,6 +651,8 @@
>                       Args.getAllArgValues(OPT_fsanitize_recover_EQ), Diags,
>                       Opts.SanitizeRecover);
> 
> +  getFunctionAttributes(Opts);
> +
>   return Success;
> }
> 
> Index: test/CodeGen/fn-attr.c
> ===================================================================
> --- /dev/null
> +++ test/CodeGen/fn-attr.c
> @@ -0,0 +1,7 @@
> +// RUN: %clang_cc1 -triple thumbv7-apple-ios5 -backend-option -arm-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 = { {{.*}} "arm-long-calls
> +// NOLONGCALL-NOT: attributes #0 = { {{.*}} "arm-long-calls
> +
> +int foo1(int a) { return a; }
> 
> EMAIL PREFERENCES
>  http://reviews.llvm.org/settings/panel/emailpreferences/
> <D9414.24762.patch>