[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro marked 2 inline comments as done.
djtodoro added a comment.

In D73534#1864855 <https://reviews.llvm.org/D73534#1864855>, @vsk wrote:

> Hm, does this turn on entry values at -O0? Could you please add a clang test that ensures that at -O0 + -gdwarf5 + lldb/gdb tuning, entry values are not emitted?


We do not generate the debug entry values for the `-O0`. The test is added.



================
Comment at: llvm/include/llvm/Target/TargetOptions.h:262
+    unsigned SupportsDebugEntryValues : 1;
+    ///  Emit the debug entry values.
     unsigned EnableDebugEntryValues : 1;
----------------
vsk wrote:
> Better description might be: `When set to true, the EnableDebugEntryValues option forces production of debug entry values even if the target does not officially support it. Useful for testing purposes only. This flag should never be checked directly, always use \ref ShouldEmitDebugEntryValues instead.`
Very good, thanks! :)


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:840
+      if (Asm->TM.Options.ShouldEmitDebugEntryValues() &&
           (tuneForGDB() || tuneForLLDB())) {
         ParamSet Params;
----------------
probinson wrote:
> This implementation makes it impractical to enable entry-values while still using SCE tuning.  The Sony debugger people will want to be able to do that.
> 
> And in fact this is not how tuning controls are supposed to work.  There should be a flag in DwarfDebug, initialized in the ctor, which has a *default* based on the tuning; but the flag can still be overridden regardless of tuning, by a command-line option just as many other tuning-related options are handled in the DwarfDebug ctor.
> 
> Based on comments by @vsk on the llvm-dev thread, I think you are trying to track all of the following conditions: (1) the target supports *tracking* entry values; (2) someone wants to *track* entry values even if the target doesn't support it by default; (3) whether to *emit* entry values in the DWARF.  Currently (3) is not user-selectable because it is hard-coded based on tuning; this is a bug.
> 
> I would also rather not have two separate command-line options for (2) and (3), but if that's what we need, so be it.
@probinson Thanks for the comment! I agree.

>I would also rather not have two separate command-line options for (2) and (3), but if that's what we need, so be it.
I think (2) and (3) are different, so I've introduced the `-emit-debug-entry-values` within `DwarfDebug`.






CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73534/new/

https://reviews.llvm.org/D73534