[Clang][Driver] Expose `-fno-eliminate-unused-debug-types` to clang-cl #95259

aganea · 2024-06-12T15:10:16Z

This is used to set DebugInfoKind to "UnusedTypeInfo". This helps in the context of Unreal Engine and NATVIS files that reference unused otherwise static constexpr class members. See https://udn.unrealengine.com/s/question/0D5QP00000N012h0AB/fname-debug-visualizer-fails-to-work-with-the-clang-compiler

This partially fixes #46924

This fixes llvm#46924

llvmbot · 2024-06-12T15:10:49Z

@llvm/pr-subscribers-clang-driver

Author: Alexandre Ganea (aganea)

Changes

This is used to set DebugInfoKind to "UnusedTypeInfo". This helps in the context Unreal Engine and NATVIS files that reference unused otherwise static constexpr class members. See https://udn.unrealengine.com/s/question/0D5QP00000N012h0AB/fname-debug-visualizer-fails-to-work-with-the-clang-compiler

This fixes #46924

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

3 Files Affected:

(modified) clang/docs/UsersManual.rst (+3)
(modified) clang/include/clang/Driver/Options.td (+1-1)
(modified) clang/test/Driver/cl-options.c (+2)

diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index f954857b0235a..d36db8a01949c 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -3338,6 +3338,9 @@ below. If multiple flags are present, the last one is used.
   By default, Clang does not emit type information for types that are defined
   but not used in a program. To retain the debug info for these unused types,
   the negation **-fno-eliminate-unused-debug-types** can be used.
+  This can be particulary useful on Windows, when using NATVIS files that
+  can reference const symbols that would otherwise be stripped, even in full
+  debug or standalone debug modes.
 
 Controlling Macro Debug Info Generation
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 9f7904dd94b94..b75d67551bc97 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2120,7 +2120,7 @@ def fno_elide_type : Flag<["-"], "fno-elide-type">, Group<f_Group>,
     MarshallingInfoNegativeFlag<DiagnosticOpts<"ElideType">>;
 def feliminate_unused_debug_symbols : Flag<["-"], "feliminate-unused-debug-symbols">, Group<f_Group>;
 defm eliminate_unused_debug_types : OptOutCC1FFlag<"eliminate-unused-debug-types",
-  "Do not emit ", "Emit ", " debug info for defined but unused types">;
+  "Do not emit ", "Emit ", " debug info for defined but unused types", [ClangOption, CLOption]>;
 def femit_all_decls : Flag<["-"], "femit-all-decls">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option]>,
   HelpText<"Emit all declarations, even if unused">,
diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 2c17459dde656..d7d7c5c825815 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -696,6 +696,8 @@
 // RUN:     -Wunused-variable \
 // RUN:     -fmacro-backtrace-limit=0 \
 // RUN:     -fstandalone-debug \
+// RUN:     -feliminate-unused-debug-types \
+// RUN:     -fno-eliminate-unused-debug-types \
 // RUN:     -flimit-debug-info \
 // RUN:     -flto \
 // RUN:     -fmerge-all-constants \

llvmbot · 2024-06-12T15:10:49Z

@llvm/pr-subscribers-clang

Author: Alexandre Ganea (aganea)

Changes

This is used to set DebugInfoKind to "UnusedTypeInfo". This helps in the context Unreal Engine and NATVIS files that reference unused otherwise static constexpr class members. See https://udn.unrealengine.com/s/question/0D5QP00000N012h0AB/fname-debug-visualizer-fails-to-work-with-the-clang-compiler

This fixes #46924

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

3 Files Affected:

(modified) clang/docs/UsersManual.rst (+3)
(modified) clang/include/clang/Driver/Options.td (+1-1)
(modified) clang/test/Driver/cl-options.c (+2)

diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index f954857b0235a..d36db8a01949c 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -3338,6 +3338,9 @@ below. If multiple flags are present, the last one is used.
   By default, Clang does not emit type information for types that are defined
   but not used in a program. To retain the debug info for these unused types,
   the negation **-fno-eliminate-unused-debug-types** can be used.
+  This can be particulary useful on Windows, when using NATVIS files that
+  can reference const symbols that would otherwise be stripped, even in full
+  debug or standalone debug modes.
 
 Controlling Macro Debug Info Generation
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 9f7904dd94b94..b75d67551bc97 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2120,7 +2120,7 @@ def fno_elide_type : Flag<["-"], "fno-elide-type">, Group<f_Group>,
     MarshallingInfoNegativeFlag<DiagnosticOpts<"ElideType">>;
 def feliminate_unused_debug_symbols : Flag<["-"], "feliminate-unused-debug-symbols">, Group<f_Group>;
 defm eliminate_unused_debug_types : OptOutCC1FFlag<"eliminate-unused-debug-types",
-  "Do not emit ", "Emit ", " debug info for defined but unused types">;
+  "Do not emit ", "Emit ", " debug info for defined but unused types", [ClangOption, CLOption]>;
 def femit_all_decls : Flag<["-"], "femit-all-decls">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option]>,
   HelpText<"Emit all declarations, even if unused">,
diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 2c17459dde656..d7d7c5c825815 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -696,6 +696,8 @@
 // RUN:     -Wunused-variable \
 // RUN:     -fmacro-backtrace-limit=0 \
 // RUN:     -fstandalone-debug \
+// RUN:     -feliminate-unused-debug-types \
+// RUN:     -fno-eliminate-unused-debug-types \
 // RUN:     -flimit-debug-info \
 // RUN:     -flto \
 // RUN:     -fmerge-all-constants \

amykhuang

Looks reasonable to me.

aganea · 2024-06-15T15:32:18Z

Thanks @amykhuang for taking a look!

I won't close #46924 right away, since we also need https://reviews.llvm.org/D89286. Do you mind if I took ownership of that patch and send a PR crediting you?

amykhuang · 2024-06-17T17:42:45Z

Yes, feel free to take over that part, thanks!

aganea · 2024-06-19T23:13:48Z

Yes, feel free to take over that part, thanks!

@amykhuang : I took a look at https://reviews.llvm.org/D89286. It works but doesn't fix the most problematic case where the constexpr member needs an evaluation that relies on dependent values:

    template <int VAL>
    struct C {
        static constexpr int constexprVal_C = VAL;
    };
    C<200> globalC;

I've tried your suggestion, that is forcing evaluation of constexpr members upon instantiation:

Actually, can we just instantiate the initializer for static constexpr data members?

currently it delays creating the initializer for inline static data members; can we not do that if it's a constexpr?

While this works for the above example, it fails a few tests in Clang, like this one. It might be because the constexpr expressions are only supposed to be evaluated when used. I'm not sure what a good way forward would be if we wanted to fix this. @zygoloid (note: this is only to generate constants in the debug info).

Overall this is a minor issue and I've fixed it by switching to a plain const. The root issue is that we have .NATVIS files that are referencing these otherwise unused members. If we wanted to do things the right way, perhaps NATVIS merged into the PDB would need to be evaluated at compile time, but that sounds like a lot of work. Also, NATVIS come too late in the process, at link time. Hopefully, Visual Studio has a debug mode where failed NATVIS evaluations are shown in the debug output, so they are easy to spot.

dwblaikie · 2024-06-22T18:55:57Z

Yes, the initializers do have to be lazyily evaluated to be a conforming C++ compiler.

eg: this code must compile without error so far as I understand:

template<int Value>
struct t1 {
  static constexpr int x = 3 / Value;
};
t1<0> v1;

Though it seems MSVC doesn't implement this correctly? https://godbolt.org/z/c1f6xKn3T

One way I've seen someone force the instantiation of the variable definition, and thus get the constant emitted into the DWARF, is using a static_assert:

template<int Value>
struct t1 {
  static constexpr int x = 3 / Value;
  static_assert(x, true);
};
t1<1> v1;

(of course, in this case, t1<0> can no longer be instantiated)

dwblaikie · 2024-06-22T18:58:20Z

I will say, -fno-eliminate-unused-debug-types is a really heavy hammer that makes debug info much larger - and my understanding was that games tended to have trouble with large debug builds, so I'd be surprised if this was a great path forward.

Do you have a small example of where Clang and MSVC differ in emitting some particular unreferenced type? I would imagine MSVC isn't emitting /every/ written type...

aganea · 2024-06-23T19:27:26Z

I will say, -fno-eliminate-unused-debug-types is a really heavy hammer that makes debug info much larger - and my understanding was that games tended to have trouble with large debug builds, so I'd be surprised if this was a great path forward.

Absolutely, this is only a short term hack until I figure out a better way to fix it.

Do you have a small example of where Clang and MSVC differ in emitting some particular unreferenced type? I would imagine MSVC isn't emitting /every/ written type...

It's essentially the examples in #46924. A class that is only used to hold some const/constexpr values. These values are then used by the .NATVIS file. With /Z7, MSVC seems to always emit them as S_CONSTANTs. But in Clang since the type isn't used, it is never emitted by CGDebugInfo::EmitAndRetainType(). Setting DebugInfo == llvm::codegenoptions::UnusedTypeInfo fixes the problem. Is there a better way?

dwblaikie · 2024-06-24T15:59:40Z

I will say, -fno-eliminate-unused-debug-types is a really heavy hammer that makes debug info much larger - and my understanding was that games tended to have trouble with large debug builds, so I'd be surprised if this was a great path forward.

Absolutely, this is only a short term hack until I figure out a better way to fix it.

Do you have a small example of where Clang and MSVC differ in emitting some particular unreferenced type? I would imagine MSVC isn't emitting /every/ written type...

It's essentially the examples in #46924. A class that is only used to hold some const/constexpr values. These values are then used by the .NATVIS file. With /Z7, MSVC seems to always emit them as S_CONSTANTs. But in Clang since the type isn't used, it is never emitted by CGDebugInfo::EmitAndRetainType(). Setting DebugInfo == llvm::codegenoptions::UnusedTypeInfo fixes the problem. Is there a better way?

Ah, I'll take up the discussion on the issue, I think - thanks for pointing that out.

[Clang][Driver] Expose -fno-eliminate-unused-debug-types to clang-cl

75fd057

This fixes llvm#46924

aganea requested review from rnk, amykhuang, dwblaikie and sylvain-audi June 12, 2024 15:10

llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jun 12, 2024

amykhuang approved these changes Jun 12, 2024

View reviewed changes

aganea merged commit 7cb5faf into llvm:main Jun 15, 2024
11 checks passed

aganea deleted the unused_types_clang-cl branch June 15, 2024 15:33

jpienaar mentioned this pull request Jun 17, 2024

[mlirc] Add missing extern C #95829

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Clang][Driver] Expose `-fno-eliminate-unused-debug-types` to clang-cl #95259

[Clang][Driver] Expose `-fno-eliminate-unused-debug-types` to clang-cl #95259

Uh oh!

aganea commented Jun 12, 2024 •

edited

Loading

Uh oh!

llvmbot commented Jun 12, 2024

Uh oh!

llvmbot commented Jun 12, 2024

Uh oh!

amykhuang left a comment

Uh oh!

aganea commented Jun 15, 2024

Uh oh!

Uh oh!

amykhuang commented Jun 17, 2024

Uh oh!

aganea commented Jun 19, 2024

Uh oh!

dwblaikie commented Jun 22, 2024

Uh oh!

dwblaikie commented Jun 22, 2024

Uh oh!

aganea commented Jun 23, 2024

Uh oh!

dwblaikie commented Jun 24, 2024

Uh oh!

Uh oh!

[Clang][Driver] Expose -fno-eliminate-unused-debug-types to clang-cl #95259

[Clang][Driver] Expose -fno-eliminate-unused-debug-types to clang-cl #95259

Uh oh!

Conversation

aganea commented Jun 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jun 12, 2024

Uh oh!

llvmbot commented Jun 12, 2024

Uh oh!

amykhuang left a comment

Choose a reason for hiding this comment

Uh oh!

aganea commented Jun 15, 2024

Uh oh!

Uh oh!

amykhuang commented Jun 17, 2024

Uh oh!

aganea commented Jun 19, 2024

Uh oh!

dwblaikie commented Jun 22, 2024

Uh oh!

dwblaikie commented Jun 22, 2024

Uh oh!

aganea commented Jun 23, 2024

Uh oh!

dwblaikie commented Jun 24, 2024

Uh oh!

Uh oh!

[Clang][Driver] Expose `-fno-eliminate-unused-debug-types` to clang-cl #95259

[Clang][Driver] Expose `-fno-eliminate-unused-debug-types` to clang-cl #95259

aganea commented Jun 12, 2024 •

edited

Loading