r234668 - [Driver] Properly support -mglobal-merge using explicit options.
Eric Christopher
echristo at gmail.com
Tue Apr 14 08:04:41 PDT 2015
On Tue, Apr 14, 2015 at 3:42 PM Ahmed Bougacha <ahmed.bougacha at gmail.com>
wrote:
> On Tue, Apr 14, 2015 at 5:40 AM, Eric Christopher <echristo at gmail.com>
> wrote:
> > So, this is fine for now as a workaround, but using backend options is
> > pretty terrible.
>
> Oh yes, agreed.
>
> > What we really need is a way of describing to the backend
> > the particular options we'd like to communicate on and off
> programmatically
> > without relying on cl::ParseCommandLineOptions. Currently I'm thinking
> of a
> > target dependent struct that's passed along to TargetMachine creation.
> Other
> > thoughts? I think Akira had mentioned a possibility of a string/string
> map
> > as well.
>
> I might be missing the point, but where would this logic go? More
> specifically, the driver (where we currently do this) is always one
> step removed from TargetMachine creation, no? How do we pass this
> through to -cc1, or to the LTO link invocation?
>
>
Going to separate this out to the -cc1 side of things. LTO is terrible for
passing code generation options around. I've got some ideas there, but you
haven't really solved the LTO problem anyhow have you?
For -cc1 I'd just do what we do with a lot of things, pass a flag to -cc1
and don't worry about it. Parse it via the cc1 options and then use that to
fill in whatever data structure we want. What you're doing is basically the
string idea via "environment" parsing which isn't any better really.
> More generally, how does this relate to LTO? The usual annoying use
> case is -msomething on one .cpp, -mno-something on the other, and LTO
> on all this. From what I saw, this makes sense on some (?) but not all
> such options.
>
>
It makes sense on no fronts really. This is why I want global code
generation options for LTO. Either you agree on everything which is great,
or you don't at which point you, what? Error? Pick one?
> To get this working for GlobalMerge in particular, the best proposed
> solution so far was having clang annotate each global with some
> target-specific string attribute, much as we do for functions.
>
>
Yeah, I really don't like this. I'd rather the lto code be able to take
options to change the pass manager. We already support plugin-opt for the
gold plugin, there's no real reason we couldn't expand that further (or, we
start driving LTO via clang which is my pie in the sky idea on how this
should work, but that's a lot more work that I'm not signing up to do
myself right now).
> Currently, we don't support these mismatching flags, and from that
> standpoint, either of the solutions, if I understand them correctly,
> sound reasonable to me (with a slight preference for the struct,
> because then at least, out-of-sync means broken build).
>
>
Right. That's my preference too.
> So, my real question is: when, if ever, does it make sense to support
> that kind of thing?
>
>
I think I explained that above, or I've missed something. Let's keep this
thread going and nail down the solution :)
Thanks!
-eric
> -Ahmed
>
> > -eric
> >
> > On Sat, Apr 11, 2015 at 1:16 AM Ahmed Bougacha <ahmed.bougacha at gmail.com
> >
> > wrote:
> >>
> >> Author: ab
> >> Date: Fri Apr 10 19:10:44 2015
> >> New Revision: 234668
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=234668&view=rev
> >> Log:
> >> [Driver] Properly support -mglobal-merge using explicit options.
> >>
> >> Follow-up to r234666. With this, the -m[no-]global-merge options
> >> have the expected behavior. Previously, -mglobal-merge was ignored,
> >> and there was no way of enabling the optimization.
> >>
> >> Added:
> >> cfe/trunk/test/Driver/mglobal-merge.c
> >> - copied, changed from r234656,
> >> cfe/trunk/test/Driver/mno-global-merge.c
> >> Removed:
> >> cfe/trunk/test/Driver/mno-global-merge.c
> >> Modified:
> >> cfe/trunk/include/clang/Driver/Options.td
> >> cfe/trunk/include/clang/Frontend/CodeGenOptions.def
> >> cfe/trunk/lib/CodeGen/BackendUtil.cpp
> >> cfe/trunk/lib/Driver/Tools.cpp
> >> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> >>
> >> Modified: cfe/trunk/include/clang/Driver/Options.td
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=234668&r1=234667&r2=234668&view=diff
> >>
> >>
> ==============================================================================
> >> --- cfe/trunk/include/clang/Driver/Options.td (original)
> >> +++ cfe/trunk/include/clang/Driver/Options.td Fri Apr 10 19:10:44 2015
> >> @@ -1146,7 +1146,8 @@ def mfloat_abi_EQ : Joined<["-"], "mfloa
> >> def mfpmath_EQ : Joined<["-"], "mfpmath=">, Group<m_Group>;
> >> def mfpu_EQ : Joined<["-"], "mfpu=">, Group<m_Group>;
> >> def mhwdiv_EQ : Joined<["-"], "mhwdiv=">, Group<m_Group>;
> >> -def mglobal_merge : Flag<["-"], "mglobal-merge">, Group<m_Group>;
> >> +def mglobal_merge : Flag<["-"], "mglobal-merge">, Group<m_Group>,
> >> Flags<[CC1Option]>,
> >> + HelpText<"Enable merging of globals">;
> >> def mhard_float : Flag<["-"], "mhard-float">, Group<m_Group>;
> >> def miphoneos_version_min_EQ : Joined<["-"], "miphoneos-version-min=">,
> >> Group<m_Group>;
> >> def mios_version_min_EQ : Joined<["-"], "mios-version-min=">,
> >>
> >> Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=234668&r1=234667&r2=234668&view=diff
> >>
> >>
> ==============================================================================
> >> --- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
> >> +++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Fri Apr 10
> >> 19:10:44 2015
> >> @@ -77,7 +77,6 @@ CODEGENOPT(NoExecStack , 1, 0) ///
> >> CODEGENOPT(FatalWarnings , 1, 0) ///< Set when -Wa,--fatal-warnings
> >> is
> >> ///< enabled.
> >> CODEGENOPT(EnableSegmentedStacks , 1, 0) ///< Set when -fsplit-stack is
> >> enabled.
> >> -CODEGENOPT(NoGlobalMerge , 1, 0) ///< Set when -mno-global-merge is
> >> enabled.
> >> CODEGENOPT(NoImplicitFloat , 1, 0) ///< Set when -mno-implicit-float
> is
> >> enabled.
> >> CODEGENOPT(NoInfsFPMath , 1, 0) ///< Assume FP arguments, results
> >> not +-Inf.
> >> CODEGENOPT(NoSignedZeros , 1, 0) ///< Allow ignoring the signedness
> >> of FP zero
> >>
> >> Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=234668&r1=234667&r2=234668&view=diff
> >>
> >>
> ==============================================================================
> >> --- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
> >> +++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Fri Apr 10 19:10:44 2015
> >> @@ -436,8 +436,6 @@ TargetMachine *EmitAssemblyHelper::Creat
> >> BackendArgs.push_back("-time-passes");
> >> for (unsigned i = 0, e = CodeGenOpts.BackendOptions.size(); i != e;
> >> ++i)
> >> BackendArgs.push_back(CodeGenOpts.BackendOptions[i].c_str());
> >> - if (CodeGenOpts.NoGlobalMerge)
> >> - BackendArgs.push_back("-enable-global-merge=false");
> >> BackendArgs.push_back(nullptr);
> >> llvm::cl::ParseCommandLineOptions(BackendArgs.size() - 1,
> >> BackendArgs.data());
> >>
> >> Modified: cfe/trunk/lib/Driver/Tools.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=234668&r1=234667&r2=234668&view=diff
> >>
> >>
> ==============================================================================
> >> --- cfe/trunk/lib/Driver/Tools.cpp (original)
> >> +++ cfe/trunk/lib/Driver/Tools.cpp Fri Apr 10 19:10:44 2015
> >> @@ -861,12 +861,14 @@ void Clang::AddARMTargetArgs(const ArgLi
> >> }
> >> }
> >>
> >> - // Setting -mno-global-merge disables the codegen global merge pass.
> >> Setting
> >> - // -mglobal-merge has no effect as the pass is enabled by default.
> >> + // Forward the -mglobal-merge option for explicit control over the
> >> pass.
> >> if (Arg *A = Args.getLastArg(options::OPT_mglobal_merge,
> >> options::OPT_mno_global_merge)) {
> >> + CmdArgs.push_back("-backend-option");
> >> if (A->getOption().matches(options::OPT_mno_global_merge))
> >> - CmdArgs.push_back("-mno-global-merge");
> >> + CmdArgs.push_back("-arm-global-merge=false");
> >> + else
> >> + CmdArgs.push_back("-arm-global-merge=true");
> >> }
> >>
> >> if (!Args.hasFlag(options::OPT_mimplicit_float,
> >> @@ -957,12 +959,14 @@ void Clang::AddAArch64TargetArgs(const A
> >> CmdArgs.push_back("-aarch64-fix-cortex-a53-835769=1");
> >> }
> >>
> >> - // Setting -mno-global-merge disables the codegen global merge pass.
> >> Setting
> >> - // -mglobal-merge has no effect as the pass is enabled by default.
> >> + // Forward the -mglobal-merge option for explicit control over the
> >> pass.
> >> if (Arg *A = Args.getLastArg(options::OPT_mglobal_merge,
> >> options::OPT_mno_global_merge)) {
> >> + CmdArgs.push_back("-backend-option");
> >> if (A->getOption().matches(options::OPT_mno_global_merge))
> >> - CmdArgs.push_back("-mno-global-merge");
> >> + CmdArgs.push_back("-aarch64-global-merge=false");
> >> + else
> >> + CmdArgs.push_back("-aarch64-global-merge=true");
> >> }
> >>
> >> if (Args.hasArg(options::OPT_ffixed_x18)) {
> >>
> >> Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=234668&r1=234667&r2=234668&view=diff
> >>
> >>
> ==============================================================================
> >> --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
> >> +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Fri Apr 10 19:10:44
> 2015
> >> @@ -455,7 +455,6 @@ static bool ParseCodeGenArgs(CodeGenOpti
> >> Opts.NoZeroInitializedInBSS =
> >> Args.hasArg(OPT_mno_zero_initialized_in_bss);
> >> Opts.BackendOptions = Args.getAllArgValues(OPT_backend_option);
> >> Opts.NumRegisterParameters = getLastArgIntValue(Args, OPT_mregparm,
> 0,
> >> Diags);
> >> - Opts.NoGlobalMerge = Args.hasArg(OPT_mno_global_merge);
> >> Opts.NoExecStack = Args.hasArg(OPT_mno_exec_stack);
> >> Opts.FatalWarnings = Args.hasArg(OPT_massembler_fatal_warnings);
> >> Opts.EnableSegmentedStacks = Args.hasArg(OPT_split_stacks);
> >>
> >> Copied: cfe/trunk/test/Driver/mglobal-merge.c (from r234656,
> >> cfe/trunk/test/Driver/mno-global-merge.c)
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/mglobal-merge.c?p2=cfe/trunk/test/Driver/mglobal-merge.c&p1=cfe/trunk/test/Driver/mno-global-merge.c&r1=234656&r2=234668&rev=234668&view=diff
> >>
> >>
> ==============================================================================
> >> --- cfe/trunk/test/Driver/mno-global-merge.c (original)
> >> +++ cfe/trunk/test/Driver/mglobal-merge.c Fri Apr 10 19:10:44 2015
> >> @@ -1,20 +1,40 @@
> >> -// RUN: %clang -target armv7-apple-darwin10 \
> >> -// RUN: -mno-global-merge -### -fsyntax-only %s 2> %t
> >> -// RUN: FileCheck --check-prefix=CHECK-NGM < %t %s
> >> +// RUN: %clang -target armv7-unknown-unknown -### -fsyntax-only %s 2>
> %t
> >> \
> >> +// RUN: -mno-global-merge
> >> +// RUN: FileCheck --check-prefix=CHECK-NGM-ARM < %t %s
> >>
> >> -// RUN: %clang -target arm64-apple-ios7 \
> >> -// RUN: -mno-global-merge -### -fsyntax-only %s 2> %t
> >> -// RUN: FileCheck --check-prefix=CHECK-NGM < %t %s
> >> +// RUN: %clang -target aarch64-unknown-unknown -### -fsyntax-only %s 2>
> >> %t \
> >> +// RUN: -mno-global-merge
> >> +// RUN: FileCheck --check-prefix=CHECK-NGM-AARCH64 < %t %s
> >>
> >> -// CHECK-NGM: "-mno-global-merge"
> >> +// RUN: %clang -target x86_64-unknown-unknown -### -fsyntax-only %s 2>
> %t
> >> \
> >> +// RUN: -mno-global-merge
> >> +// RUN: FileCheck --check-prefix=CHECK-NONE < %t %s
> >>
> >> -// RUN: %clang -target armv7-apple-darwin10 \
> >> -// RUN: -mglobal-merge -### -fsyntax-only %s 2> %t
> >> -// RUN: FileCheck --check-prefix=CHECK-GM < %t %s
> >> +// CHECK-NGM-ARM: "-backend-option" "-arm-global-merge=false"
> >> +// CHECK-NGM-AARCH64: "-backend-option" "-aarch64-global-merge=false"
> >>
> >> -// RUN: %clang -target arm64-apple-ios7 \
> >> -// RUN: -mglobal-merge -### -fsyntax-only %s 2> %t
> >> -// RUN: FileCheck --check-prefix=CHECK-GM < %t %s
> >> +// RUN: %clang -target armv7-unknown-unknown -### -fsyntax-only %s 2>
> %t
> >> \
> >> +// RUN: -mglobal-merge
> >> +// RUN: FileCheck --check-prefix=CHECK-GM-ARM < %t %s
> >>
> >> -// CHECK-GM-NOT: "-mglobal-merge"
> >> +// RUN: %clang -target aarch64-unknown-unknown -### -fsyntax-only %s 2>
> >> %t \
> >> +// RUN: -mglobal-merge
> >> +// RUN: FileCheck --check-prefix=CHECK-GM-AARCH64 < %t %s
> >>
> >> +// RUN: %clang -target x86_64-unknown-unknown -### -fsyntax-only %s 2>
> %t
> >> \
> >> +// RUN: -mglobal-merge
> >> +// RUN: FileCheck --check-prefix=CHECK-NONE < %t %s
> >> +
> >> +// CHECK-GM-ARM: "-backend-option" "-arm-global-merge=true"
> >> +// CHECK-GM-AARCH64: "-backend-option" "-aarch64-global-merge=true"
> >> +
> >> +// RUN: %clang -target armv7-unknown-unknown -### -fsyntax-only %s 2>
> %t
> >> +// RUN: FileCheck --check-prefix=CHECK-NONE < %t %s
> >> +
> >> +// RUN: %clang -target aarch64-unknown-unknown -### -fsyntax-only %s 2>
> >> %t
> >> +// RUN: FileCheck --check-prefix=CHECK-NONE < %t %s
> >> +
> >> +// RUN: %clang -target x86_64-unknown-unknown -### -fsyntax-only %s 2>
> %t
> >> +// RUN: FileCheck --check-prefix=CHECK-NONE < %t %s
> >> +
> >> +// CHECK-NONE-NOT: -global-merge=
> >>
> >> Removed: cfe/trunk/test/Driver/mno-global-merge.c
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/mno-global-merge.c?rev=234667&view=auto
> >>
> >>
> ==============================================================================
> >> --- cfe/trunk/test/Driver/mno-global-merge.c (original)
> >> +++ cfe/trunk/test/Driver/mno-global-merge.c (removed)
> >> @@ -1,20 +0,0 @@
> >> -// RUN: %clang -target armv7-apple-darwin10 \
> >> -// RUN: -mno-global-merge -### -fsyntax-only %s 2> %t
> >> -// RUN: FileCheck --check-prefix=CHECK-NGM < %t %s
> >> -
> >> -// RUN: %clang -target arm64-apple-ios7 \
> >> -// RUN: -mno-global-merge -### -fsyntax-only %s 2> %t
> >> -// RUN: FileCheck --check-prefix=CHECK-NGM < %t %s
> >> -
> >> -// CHECK-NGM: "-mno-global-merge"
> >> -
> >> -// RUN: %clang -target armv7-apple-darwin10 \
> >> -// RUN: -mglobal-merge -### -fsyntax-only %s 2> %t
> >> -// RUN: FileCheck --check-prefix=CHECK-GM < %t %s
> >> -
> >> -// RUN: %clang -target arm64-apple-ios7 \
> >> -// RUN: -mglobal-merge -### -fsyntax-only %s 2> %t
> >> -// RUN: FileCheck --check-prefix=CHECK-GM < %t %s
> >> -
> >> -// CHECK-GM-NOT: "-mglobal-merge"
> >> -
> >>
> >>
> >> _______________________________________________
> >> cfe-commits mailing list
> >> cfe-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150414/33c8e1c7/attachment.html>
More information about the cfe-commits
mailing list