[DWARF] Avoid entry_values production for SCE

[djtodoro]

Summary

SONY debugger does not prefer debug entry values feature, so the plan is to avoid production of the entry values by default when the tuning is SCE debugger.

The feature still can be enabled with the -debug-entry-values option for the testing/development purposes.

This patch addresses PR46643.

[probinson]

Between this code and DwarfDebug, I'm having a hard time understanding the various controls over emitting entry-values.

• There's an existing tuning check in DwarfDebug, now you're adding the complementary check in TargetOptions. Does this make the DwarfDebug check redundant?
• There are 3 different flags to sort out whether to emit this stuff. SupportsDebugEntryValues is strictly based on the target, I get that. Then there's EnableDebugEntryValues in the TargetOptions, which seems to be tied to a tool-command-line option; and separately there's EmitDwarfDebugEntryValues in DwarfDebug, with its own command-line option. Can we simplify this at all?

[djtodoro]

In D83462#2141719, @probinson wrote:

Between this code and DwarfDebug, I'm having a hard time understanding the various controls over emitting entry-values.

I see.. I think we can/should simplify it.

• There's an existing tuning check in DwarfDebug, now you're adding the complementary check in TargetOptions. Does this make the DwarfDebug check redundant?

It is is redundant now.. Thanks!

• There are 3 different flags to sort out whether to emit this stuff. SupportsDebugEntryValues is strictly based on the target, I get that. Then there's EnableDebugEntryValues in the TargetOptions, which seems to be tied to a tool-command-line option; and separately there's EmitDwarfDebugEntryValues in DwarfDebug, with its own command-line option. Can we simplify this at all?

I think yes. We needed the first two (SupportsDebugEntryValues and EnableDebugEntryValues from TargetOptions) in order to enable it by default for some targets and to leave an experimental option for developers who want to extend the support for the other architectures. There is a situation when different debuggers come into the game, so we thought to control it on the very end (within DwarfDebug with the EmitDwarfDebugEntryValues), but at that place we can control only caller-side-dwarf-symbols (dw_tag_call_sites) but not the dw_entry_values (callee side; since it is being produced during LiveDebugValues phase). So, having this patch, we can get rid of the DwarfDebug::EmitDwarfDebugEntryValues option, since we can experiment everything with the fields from TargetOptions.

[djtodoro]

• Remove unnecessary DwarfDebug entry-values option

[vsk]

+1 for removing EmitDwarfDebugEntryValues, that's a nice cleanup. Thanks!

[echristo]

So the tuning here for SCE is also a "does not support" or something else?

[probinson]

In D83462#2142036, @echristo wrote:

So the tuning here for SCE is also a "does not support" or something else?

I'm told our debugger currently does not support the entry-value opcode. Locations with that opcode would be dropped/ignored (presumably just the individual location-list elements, although I haven't verified).

I am nudging our folks in the direction of supporting it, but need to demonstrate value first; that the additional location descriptions increase availability, and that the expressions can be evaluated usefully. I have to say a few preliminary experiments don't make me feel too positive about that second part, as the entry-value expressions tend to rely on registers that aren't saved by the ABI, so unwinding won't recover them.

In the meantime I'd prefer that they weren't emitted. for the usual size reasons.

[djtodoro]

• Update the comment around ShouldEmitDebugEntryValues()

[aprantl]

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

429

Wouldn't we want (tuneForGDB() || tuneForLLDB()) to override the target default? If someone were to debug with a hypothetical GDB port for SCE targets, for example?

llvm/test/DebugInfo/MIR/X86/DW_OP_entry_value.mir

18

Wouldn't you want to check for DW_OP_{{.*}}entry_value?

[djtodoro]

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

429

We can still do that by using the -debug-entry-values option?

llvm/test/DebugInfo/MIR/X86/DW_OP_entry_value.mir

18

It makes sense, thanks! The default @entry encoding for SCE will be GNU in this case, but it is safe to check both in the case the behavior changes.

[djtodoro]

• Improve the test

[djtodoro]

Any additional comment on this? :)

[probinson]

LGTM

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

429

The tuning decision is made in TargetOptionsImpl, which is unusual but that's where the tuning option is actually kept, so I think it's okay.

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

 static cl::opt<bool>
 DisableDebugInfoPrinting("disable-debug-info-print", cl::Hidden,
                          cl::desc("Disable debug info printing"));
 
 static cl::opt<bool> UseDwarfRangesBaseAddressSpecifier(
     "use-dwarf-ranges-base-address-specifier", cl::Hidden,
     cl::desc("Use base address specifiers in debug_ranges"), cl::init(false));
 
-static cl::opt<bool> EmitDwarfDebugEntryValues(
-    "emit-debug-entry-values", cl::Hidden,
-    cl::desc("Emit the debug entry values"), cl::init(false));
-
 static cl::opt<bool> GenerateARangeSection("generate-arange-section",
                                            cl::Hidden,
                                            cl::desc("Generate dwarf aranges"),
                                            cl::init(false));
 
 static cl::opt<bool>
     GenerateDwarfTypeUnits("generate-type-units", cl::Hidden,
                            cl::desc("Generate DWARF4 type units."),
DwarfDebug::DwarfDebug(AsmPrinter *A, Module *M)
   // The DWARF v5 string offsets table has - possibly shared - contributions
   // from each compile and type unit each preceded by a header. The string
   // offsets table used by the pre-DWARF v5 split-DWARF implementation uses
   // a monolithic string offsets table without any header.
   UseSegmentedStringOffsetsTable = DwarfVersion >= 5;
 
   // Emit call-site-param debug info for GDB and LLDB, if the target supports
   // the debug entry values feature. It can also be enabled explicitly.
-  EmitDebugEntryValues = (Asm->TM.Options.ShouldEmitDebugEntryValues() &&
-                          (tuneForGDB() || tuneForLLDB())) ||
-                         EmitDwarfDebugEntryValues;
+  EmitDebugEntryValues = Asm->TM.Options.ShouldEmitDebugEntryValues();
 
   Asm->OutStreamer->getContext().setDwarfVersion(DwarfVersion);
 }
 
 // Define out of line so we don't have to include DwarfUnit.h in DwarfDebug.h.
 DwarfDebug::~DwarfDebug() = default;
 
 static bool isObjCClass(StringRef Name) {

llvm/lib/CodeGen/TargetOptionsImpl.cpp

 
 /// HonorSignDependentRoundingFPMath - Return true if the codegen must assume
 /// that the rounding mode of the FPU can change from its default.
 bool TargetOptions::HonorSignDependentRoundingFPMath() const {
   return !UnsafeFPMath && HonorSignDependentRoundingFPMathOption;
 }
 
 /// NOTE: There are targets that still do not support the debug entry values
-/// production.
+/// production and that is being controlled with the SupportsDebugEntryValues.
+/// In addition, SCE debugger does not have the feature implemented, so prefer
+/// not to emit the debug entry values in that case.
+/// The EnableDebugEntryValues can be used for the testing purposes.
 bool TargetOptions::ShouldEmitDebugEntryValues() const {
-  return SupportsDebugEntryValues || EnableDebugEntryValues;
+  return (SupportsDebugEntryValues && DebuggerTuning != DebuggerKind::SCE) ||
+         EnableDebugEntryValues;
 }

llvm/test/DebugInfo/MIR/X86/DW_OP_entry_value.mir

 # RUN: llc -start-before=livedebugvalues -mtriple=x86_64-apple-darwin -o %t %s -filetype=obj
 # RUN: llvm-dwarfdump %t | FileCheck %s
-#
-# int global;
-# int foo(int p, int q, int r) {
-#   global = p + 1;
-#   asm __volatile("" : : : "edi", "esi", "edx");
-#   return 123;
-# }
+
+# RUN: llc -start-before=livedebugvalues -debugger-tune=sce -mtriple=x86_64-sce-ps4 -o %t1 %s -filetype=obj
+# RUN: llvm-dwarfdump %t1 | FileCheck %s -check-prefix=SCE
+
+## Based on:
+## int global;
+## int foo(int p, int q, int r) {
+##   global = p + 1;
+##   asm __volatile("" : : : "edi", "esi", "edx");
+##   return 123;
+## }
 
 # CHECK: DW_TAG_formal_parameter
 # CHECK:   DW_OP_entry_value
 
+# SCE-NOT: DW_OP_GNU_entry_value
+
 --- |
   ; ModuleID = 'multiple-param-dbg-value-entry.ll'
   source_filename = "multiple-param-dbg-value-entry.c"
   target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
   @global = common dso_local local_unnamed_addr global i32 0, align 4, !dbg !0
 
   define dso_local i32 @foo(i32 %p, i32 %q, i32 %r) local_unnamed_addr !dbg !11 {

llvm/test/DebugInfo/MIR/X86/call-site-gnu-vs-dwarf5-attrs.mir

 # RUN:     | llvm-dwarfdump - | FileCheck %s -check-prefixes=CHECK-DWARF5 -implicit-check-not=DW_AT_call
 #
 # === DWARF5, tune for lldb ===
 # RUN: llc -dwarf-version 5 -debugger-tune=lldb -emit-call-site-info -filetype=obj \
 # RUN:     -mtriple=x86_64-unknown-unknown -start-after=machineverifier -o - %s   \
 # RUN:     | llvm-dwarfdump - | FileCheck %s -check-prefixes=CHECK-DWARF5 -implicit-check-not=DW_AT_call
 #
 # RUN: llc -emit-call-site-info -dwarf-version 5 -filetype=obj -debugger-tune=sce \
-# RUN:     -emit-debug-entry-values -debug-entry-values -mtriple=x86_64-unknown-unknown \
+# RUN:     -debug-entry-values -mtriple=x86_64-unknown-unknown \
 # RUN:     -start-after=machineverifier -o - %s | llvm-dwarfdump - | FileCheck %s -check-prefixes=CHECK-DWARF5
 #
 # This is based on the following reproducer:
 #
 # extern void fn();
 # extern void fn2(int x);
 # extern int fn3();
 #