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

[EricWF]

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.

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.

[lebedev.ri]

lib/Driver/ToolChains/Clang.cpp

3269

This logic seems sidewards.
If NDEBUG is specified, then it is assert()-less build.
If NDEBUG is *not* specified, then assert()'s are actually functional.

3270

-assert is assert()-less build

[EricWF]

lib/Driver/ToolChains/Clang.cpp

3269

Woops! Silly me. Thats a dumb mistake and I feel dumb having made it.

[EricWF]

• Address really really really dumb mistake.

[lebedev.ri]

test/Driver/clang_f_opts.c

522

I wonder if it is also possible to check that if neither of -f[no-]discard-value-names is specified, what happens.
The caveat is of course the asserts-vs-no-asserts build type.

[aaron.ballman]

docs/UsersManual.rst

1859

Underlining is incorrect here (too long).

1861

s/Values/Value

1862

Same here.

1865

By default value -> By default, value
in assertion enabled builds -> in assertion-enabled builds

[bogner]

lib/Driver/ToolChains/Clang.cpp

3269–3285

It might be a few more characters, but I feel like this is more readable if you put entire statements in the branches of the #if, ie:

#ifdef NDEBUG

const bool IsAssertBuild = false;

#else

const bool IsAssertBuild = true;

#endif

[EricWF]

lib/Driver/ToolChains/Clang.cpp

3269–3285

Ack. Done.

test/Driver/clang_f_opts.c

522

I don't think so, at least not easily and without changes to the lit configuration.

It's gone untested this long, I would love to get this patch in without being responsible for adding those tests.

[EricWF]

• Fix documentation as requested.
• Put entire statement inside of #ifdef blocks.

[aaron.ballman]

LGTM!

docs/UsersManual.rst

 .. option:: -ggdb, -glldb, -gsce
 
   Tune the debug info for the ``gdb``, ``lldb``, or Sony PlayStation\ |reg|
   debugger, respectively. Each of these options implies **-g**. (Therefore, if
   you want both **-gline-tables-only** and debugger tuning, the tuning option
   must come first.)
 
 
+Controlling LLVM IR Output
+---------------------------
+
+Controlling Values 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
 -----------------------
 
 Clang parses Doxygen and non-Doxygen style documentation comments and attaches
 them to the appropriate declaration nodes.  By default, it only parses
 Doxygen-style comments and ignores ordinary comments starting with ``//`` and
 ``/*``.
 

include/clang/Driver/Options.td

def fdiagnostics_show_note_include_stack : Flag<["-"], "fdiagnostics-show-note-include-stack">,
     Group<f_Group>,  Flags<[CC1Option]>, HelpText<"Display include stacks for diagnostic notes">;
 def fdiagnostics_format_EQ : Joined<["-"], "fdiagnostics-format=">, Group<f_clang_Group>;
 def fdiagnostics_show_category_EQ : Joined<["-"], "fdiagnostics-show-category=">, Group<f_clang_Group>;
 def fdiagnostics_show_template_tree : Flag<["-"], "fdiagnostics-show-template-tree">,
     Group<f_Group>, Flags<[CC1Option]>,
     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>;
 def fno_dwarf2_cfi_asm : Flag<["-"], "fno-dwarf2-cfi-asm">, Group<clang_ignored_f_Group>;
 def fdwarf_directory_asm : Flag<["-"], "fdwarf-directory-asm">, Group<f_Group>;
 def fno_dwarf_directory_asm : Flag<["-"], "fno-dwarf-directory-asm">, Group<f_Group>, Flags<[CC1Option]>;
 def felide_constructors : Flag<["-"], "felide-constructors">, Group<f_Group>;
 def fno_elide_type : Flag<["-"], "fno-elide-type">, Group<f_Group>,

lib/Driver/ToolChains/Clang.cpp

     CmdArgs.push_back("-fembed-bitcode=marker");
 
   // We normally speed up the clang process a bit by skipping destructors at
   // exit, but when we're generating diagnostics we can rely on some of the
   // cleanup.
   if (!C.isForDiagnostics())
     CmdArgs.push_back("-disable-free");
 
-// Disable the verification pass in -asserts builds.
+  const bool IsAssertBuild =
 #ifdef NDEBUG
-  CmdArgs.push_back("-disable-llvm-verifier");
-  // Discard LLVM value names in -asserts builds.
-  CmdArgs.push_back("-discard-value-names");
+      true;
+#else
+      false;
 #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))
+      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");
   CmdArgs.push_back(getBaseInputName(Args, Input));
 
   // Some flags which affect the language (via preprocessor
   // defines).

test/Driver/clang_f_opts.c

 // RUN: %clang -### -S -fcf-protection=return %s 2>&1 | FileCheck -check-prefix=CHECK-CF-PROTECTION-RETURN %s
 // RUN: %clang -### -S %s 2>&1 | FileCheck -check-prefix=CHECK-NO-CF-PROTECTION-RETURN %s
 // CHECK-CF-PROTECTION-RETURN: -fcf-protection=return
 // CHECK-NO-CF-PROTECTION-RETURN-NOT: -fcf-protection=return
 // RUN: %clang -### -S -fcf-protection=branch %s 2>&1 | FileCheck -check-prefix=CHECK-CF-PROTECTION-BRANCH %s
 // 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
+// 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"