On Thu, Apr 30, 2015 at 1:45 PM, Duncan P. N. Exon Smith < [email protected]> wrote:
> > > On 2015-Apr-30, at 13:00, Akira Hatanaka <[email protected]> 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? > > That's correct. I wasn't worried about the first case, but wasn't sure about the second case. If we don't have to support it, binary should be fine. > 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> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
