r234668 - [Driver] Properly support -mglobal-merge using explicit options.
Ahmed Bougacha
ahmed.bougacha at gmail.com
Tue Apr 14 07:41:30 PDT 2015
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?
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.
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.
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).
So, my real question is: when, if ever, does it make sense to support
that kind of thing?
-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
More information about the cfe-commits
mailing list