Revert "[PS4/PS5][Driver][DWARF] Always emit .debug_aranges for SCE t… #99711
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…uning (llvm#99629)" This reverts commit 22eb290.
llvmbot
added
clang
|
@llvm/pr-subscribers-debuginfo @llvm/pr-subscribers-clang-driver Author: Edd Dawson (playstation-edd) Changes…uning (#99629)" This reverts commit 22eb290. Full diff: https://github.com/llvm/llvm-project/pull/99711.diff 8 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index abe517f0f5dea..f7b987bf810c1 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4659,8 +4659,11 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T,
// -gdwarf-aranges turns on the emission of the aranges section in the
// backend.
- if (const Arg *A = Args.getLastArg(options::OPT_gdwarf_aranges);
- A && checkDebugInfoOption(A, Args, D, TC)) {
+ // Always enabled for SCE tuning.
+ bool NeedAranges = DebuggerTuning == llvm::DebuggerKind::SCE;
+ if (const Arg *A = Args.getLastArg(options::OPT_gdwarf_aranges))
+ NeedAranges = checkDebugInfoOption(A, Args, D, TC) || NeedAranges;
+ if (NeedAranges) {
CmdArgs.push_back("-mllvm");
CmdArgs.push_back("-generate-arange-section");
}
diff --git a/clang/lib/Driver/ToolChains/PS4CPU.cpp b/clang/lib/Driver/ToolChains/PS4CPU.cpp
index d6af9388e54a6..974e486a0082b 100644
--- a/clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ b/clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -162,6 +162,10 @@ void tools::PS4cpu::Linker::ConstructJob(Compilation &C, const JobAction &JA,
};
if (UseLTO) {
+ // We default to creating the arange section, but LTO does not. Enable it
+ // here.
+ AddCodeGenFlag("-generate-arange-section");
+
// This tells LTO to perform JustMyCode instrumentation.
if (UseJMC)
AddCodeGenFlag("-enable-jmc-instrument");
@@ -268,6 +272,10 @@ void tools::PS5cpu::Linker::ConstructJob(Compilation &C, const JobAction &JA,
};
if (UseLTO) {
+ // We default to creating the arange section, but LTO does not. Enable it
+ // here.
+ AddCodeGenFlag("-generate-arange-section");
+
// This tells LTO to perform JustMyCode instrumentation.
if (UseJMC)
AddCodeGenFlag("-enable-jmc-instrument");
diff --git a/clang/test/Driver/debug-options.c b/clang/test/Driver/debug-options.c
index 21785ba01cb41..0a665f7017d63 100644
--- a/clang/test/Driver/debug-options.c
+++ b/clang/test/Driver/debug-options.c
@@ -118,28 +118,27 @@
// RUN: %clang_cl -### -c -Z7 -target x86_64-windows-msvc -- %s 2>&1 \
// RUN: | FileCheck -check-prefix=G_NOTUNING %s
-// On the PS4/PS5, -g defaults to -gno-column-info. We default to always
-// generating the arange section, but keyed off SCE DebuggerTuning being in
-// play during codegen, instead of -generate-arange-section.
+// On the PS4/PS5, -g defaults to -gno-column-info, and we always generate the
+// arange section.
// RUN: %clang -### -c %s -target x86_64-scei-ps4 2>&1 \
// RUN: | FileCheck -check-prefix=NOG_PS %s
// RUN: %clang -### -c %s -target x86_64-sie-ps5 2>&1 \
// RUN: | FileCheck -check-prefix=NOG_PS %s
/// PS4 will stay on v4 even if the generic default version changes.
// RUN: %clang -### -c %s -g -target x86_64-scei-ps4 2>&1 \
-// RUN: | FileCheck -check-prefixes=G_DWARF4,G_SCE,NOCI,FWD_TMPL_PARAMS %s
+// RUN: | FileCheck -check-prefixes=G_DWARF4,GARANGE,G_SCE,NOCI,FWD_TMPL_PARAMS %s
// RUN: %clang -### -c %s -g -target x86_64-sie-ps5 2>&1 \
-// RUN: | FileCheck -check-prefixes=G_DWARF5,G_SCE,NOCI,FWD_TMPL_PARAMS %s
+// RUN: | FileCheck -check-prefixes=G_DWARF5,GARANGE,G_SCE,NOCI,FWD_TMPL_PARAMS %s
// RUN: %clang -### -c %s -g -gcolumn-info -target x86_64-scei-ps4 2>&1 \
// RUN: | FileCheck -check-prefix=CI %s
// RUN: %clang -### -c %s -gsce -target x86_64-unknown-linux 2>&1 \
// RUN: | FileCheck -check-prefix=NOCI %s
// RUN: %clang -### %s -g -flto=thin -target x86_64-scei-ps4 2>&1 \
-// RUN: | FileCheck -check-prefix=LDGARANGE %s
+// RUN: | FileCheck -check-prefix=SNLDTLTOGARANGE %s
// RUN: %clang -### %s -g -flto=full -target x86_64-scei-ps4 2>&1 \
-// RUN: | FileCheck -check-prefix=LDGARANGE %s
+// RUN: | FileCheck -check-prefix=SNLDFLTOGARANGE %s
// RUN: %clang -### %s -g -flto -target x86_64-scei-ps5 2>&1 \
-// RUN: | FileCheck -check-prefix=LDGARANGE %s
+// RUN: | FileCheck -check-prefix=LLDGARANGE %s
// RUN: %clang -### %s -g -target x86_64-scei-ps5 2>&1 \
// RUN: | FileCheck -check-prefix=LDGARANGE %s
@@ -322,7 +321,8 @@
//
// NOG_PS: "-cc1"
// NOG_PS-NOT: "-dwarf-version=
-// NOG_PS-NOT: "-generate-arange-section"
+// NOG_PS: "-generate-arange-section"
+// NOG_PS-NOT: "-dwarf-version=
//
// G_ERR: error: unknown argument:
//
@@ -402,7 +402,8 @@
//
// LDGARANGE: {{".*ld.*"}} {{.*}}
-// LDGARANGE-NOT: -generate-arange-section"
+// LDGARANGE-NOT: "-plugin-opt=-generate-arange-section"
+// LLDGARANGE: {{".*lld.*"}} {{.*}} "-plugin-opt=-generate-arange-section"
// SNLDTLTOGARANGE: {{".*orbis-ld.*"}} {{.*}} "-lto-thin-debug-options= -generate-arange-section"
// SNLDFLTOGARANGE: {{".*orbis-ld.*"}} {{.*}} "-lto-debug-options= -generate-arange-section"
diff --git a/clang/test/Driver/lto-jobs.c b/clang/test/Driver/lto-jobs.c
index b4f109e4c502c..43a478b0664d8 100644
--- a/clang/test/Driver/lto-jobs.c
+++ b/clang/test/Driver/lto-jobs.c
@@ -11,7 +11,7 @@
// RUN: %clang --target=x86_64-scei-ps4 -### %s -flto=thin -flto-jobs=5 2> %t
// RUN: FileCheck -check-prefix=CHECK-PS4-LINK-THIN-JOBS-ACTION < %t %s
//
-// CHECK-PS4-LINK-THIN-JOBS-ACTION: "-lto-thin-debug-options= -threads=5"
+// CHECK-PS4-LINK-THIN-JOBS-ACTION: "-lto-thin-debug-options= -generate-arange-section -threads=5"
// RUN: %clang --target=x86_64-apple-darwin13.3.0 -### %s -flto=thin -flto-jobs=5 2> %t
// RUN: FileCheck -check-prefix=CHECK-LINK-THIN-JOBS2-ACTION < %t %s
diff --git a/clang/test/Driver/ps4-linker.c b/clang/test/Driver/ps4-linker.c
index be989cdd7d5b1..be0103bffe813 100644
--- a/clang/test/Driver/ps4-linker.c
+++ b/clang/test/Driver/ps4-linker.c
@@ -5,8 +5,8 @@
// RUN: %clang --target=x86_64-scei-ps4 -flto=full -fjmc %s -### 2>&1 | FileCheck --check-prefixes=CHECK-FULL-LTO,CHECK-LIB %s
// CHECK-NOT: -enable-jmc-instrument
-// CHECK-THIN-LTO: "-lto-thin-debug-options= -enable-jmc-instrument"
-// CHECK-FULL-LTO: "-lto-debug-options= -enable-jmc-instrument"
+// CHECK-THIN-LTO: "-lto-thin-debug-options= -generate-arange-section -enable-jmc-instrument"
+// CHECK-FULL-LTO: "-lto-debug-options= -generate-arange-section -enable-jmc-instrument"
// Check the default library name.
// CHECK-LIB: "--whole-archive" "-lSceDbgJmc" "--no-whole-archive"
@@ -16,5 +16,5 @@
// RUN: %clang --target=x86_64-scei-ps4 -flto=thin -fcrash-diagnostics-dir=mydumps %s -### 2>&1 | FileCheck --check-prefixes=CHECK-DIAG-THIN-LTO %s
// RUN: %clang --target=x86_64-scei-ps4 -flto=full -fcrash-diagnostics-dir=mydumps %s -### 2>&1 | FileCheck --check-prefixes=CHECK-DIAG-FULL-LTO %s
-// CHECK-DIAG-THIN-LTO: "-lto-thin-debug-options= -crash-diagnostics-dir=mydumps"
-// CHECK-DIAG-FULL-LTO: "-lto-debug-options= -crash-diagnostics-dir=mydumps"
+// CHECK-DIAG-THIN-LTO: "-lto-thin-debug-options= -generate-arange-section -crash-diagnostics-dir=mydumps"
+// CHECK-DIAG-FULL-LTO: "-lto-debug-options= -generate-arange-section -crash-diagnostics-dir=mydumps"
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 578a90d402c54..80cd5ec501f25 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -354,9 +354,6 @@ DwarfDebug::DwarfDebug(AsmPrinter *A)
UseLocSection = !TT.isNVPTX();
- // Always emit .debug_aranges for SCE tuning.
- UseARangesSection = GenerateARangeSection || tuneForSCE();
-
HasAppleExtensionAttributes = tuneForLLDB();
// Handle split DWARF.
@@ -1453,7 +1450,7 @@ void DwarfDebug::endModule() {
emitDebugInfo();
// Emit info into a debug aranges section.
- if (UseARangesSection)
+ if (GenerateARangeSection)
emitDebugARanges();
// Emit info into a debug ranges section.
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
index 13f4c379e0027..452485b632c45 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
@@ -435,9 +435,6 @@ class DwarfDebug : public DebugHandlerBase {
///Allow emission of the .debug_loc section.
bool UseLocSection = true;
- /// Allow emission of .debug_aranges section
- bool UseARangesSection = false;
-
/// Generate DWARF v4 type units.
bool GenerateTypeUnits;
diff --git a/llvm/test/DebugInfo/X86/debug-aranges-sce-tuning.ll b/llvm/test/DebugInfo/X86/debug-aranges-sce-tuning.ll
deleted file mode 100644
index ac0d61aef2b1c..0000000000000
--- a/llvm/test/DebugInfo/X86/debug-aranges-sce-tuning.ll
+++ /dev/null
@@ -1,33 +0,0 @@
-; This checks that .debug_aranges is always generated for the SCE debugger
-; tuning.
-
-; RUN: llc -mtriple=x86_64 -debugger-tune=sce -filetype=obj %s -o %t
-; RUN: llvm-dwarfdump -debug-aranges %t | FileCheck %s
-
-; CHECK: .debug_aranges contents:
-; CHECK-NEXT: Address Range Header:
-; CHECK-SAME: length = 0x0000002c,
-
-; IR generated and reduced from:
-; $ cat foo.c
-; int foo;
-; $ clang -g -S -emit-llvm foo.c -o foo.ll
-
-target triple = "x86_64-unknown-linux-gnu"
-
-@foo = dso_local global i32 0, align 4, !dbg !0
-
-!llvm.dbg.cu = !{!2}
-!llvm.module.flags = !{!6, !7, !8}
-!llvm.ident = !{!9}
-
-!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
-!1 = distinct !DIGlobalVariable(name: "foo", scope: !2, file: !3, line: 1, type: !5, isLocal: false, isDefinition: true)
-!2 = distinct !DICompileUnit(language: DW_LANG_C11, file: !3, producer: "clang version 19.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, globals: !4, splitDebugInlining: false, nameTableKind: None)
-!3 = !DIFile(filename: "foo.c", directory: "/tmp")
-!4 = !{!0}
-!5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-!6 = !{i32 7, !"Dwarf Version", i32 5}
-!7 = !{i32 2, !"Debug Info Version", i32 3}
-!8 = !{i32 1, !"wchar_size", i32 4}
-!9 = !{!"clang version 19.0.0"}
|
|
For future reverts, consider adding some rationale :) |
|
Apologies - will do. For the record, there was a failure on builder llvm-clang-x86_64-sie-win. |
|
also, if you're sending a pull request just because it's convenient (via the web UI "revert" button) or to run presubmits but not for review, please include the skip-presubmit-approval label so it's clear something isn't getting committed without needed review |
playstation-edd
added a commit
to playstation-edd/llvm-project
that referenced
this pull request
Jul 23, 2024
Some of SIE's post-mortem analysis infrastructure currently makes use of .debug_aranges, so we'd like to ensure the section's presence in PlayStation binaries. The simplest way to do this is to force emission when the debugger tuning is set to SCE (which is in turn typically initialized from the target triple). This also simplifies the driver. llvm/test/DebugInfo/debuglineinfo-path.ll has been marked as UNSUPPORTED on PlayStation. When aranges are emitted, the DWARF in the test case is such that relocations need to be applied to the aranges section in order for symbolization to work. An alternative approach would be to implement the application of relocations in DWARFDebugArangeSet. While experiments show that this can be made to work with a modest patch, the test cases would be rather contrived. Since I expect the only utility for such a change would be to make this test case pass for PlayStation targets, and few - if any - outside of PlayStation care about aranges, UNSUPPORTED would seem to be a more practical option. This was originally commited as 22eb290 (llvm#99629) and later reverted at 84658fb (llvm#99711) due to test failures on SIE built bots. These failures shouldn't recur due to 3b24e5d (llvm#99897) and the aforementioned change to debuglineinfo-path.ll. SIE tracker: TOOLCHAIN-16951
playstation-edd
added a commit
that referenced
this pull request
Jul 24, 2024
…100160) Some of SIE's post-mortem analysis infrastructure currently makes use of .debug_aranges, so we'd like to ensure the section's presence in PlayStation binaries. The simplest way to do this is to force emission when the debugger tuning is set to SCE (which is in turn typically initialized from the target triple). This also simplifies the driver. llvm/test/DebugInfo/debuglineinfo-path.ll has been marked as UNSUPPORTED on PlayStation. When aranges are emitted, the DWARF in the test case is such that relocations need to be applied to the aranges section in order for symbolization to work. An alternative approach would be to implement the application of relocations in DWARFDebugArangeSet. While experiments show that this can be made to work with a modest patch, the test cases would be rather contrived. Since I expect the only utility for such a change would be to make this test case pass for PlayStation targets, and few - if any - outside of PlayStation care about aranges, UNSUPPORTED would seem to be a more practical option. This was originally commited as 22eb290 (#99629) and later reverted at 84658fb (#99711) due to test failures on SIE built bots. These failures shouldn't recur due to 3b24e5d (#99897) and the aforementioned change to debuglineinfo-path.ll. SIE tracker: TOOLCHAIN-16951
yuxuanchen1997
pushed a commit
that referenced
this pull request
Jul 25, 2024
#99711) Summary: …uning (#99629)" This reverts commit 22eb290. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251322
yuxuanchen1997
pushed a commit
that referenced
this pull request
Jul 25, 2024
…100160) Summary: Some of SIE's post-mortem analysis infrastructure currently makes use of .debug_aranges, so we'd like to ensure the section's presence in PlayStation binaries. The simplest way to do this is to force emission when the debugger tuning is set to SCE (which is in turn typically initialized from the target triple). This also simplifies the driver. llvm/test/DebugInfo/debuglineinfo-path.ll has been marked as UNSUPPORTED on PlayStation. When aranges are emitted, the DWARF in the test case is such that relocations need to be applied to the aranges section in order for symbolization to work. An alternative approach would be to implement the application of relocations in DWARFDebugArangeSet. While experiments show that this can be made to work with a modest patch, the test cases would be rather contrived. Since I expect the only utility for such a change would be to make this test case pass for PlayStation targets, and few - if any - outside of PlayStation care about aranges, UNSUPPORTED would seem to be a more practical option. This was originally commited as 22eb290 (#99629) and later reverted at 84658fb (#99711) due to test failures on SIE built bots. These failures shouldn't recur due to 3b24e5d (#99897) and the aforementioned change to debuglineinfo-path.ll. SIE tracker: TOOLCHAIN-16951 Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60250743
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…uning (#99629)"
This reverts commit 22eb290.