r324498 - [Driver] Add option to manually control discarding value names in LLVM IR.

Mon Feb 12 08:15:01 PST 2018

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".

>
> 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))

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?

> +// 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/9eea2b40/attachment-0001.html>