r324498 - [Driver] Add option to manually control discarding value names in LLVM IR.
Eric Fiselier via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 12 14:25:41 PST 2018
On Mon, Feb 12, 2018 at 9:15 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Wed, Feb 7, 2018 at 10:38 AM Eric Fiselier via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: ericwf
>> Date: Wed Feb 7 10:36:51 2018
>> New Revision: 324498
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=324498&view=rev
>> Log:
>> [Driver] Add option to manually control discarding value names in LLVM IR.
>>
>> Summary:
>> Currently, assertion-disabled Clang builds emit value names when
>> generating LLVM IR. This is controlled by the `NDEBUG` macro, and is not
>> easily overridable. In order to get IR output containing names from a
>> release build of Clang, the user must manually construct the CC1 invocation
>> w/o the `-discard-value-names` option. This is less than ideal.
>>
>> For example, Godbolt uses a release build of Clang, and so when asked to
>> emit LLVM IR the result lacks names, making it harder to read. Manually
>> invoking CC1 on Compiler Explorer is not feasible.
>>
>
> It wouldn't necessarily have to invoke CC1, it could use "-Xclang
> -discard-value-names".
>
If you were using an assertion build, and wanted to disable value names,
then yes -- that would work. However it's the opposite case that is of
interest:
When you're using a non-assertion build and want to keep value names. In
that case invoking CC1 directly is required; otherwise the driver would pass
"-discard-value-names".
>
>
>>
>> This patch adds the driver options `-fdiscard-value-names` and
>> `-fno-discard-value-names` which allow the user to override the default
>> behavior. If neither is specified, the old behavior remains.
>>
>> Reviewers: erichkeane, aaron.ballman, lebedev.ri
>>
>> Reviewed By: aaron.ballman
>>
>> Subscribers: bogner, cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D42887
>>
>> Modified:
>> cfe/trunk/docs/UsersManual.rst
>> cfe/trunk/include/clang/Driver/Options.td
>> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>> cfe/trunk/test/Driver/clang_f_opts.c
>>
>> Modified: cfe/trunk/docs/UsersManual.rst
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/
>> UsersManual.rst?rev=324498&r1=324497&r2=324498&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/docs/UsersManual.rst (original)
>> +++ cfe/trunk/docs/UsersManual.rst Wed Feb 7 10:36:51 2018
>> @@ -1855,6 +1855,27 @@ features. You can "tune" the debug info
>> must come first.)
>>
>>
>> +Controlling LLVM IR Output
>> +--------------------------
>> +
>> +Controlling Value Names in LLVM IR
>> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> +
>> +Emitting value names in LLVM IR increases the size and verbosity of the
>> IR.
>> +By default, value names are only emitted in assertion-enabled builds of
>> Clang.
>> +However, when reading IR it can be useful to re-enable the emission of
>> value
>> +names to improve readability.
>> +
>> +.. option:: -fdiscard-value-names
>> +
>> + Discard value names when generating LLVM IR.
>> +
>> +.. option:: -fno-discard-value-names
>> +
>> + Do not discard value names when generating LLVM IR. This option can be
>> used
>> + to re-enable names for release builds of Clang.
>> +
>> +
>> Comment Parsing Options
>> -----------------------
>>
>>
>> Modified: cfe/trunk/include/clang/Driver/Options.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
>> clang/Driver/Options.td?rev=324498&r1=324497&r2=324498&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/include/clang/Driver/Options.td (original)
>> +++ cfe/trunk/include/clang/Driver/Options.td Wed Feb 7 10:36:51 2018
>> @@ -790,6 +790,10 @@ def fdiagnostics_show_template_tree : Fl
>> HelpText<"Print a template comparison tree for differing templates">;
>> def fdeclspec : Flag<["-"], "fdeclspec">, Group<f_clang_Group>,
>> HelpText<"Allow __declspec as a keyword">, Flags<[CC1Option]>;
>> +def fdiscard_value_names : Flag<["-"], "fdiscard-value-names">,
>> Group<f_clang_Group>,
>> + HelpText<"Discard value names in LLVM IR">, Flags<[DriverOption]>;
>> +def fno_discard_value_names : Flag<["-"], "fno-discard-value-names">,
>> Group<f_clang_Group>,
>> + HelpText<"Do not discard value names in LLVM IR">,
>> Flags<[DriverOption]>;
>> def fdollars_in_identifiers : Flag<["-"], "fdollars-in-identifiers">,
>> Group<f_Group>,
>> HelpText<"Allow '$' in identifiers">, Flags<[CC1Option]>;
>> def fdwarf2_cfi_asm : Flag<["-"], "fdwarf2-cfi-asm">,
>> Group<clang_ignored_f_Group>;
>>
>> Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/
>> ToolChains/Clang.cpp?rev=324498&r1=324497&r2=324498&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
>> +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Wed Feb 7 10:36:51 2018
>> @@ -3266,13 +3266,24 @@ void Clang::ConstructJob(Compilation &C,
>> if (!C.isForDiagnostics())
>> CmdArgs.push_back("-disable-free");
>>
>> -// Disable the verification pass in -asserts builds.
>> #ifdef NDEBUG
>> - CmdArgs.push_back("-disable-llvm-verifier");
>> - // Discard LLVM value names in -asserts builds.
>> - CmdArgs.push_back("-discard-value-names");
>> + const bool IsAssertBuild = false;
>> +#else
>> + const bool IsAssertBuild = true;
>> #endif
>>
>> + // Disable the verification pass in -asserts builds.
>> + if (!IsAssertBuild)
>> + CmdArgs.push_back("disable-llvm-verifier");
>> +
>> + // Discard value names in assert builds unless otherwise specified.
>> + if (const Arg *A = Args.getLastArg(options::OPT_fdiscard_value_names,
>> + options::OPT_fno_discard_value_names))
>> {
>> + if (A->getOption().matches(options::OPT_fdiscard_value_names))
>>
>
> Should this ^ be:
>
> if (Args.hasFlag(options::OPT_fdiscard_value_names,
> options::OPT_fno_discard_value_names, false))
>
Yeah, that looks better, but perhaps this would be even cleaner:
if (Args.hasFlag(options::OPT_fdiscard_value_names,
options::OPT_fno_discard_value_names, !IsAssertBuild))
>
> instead? (simpler, one operation instead of two, etc)
>
>
>> + CmdArgs.push_back("-discard-value-names");
>> + } else if (!IsAssertBuild)
>> + CmdArgs.push_back("-discard-value-names");
>> +
>> // Set the main file name, so that debug info works even with
>> // -save-temps.
>> CmdArgs.push_back("-main-file-name");
>>
>> Modified: cfe/trunk/test/Driver/clang_f_opts.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/
>> clang_f_opts.c?rev=324498&r1=324497&r2=324498&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/test/Driver/clang_f_opts.c (original)
>> +++ cfe/trunk/test/Driver/clang_f_opts.c Wed Feb 7 10:36:51 2018
>> @@ -517,3 +517,8 @@
>> // RUN: %clang -### -S %s 2>&1 | FileCheck -check-prefix=CHECK-NO-CF-PROTECTION-BRANCH
>> %s
>> // CHECK-CF-PROTECTION-BRANCH: -fcf-protection=branch
>> // CHECK-NO-CF-PROTECTION-BRANCH-NOT: -fcf-protection=branch
>> +
>> +// RUN: %clang -### -S -fdiscard-value-names %s 2>&1 | FileCheck
>> -check-prefix=CHECK-DISCARD-NAMES %s
>>
>
> Should there also be a RUN line here for the default behavior, when no arg
> is specified?
>
AFAIK it is not currently testable, since the result of the test depends on
the build configuration, and AFAIK that information
isn't available in the LIT test suite.
I'm sure with a bit of messy CMake I could find a way to transmit if
assertions are enabled to LIT, add it to the set of available
features, and write two different tests that have REQUIRE/UNSUPPORTED lines
which turn them on/off depending on
the build type.
However, this behavior has gone untested for years, so I wasn't sure a test
was required now.
>
>
>> +// RUN: %clang -### -S -fno-discard-value-names %s 2>&1 | FileCheck
>> -check-prefix=CHECK-NO-DISCARD-NAMES %s
>> +// CHECK-DISCARD-NAMES: "-discard-value-names"
>> +// CHECK-NO-DISCARD-NAMES-NOT: "-discard-value-names"
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180212/e5577070/attachment-0001.html>
More information about the cfe-commits
mailing list