Skip to content

CodeGen: Remove -disable-debug-info-print cl::opt #100319

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 25, 2024

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Jul 24, 2024

This was first introduced way back in in 2010 by
6c74a87, and has little evidence
of use. Only one test attempts to make use of this, but it's
also redundant since it's also using strip to drop debug info anyway
(and that also makes the test buggy, since it's intended to test
with and without debug info).

The other tests using it were only added to test the option after
discovering it was untested and moved, in later commits.

@arsenm arsenm marked this pull request as ready for review July 24, 2024 08:00
@llvmbot
Copy link
Member

llvmbot commented Jul 24, 2024

@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-debuginfo

Author: Matt Arsenault (arsenm)

Changes

This was first introduced way back in in 2010 by
6c74a87, and has little evidence
of use. Only one test attempts to make use of this, but it's
also redundant since it's also using strip to drop debug info anyway
(and that also makes the test buggy, since it's intended to test
with and without debug info).

The other tests using it were only added to test the option after
discovering it was untested and moved, in later commits.


Full diff: https://github.com/llvm/llvm-project/pull/100319.diff

4 Files Affected:

  • (modified) llvm/lib/CodeGen/MachineModuleInfo.cpp (+2-8)
  • (removed) llvm/test/CodeGen/Generic/disable-debug-info-print.ll (-50)
  • (removed) llvm/test/CodeGen/X86/disable-debug-info-print-codeview.ll (-19)
  • (modified) llvm/test/CodeGen/X86/frame-order.ll (+2-2)
diff --git a/llvm/lib/CodeGen/MachineModuleInfo.cpp b/llvm/lib/CodeGen/MachineModuleInfo.cpp
index b950f4fdbcf79..883d870736490 100644
--- a/llvm/lib/CodeGen/MachineModuleInfo.cpp
+++ b/llvm/lib/CodeGen/MachineModuleInfo.cpp
@@ -31,10 +31,6 @@
 using namespace llvm;
 using namespace llvm::dwarf;
 
-static cl::opt<bool>
-    DisableDebugInfoPrinting("disable-debug-info-print", cl::Hidden,
-                             cl::desc("Disable debug info printing"));
-
 // Out of line virtual method.
 MachineModuleInfoImpl::~MachineModuleInfoImpl() = default;
 
@@ -224,8 +220,7 @@ bool MachineModuleInfoWrapperPass::doInitialization(Module &M) {
         Ctx.diagnose(
             DiagnosticInfoSrcMgr(SMD, M.getName(), IsInlineAsm, LocCookie));
       });
-  MMI.DbgInfoAvailable = !DisableDebugInfoPrinting &&
-                         !M.debug_compile_units().empty();
+  MMI.DbgInfoAvailable = !M.debug_compile_units().empty();
   return false;
 }
 
@@ -250,7 +245,6 @@ MachineModuleAnalysis::run(Module &M, ModuleAnalysisManager &) {
         Ctx.diagnose(
             DiagnosticInfoSrcMgr(SMD, M.getName(), IsInlineAsm, LocCookie));
       });
-  MMI.DbgInfoAvailable =
-      !DisableDebugInfoPrinting && !M.debug_compile_units().empty();
+  MMI.DbgInfoAvailable = !M.debug_compile_units().empty();
   return Result(MMI);
 }
diff --git a/llvm/test/CodeGen/Generic/disable-debug-info-print.ll b/llvm/test/CodeGen/Generic/disable-debug-info-print.ll
deleted file mode 100644
index befa91c15d3c8..0000000000000
--- a/llvm/test/CodeGen/Generic/disable-debug-info-print.ll
+++ /dev/null
@@ -1,50 +0,0 @@
-; RUN: llc -disable-debug-info-print=true -exception-model=dwarf -o - %s | FileCheck %s
-; RUN: llc -disable-debug-info-print=true -exception-model=sjlj -o - %s | FileCheck %s --check-prefix=SJLJ-CHECK
-
-define i16 @main() nounwind !dbg !7 {
-entry:
-  ret i16 0, !dbg !9
-}
-
-define i16 @helper() !dbg !10 {
-entry:
-  ret i16 0, !dbg !11
-}
-
-
-; CHECK: main
-; CHECK-NOT: cfi_startproc
-; CHECK-NOT: .file
-; CHECK-NOT: .loc
-; CHECK: helper
-; CHECK: cfi_startproc
-; CHECK-NOT: .file
-; CHECK-NOT: .loc
-; CHECK: cfi_endproc
-
-; SJLJ-CHECK: main
-; SJLJ-CHECK-NOT: cfi_startproc
-; SJLJ-CHECK-NOT: .file
-; SJLJ-CHECK-NOT: .loc
-; SJLJ-CHECK: helper
-; SJLJ-CHECK-NOT: cfi_startproc
-; SJLJ-CHECK-NOT: .file
-; SJLJ-CHECK-NOT: .loc
-
-
-!llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!3, !4, !5}
-!llvm.ident = !{!6}
-
-!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 12.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2, splitDebugInlining: false, nameTableKind: None)
-!1 = !DIFile(filename: "unwind-tables.c", directory: "/tmp")
-!2 = !{}
-!3 = !{i32 7, !"Dwarf Version", i32 4}
-!4 = !{i32 2, !"Debug Info Version", i32 3}
-!5 = !{i32 1, !"wchar_size", i32 4}
-!6 = !{!"clang version 12.0.0"}
-!7 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 1, type: !8, scopeLine: 1, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
-!8 = !DISubroutineType(types: !2)
-!9 = !DILocation(line: 2, column: 3, scope: !7)
-!10 = distinct !DISubprogram(name: "helper", scope: !1, file: !1, line: 1, type: !8, scopeLine: 1, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
-!11 = !DILocation(line: 2, column: 3, scope: !10)
diff --git a/llvm/test/CodeGen/X86/disable-debug-info-print-codeview.ll b/llvm/test/CodeGen/X86/disable-debug-info-print-codeview.ll
deleted file mode 100644
index 930dafc119b3c..0000000000000
--- a/llvm/test/CodeGen/X86/disable-debug-info-print-codeview.ll
+++ /dev/null
@@ -1,19 +0,0 @@
-; RUN: llc -disable-debug-info-print -o - %s | FileCheck %s
-
-; Check that debug info isn't emitted for CodeView with
-; -disable-debug-info-print.
-
-; CHECK-NOT:      CodeViewTypes
-; CHECK-NOT:      CodeViewDebugInfo
-
-source_filename = "empty"
-target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-pc-windows-msvc19.0.24215"
-
-!llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!2, !3}
-
-!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "clang", emissionKind: FullDebug)
-!1 = !DIFile(filename: "empty", directory: "path/to")
-!2 = !{i32 2, !"CodeView", i32 1}
-!3 = !{i32 2, !"Debug Info Version", i32 3}
diff --git a/llvm/test/CodeGen/X86/frame-order.ll b/llvm/test/CodeGen/X86/frame-order.ll
index 2857a738e8efc..dcbcb481f927c 100644
--- a/llvm/test/CodeGen/X86/frame-order.ll
+++ b/llvm/test/CodeGen/X86/frame-order.ll
@@ -1,5 +1,5 @@
-; RUN: llc -mtriple=x86_64-linux-gnueabi -disable-debug-info-print < %s | FileCheck %s
-; RUN: opt -passes=strip -S < %s | llc -mtriple=x86_64-linux-gnueabi -disable-debug-info-print | FileCheck %s
+; RUN: llc -mtriple=x86_64-linux-gnueabi < %s | FileCheck %s
+; RUN: opt -passes=strip -S < %s | llc -mtriple=x86_64-linux-gnueabi | FileCheck %s
 
 ; This test checks if the code is generated correctly with and without debug info.
 

@dwblaikie
Copy link
Collaborator

Generally seems OK to me - though did find this #44225 recent (well, 2020) issue using it to debug debug-info-affects-codegen bugs, which makes sense (can straight up diff the output, since both sides don't have any debug info, which is a bit easier to check than other ways).

Myabe @jmorse and co might have opinions on whether they want this sort of thing for any ongoing work on the debug info intrinsics migration, etc.

@arsenm arsenm force-pushed the users/arsenm/remove-disable-debug-info-print branch from 167cea6 to 6f82349 Compare July 24, 2024 18:30
@vtjnash
Copy link
Member

vtjnash commented Jul 24, 2024

I have never used it or needed it, so I am not a good reviewer for it. I had thought it was a strange feature flag, but I had been forced to preserve it when I touched code in that area because of both tests and user comments when I accidentally broke it in https://reviews.llvm.org/D89613#2336529. So I am in favor of removing it, as it seems undocumented and minimally tested, but I was aware that some folks seemed to find it useful.

Copy link
Contributor

@s-barannikov s-barannikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me

arsenm added 2 commits July 25, 2024 11:02
This was first introduced way back in in 2010 by
6c74a87, and has little evidence
of use. Only one test attempts to make use of this, but it's
also redundant since it's also using strip to drop debug info anyway
(and that also makes the test buggy, since it's intended to test
with and without debug info).

The other tests using it were only added to test the option after
discovering it was untested and moved, in later commits.
@arsenm arsenm force-pushed the users/arsenm/remove-disable-debug-info-print branch from 6f82349 to 50c0a81 Compare July 25, 2024 07:10
@arsenm arsenm merged commit af1d2b9 into main Jul 25, 2024
7 checks passed
@arsenm arsenm deleted the users/arsenm/remove-disable-debug-info-print branch July 25, 2024 12:39
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
This was first introduced way back in in 2010 by
6c74a87, and has little evidence
of use. Only one test attempts to make use of this, but it's
also redundant since it's also using strip to drop debug info anyway
(and that also makes the test buggy, since it's intended to test
with and without debug info).

The other tests using it were only added to test the option after
discovering it was untested and moved, in later commits.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60250578
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants