Binary sounds good. There's already code that handles things like subtarget-features based on command lines, maybe extend that or in that same area handle function attributes on a per-target basis?
-eric On Thu, Apr 30, 2015 at 2:30 PM Akira Hatanaka <[email protected]> wrote: > 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
