Revert "[DebugInfo] Add flag to only emit referenced member functions" #93767

joker-eph · 2024-05-30T04:48:10Z

Reverts #87018

MacOS and Windows bots are broken.

…#87018)" This reverts commit bfabc95.

llvmbot · 2024-05-30T04:48:38Z

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang-codegen

Author: Mehdi Amini (joker-eph)

Changes

Reverts llvm/llvm-project#87018

MacOS and Windows bots are broken.

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

6 Files Affected:

(modified) clang/include/clang/Basic/DebugOptions.def (-2)
(modified) clang/include/clang/Driver/Options.td (-4)
(modified) clang/lib/CodeGen/CGDebugInfo.cpp (+1-1)
(modified) clang/lib/Driver/ToolChains/Clang.cpp (-15)
(removed) clang/test/CodeGenCXX/debug-info-incomplete-types.cpp (-12)
(modified) clang/test/Driver/debug-options.c (-8)

diff --git a/clang/include/clang/Basic/DebugOptions.def b/clang/include/clang/Basic/DebugOptions.def
index bc96d5dfdf890..b94f6aef9ac60 100644
--- a/clang/include/clang/Basic/DebugOptions.def
+++ b/clang/include/clang/Basic/DebugOptions.def
@@ -68,8 +68,6 @@ BENIGN_DEBUGOPT(NoInlineLineTables, 1, 0) ///< Whether debug info should contain
                                           ///< inline line tables.
 
 DEBUGOPT(DebugStrictDwarf, 1, 1) ///< Whether or not to use strict DWARF info.
-DEBUGOPT(DebugOmitUnreferencedMethods, 1, 0) ///< Omit unreferenced member
-					     ///< functions in type debug info.
 
 /// Control the Assignment Tracking debug info feature.
 BENIGN_ENUM_DEBUGOPT(AssignmentTrackingMode, AssignmentTrackingOpts, 2,
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index f64d7c60783e9..4119e69c85540 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4345,10 +4345,6 @@ defm strict_dwarf : BoolOption<"g", "strict-dwarf",
           "the specified version, avoiding features from later versions.">,
   NegFlag<SetFalse>, BothFlags<[], [ClangOption, CLOption, DXCOption]>>,
   Group<g_flags_Group>;
-defm omit_unreferenced_methods : BoolGOption<"omit-unreferenced-methods",
-  CodeGenOpts<"DebugOmitUnreferencedMethods">, DefaultFalse,
-  NegFlag<SetFalse>,
-  PosFlag<SetTrue, [], [CC1Option]>, BothFlags<[], [ClangOption, CLOption, DXCOption]>>;
 defm column_info : BoolOption<"g", "column-info",
   CodeGenOpts<"DebugColumnInfo">, DefaultTrue,
   NegFlag<SetFalse, [], [ClangOption, CC1Option]>,
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 5f6f911c7a6d6..9d7107abf8a6f 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2836,7 +2836,7 @@ CGDebugInfo::CreateTypeDefinition(const RecordType *Ty) {
 
   // Collect data fields (including static variables and any initializers).
   CollectRecordFields(RD, DefUnit, EltTys, FwdDecl);
-  if (CXXDecl && !CGM.getCodeGenOpts().DebugOmitUnreferencedMethods)
+  if (CXXDecl)
     CollectCXXMemberFunctions(CXXDecl, DefUnit, EltTys, FwdDecl);
 
   LexicalBlockStack.pop_back();
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 4e1c52462e584..97e451cfe2acb 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -45,7 +45,6 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/BinaryFormat/Magic.h"
 #include "llvm/Config/llvm-config.h"
-#include "llvm/Frontend/Debug/Options.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CodeGen.h"
@@ -4643,7 +4642,6 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T,
   Args.addOptInFlag(CmdArgs, options::OPT_fforce_dwarf_frame,
                     options::OPT_fno_force_dwarf_frame);
 
-  bool EnableTypeUnits = false;
   if (Args.hasFlag(options::OPT_fdebug_types_section,
                    options::OPT_fno_debug_types_section, false)) {
     if (!(T.isOSBinFormatELF() || T.isOSBinFormatWasm())) {
@@ -4654,24 +4652,11 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T,
     } else if (checkDebugInfoOption(
                    Args.getLastArg(options::OPT_fdebug_types_section), Args, D,
                    TC)) {
-      EnableTypeUnits = true;
       CmdArgs.push_back("-mllvm");
       CmdArgs.push_back("-generate-type-units");
     }
   }
 
-  if (const Arg *A =
-          Args.getLastArg(options::OPT_gomit_unreferenced_methods,
-                          options::OPT_gno_omit_unreferenced_methods))
-    (void)checkDebugInfoOption(A, Args, D, TC);
-  if (Args.hasFlag(options::OPT_gomit_unreferenced_methods,
-                   options::OPT_gno_omit_unreferenced_methods, false) &&
-      (DebugInfoKind == llvm::codegenoptions::DebugInfoConstructor ||
-       DebugInfoKind == llvm::codegenoptions::LimitedDebugInfo) &&
-      !EnableTypeUnits) {
-    CmdArgs.push_back("-gomit-unreferenced-methods");
-  }
-
   // To avoid join/split of directory+filename, the integrated assembler prefers
   // the directory form of .file on all DWARF versions. GNU as doesn't allow the
   // form before DWARF v5.
diff --git a/clang/test/CodeGenCXX/debug-info-incomplete-types.cpp b/clang/test/CodeGenCXX/debug-info-incomplete-types.cpp
deleted file mode 100644
index 0bf59233b4e2e..0000000000000
--- a/clang/test/CodeGenCXX/debug-info-incomplete-types.cpp
+++ /dev/null
@@ -1,12 +0,0 @@
-// RUN: %clang_cc1 -debug-info-kind=limited -gomit-unreferenced-methods %s -emit-llvm -o - | FileCheck %s
-
-struct t1 {
-  void f1();
-  void f2();
-};
-
-void t1::f1() { }
-
-// CHECK: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "t1"
-// CHECK-SAME: elements: [[ELEMENTS:![0-9]+]]
-// CHECK: [[ELEMENTS]] = !{}
diff --git a/clang/test/Driver/debug-options.c b/clang/test/Driver/debug-options.c
index b09238d7b6bb6..7d061410a229f 100644
--- a/clang/test/Driver/debug-options.c
+++ b/clang/test/Driver/debug-options.c
@@ -242,11 +242,6 @@
 // RUN: %clang -### -c %s 2>&1 | FileCheck -check-prefix=NORNGBSE %s
 // RUN: %clang -### -c -fdebug-ranges-base-address -fno-debug-ranges-base-address %s 2>&1 | FileCheck -check-prefix=NORNGBSE %s
 //
-// RUN: %clang -### -c -gomit-unreferenced-methods %s 2>&1 | FileCheck -check-prefix=INCTYPES %s
-// RUN: %clang -### -c %s 2>&1 | FileCheck -check-prefix=NOINCTYPES %s
-// RUN: %clang -### -c -gomit-unreferenced-methods -fdebug-types-section %s 2>&1 | FileCheck -check-prefix=NOINCTYPES %s
-// RUN: %clang -### -c -gomit-unreferenced-methods -fstandalone-debug %s 2>&1 | FileCheck -check-prefix=NOINCTYPES %s
-//
 // RUN: %clang -### -c -glldb %s 2>&1 | FileCheck -check-prefix=NOPUB %s
 // RUN: %clang -### -c -glldb -gno-pubnames %s 2>&1 | FileCheck -check-prefix=NOPUB %s
 //
@@ -386,9 +381,6 @@
 // RNGBSE: -fdebug-ranges-base-address
 // NORNGBSE-NOT: -fdebug-ranges-base-address
 //
-// INCTYPES: -gomit-unreferenced-methods
-// NOINCTYPES-NOT: -gomit-unreferenced-methods
-//
 // GARANGE-DAG: -generate-arange-section
 //
 // FDTS: "-mllvm" "-generate-type-units"

llvmbot · 2024-05-30T04:48:39Z

@llvm/pr-subscribers-clang

Author: Mehdi Amini (joker-eph)

Changes

Reverts llvm/llvm-project#87018

MacOS and Windows bots are broken.

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

6 Files Affected:

(modified) clang/include/clang/Basic/DebugOptions.def (-2)
(modified) clang/include/clang/Driver/Options.td (-4)
(modified) clang/lib/CodeGen/CGDebugInfo.cpp (+1-1)
(modified) clang/lib/Driver/ToolChains/Clang.cpp (-15)
(removed) clang/test/CodeGenCXX/debug-info-incomplete-types.cpp (-12)
(modified) clang/test/Driver/debug-options.c (-8)

diff --git a/clang/include/clang/Basic/DebugOptions.def b/clang/include/clang/Basic/DebugOptions.def
index bc96d5dfdf890..b94f6aef9ac60 100644
--- a/clang/include/clang/Basic/DebugOptions.def
+++ b/clang/include/clang/Basic/DebugOptions.def
@@ -68,8 +68,6 @@ BENIGN_DEBUGOPT(NoInlineLineTables, 1, 0) ///< Whether debug info should contain
                                           ///< inline line tables.
 
 DEBUGOPT(DebugStrictDwarf, 1, 1) ///< Whether or not to use strict DWARF info.
-DEBUGOPT(DebugOmitUnreferencedMethods, 1, 0) ///< Omit unreferenced member
-					     ///< functions in type debug info.
 
 /// Control the Assignment Tracking debug info feature.
 BENIGN_ENUM_DEBUGOPT(AssignmentTrackingMode, AssignmentTrackingOpts, 2,
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index f64d7c60783e9..4119e69c85540 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4345,10 +4345,6 @@ defm strict_dwarf : BoolOption<"g", "strict-dwarf",
           "the specified version, avoiding features from later versions.">,
   NegFlag<SetFalse>, BothFlags<[], [ClangOption, CLOption, DXCOption]>>,
   Group<g_flags_Group>;
-defm omit_unreferenced_methods : BoolGOption<"omit-unreferenced-methods",
-  CodeGenOpts<"DebugOmitUnreferencedMethods">, DefaultFalse,
-  NegFlag<SetFalse>,
-  PosFlag<SetTrue, [], [CC1Option]>, BothFlags<[], [ClangOption, CLOption, DXCOption]>>;
 defm column_info : BoolOption<"g", "column-info",
   CodeGenOpts<"DebugColumnInfo">, DefaultTrue,
   NegFlag<SetFalse, [], [ClangOption, CC1Option]>,
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 5f6f911c7a6d6..9d7107abf8a6f 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2836,7 +2836,7 @@ CGDebugInfo::CreateTypeDefinition(const RecordType *Ty) {
 
   // Collect data fields (including static variables and any initializers).
   CollectRecordFields(RD, DefUnit, EltTys, FwdDecl);
-  if (CXXDecl && !CGM.getCodeGenOpts().DebugOmitUnreferencedMethods)
+  if (CXXDecl)
     CollectCXXMemberFunctions(CXXDecl, DefUnit, EltTys, FwdDecl);
 
   LexicalBlockStack.pop_back();
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 4e1c52462e584..97e451cfe2acb 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -45,7 +45,6 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/BinaryFormat/Magic.h"
 #include "llvm/Config/llvm-config.h"
-#include "llvm/Frontend/Debug/Options.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CodeGen.h"
@@ -4643,7 +4642,6 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T,
   Args.addOptInFlag(CmdArgs, options::OPT_fforce_dwarf_frame,
                     options::OPT_fno_force_dwarf_frame);
 
-  bool EnableTypeUnits = false;
   if (Args.hasFlag(options::OPT_fdebug_types_section,
                    options::OPT_fno_debug_types_section, false)) {
     if (!(T.isOSBinFormatELF() || T.isOSBinFormatWasm())) {
@@ -4654,24 +4652,11 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T,
     } else if (checkDebugInfoOption(
                    Args.getLastArg(options::OPT_fdebug_types_section), Args, D,
                    TC)) {
-      EnableTypeUnits = true;
       CmdArgs.push_back("-mllvm");
       CmdArgs.push_back("-generate-type-units");
     }
   }
 
-  if (const Arg *A =
-          Args.getLastArg(options::OPT_gomit_unreferenced_methods,
-                          options::OPT_gno_omit_unreferenced_methods))
-    (void)checkDebugInfoOption(A, Args, D, TC);
-  if (Args.hasFlag(options::OPT_gomit_unreferenced_methods,
-                   options::OPT_gno_omit_unreferenced_methods, false) &&
-      (DebugInfoKind == llvm::codegenoptions::DebugInfoConstructor ||
-       DebugInfoKind == llvm::codegenoptions::LimitedDebugInfo) &&
-      !EnableTypeUnits) {
-    CmdArgs.push_back("-gomit-unreferenced-methods");
-  }
-
   // To avoid join/split of directory+filename, the integrated assembler prefers
   // the directory form of .file on all DWARF versions. GNU as doesn't allow the
   // form before DWARF v5.
diff --git a/clang/test/CodeGenCXX/debug-info-incomplete-types.cpp b/clang/test/CodeGenCXX/debug-info-incomplete-types.cpp
deleted file mode 100644
index 0bf59233b4e2e..0000000000000
--- a/clang/test/CodeGenCXX/debug-info-incomplete-types.cpp
+++ /dev/null
@@ -1,12 +0,0 @@
-// RUN: %clang_cc1 -debug-info-kind=limited -gomit-unreferenced-methods %s -emit-llvm -o - | FileCheck %s
-
-struct t1 {
-  void f1();
-  void f2();
-};
-
-void t1::f1() { }
-
-// CHECK: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "t1"
-// CHECK-SAME: elements: [[ELEMENTS:![0-9]+]]
-// CHECK: [[ELEMENTS]] = !{}
diff --git a/clang/test/Driver/debug-options.c b/clang/test/Driver/debug-options.c
index b09238d7b6bb6..7d061410a229f 100644
--- a/clang/test/Driver/debug-options.c
+++ b/clang/test/Driver/debug-options.c
@@ -242,11 +242,6 @@
 // RUN: %clang -### -c %s 2>&1 | FileCheck -check-prefix=NORNGBSE %s
 // RUN: %clang -### -c -fdebug-ranges-base-address -fno-debug-ranges-base-address %s 2>&1 | FileCheck -check-prefix=NORNGBSE %s
 //
-// RUN: %clang -### -c -gomit-unreferenced-methods %s 2>&1 | FileCheck -check-prefix=INCTYPES %s
-// RUN: %clang -### -c %s 2>&1 | FileCheck -check-prefix=NOINCTYPES %s
-// RUN: %clang -### -c -gomit-unreferenced-methods -fdebug-types-section %s 2>&1 | FileCheck -check-prefix=NOINCTYPES %s
-// RUN: %clang -### -c -gomit-unreferenced-methods -fstandalone-debug %s 2>&1 | FileCheck -check-prefix=NOINCTYPES %s
-//
 // RUN: %clang -### -c -glldb %s 2>&1 | FileCheck -check-prefix=NOPUB %s
 // RUN: %clang -### -c -glldb -gno-pubnames %s 2>&1 | FileCheck -check-prefix=NOPUB %s
 //
@@ -386,9 +381,6 @@
 // RNGBSE: -fdebug-ranges-base-address
 // NORNGBSE-NOT: -fdebug-ranges-base-address
 //
-// INCTYPES: -gomit-unreferenced-methods
-// NOINCTYPES-NOT: -gomit-unreferenced-methods
-//
 // GARANGE-DAG: -generate-arange-section
 //
 // FDTS: "-mllvm" "-generate-type-units"

dwblaikie · 2024-05-30T05:42:20Z

Thanks! Sorry I didn't get to it sooner

…s" (#93767) This reverts commit 02c6845, reapplying bfabc95. The patch was reverted due to the test failing on MacOS and Windows where type units aren't supported. This is addressed by limiting type unit flag/test coverage to Linux. Complete C++ type information can be quite expensive - and there's limited value in representing every member function, even those that can't be called (we don't do similarly for every non-member function anyway). So add a flag to opt out of this behavior for experimenting with this more terse behavior. I think Sony already does this by default, so perhaps with a change to the defaults, Sony can migrate to this rather than a downstream patch. This breaks current debuggers in some expected ways - but those breakages are visible without this feature too. Consider member function template instantiations - they can't be consistently enumerated in every translation unit: a.h: ``` struct t1 { template <int i> static int f1() { return i; } }; namespace ns { template <int i> int f1() { return i; } } // namespace ns ``` a.cpp: ``` void f1() { t1::f1<0>(); ns::f1<0>(); } ``` b.cpp: ``` void f1(); int main() { f1(); t1::f1<1>(); ns::f1<1>(); } ``` ``` (gdb) p ns::f1<0>() $1 = 0 (gdb) p ns::f1<1>() $2 = 1 (gdb) p t1::f1<0>() Couldn't find method t1::f1<0> (gdb) p t1::f1<1>() $3 = 1 (gdb) s f1 () at a.cpp:3 3 t1::f1<0>(); (gdb) p t1::f1<0>() $4 = 0 (gdb) p t1::f1<1>() Couldn't find method t1::f1<1> (gdb) ``` (other similar non-canonical features are implicit special members (copy/move ctor/assignment operator, default ctor) and nested types (eg: pimpl idiom, where the nested type is declared-but-not-defined in one TU, and defined in another TU)) lldb can't parse the template expressions above, so I'm not sure how to test it there, but I'd guess it has similar problems. ( https://stackoverflow.com/questions/64602475/how-to-print-value-returned-by-template-member-function-in-gdb-lldb-debugging so... I guess that's just totally not supported in lldb, how unfortunate. And implicit special members are instantiated implicitly by lldb, so missing those doesn't tickle the same issue) Some very rudimentary numbers for a clang debug build: .debug_info section size: -g: 476MiB -g -fdebug-types-section: 357MiB -g -gomit-unreferenced-members: 340MiB Though it also means a major reduction in .debug_str size, -fdebug-types-section doesn't reduce string usage (so the first two examples have the same .debug_str size, 247MiB), down to 175MiB. So for total clang binary size (I don't have a quick "debug section size reduction" on-hand): 1.45 (no type units) GiB -> 1.34 -> 1.22, so it saves about 120MiB of binary size. Original Differential Revision: https://reviews.llvm.org/D152017

Revert "[DebugInfo] Add flag to only emit referenced member functions (…

a35c320

…#87018)" This reverts commit bfabc95.

joker-eph added the skip-precommit-approval PR for CI feedback, not intended for review label May 30, 2024

joker-eph requested a review from dwblaikie May 30, 2024 04:48

joker-eph merged commit 02c6845 into main May 30, 2024
5 of 6 checks passed

joker-eph deleted the revert-87018-debug_incomplete_types branch May 30, 2024 04:48

joker-eph mentioned this pull request May 30, 2024

[DebugInfo] Add flag to only emit referenced member functions #87018

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Revert "[DebugInfo] Add flag to only emit referenced member functions" #93767

Revert "[DebugInfo] Add flag to only emit referenced member functions" #93767

Uh oh!

joker-eph commented May 30, 2024

Uh oh!

Uh oh!

llvmbot commented May 30, 2024 •

edited

Loading

Uh oh!

llvmbot commented May 30, 2024

Uh oh!

dwblaikie commented May 30, 2024

Uh oh!

Uh oh!

Revert "[DebugInfo] Add flag to only emit referenced member functions" #93767

Revert "[DebugInfo] Add flag to only emit referenced member functions" #93767

Uh oh!

Conversation

joker-eph commented May 30, 2024

Uh oh!

Uh oh!

llvmbot commented May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented May 30, 2024

Uh oh!

dwblaikie commented May 30, 2024

Uh oh!

Uh oh!

llvmbot commented May 30, 2024 •

edited

Loading