On Thu, Apr 30, 2015 at 1:45 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:
>
> > 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?
>
>
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>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150430/b18bf79e/attachment.html>