Skip to content

Commit f4dcc6e

Browse files
KeenutsKeenuts
authored
SPIRV: don't generate invalid debug instructions (#5397)
When generating debug instructions for a function, each one is linked to its scope. In case of member functions, this scope is the class. When declaring a class, all its member, including functions must be declared. This cycle requires a forward reference, which is allowed by the debug instruction spec, but not by parents: NonSemantic & SPIR-V specs. This is a spec issue we have to fix. In the meantime, we decided to emit a warning, and generate slightly worse debug instructions. Context: KhronosGroup/SPIRV-Registry#203 --------- Signed-off-by: Nathan Gauër <[email protected]>
1 parent 68c7d38 commit f4dcc6e

6 files changed

Lines changed: 60 additions & 5 deletions

File tree

‎tools/clang/lib/SPIRV/DebugTypeVisitor.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,14 @@ void DebugTypeVisitor::lowerDebugTypeMembers(
162162
assert(false && "Uknown DeclContext for DebugTypeMember generation");
163163
}
164164

165+
// Note:
166+
// The NonSemantic.Shader.DebugInfo.100 way to define member functions
167+
// breaks both the NonSemantic and SPIR-V specification. Until this is
168+
// resolved, we cannot emit debug instructions for member functions without
169+
// creating invalid forward references.
170+
//
171+
// See https://github.com/KhronosGroup/SPIRV-Registry/issues/203
172+
#if 0
165173
// Push member functions to DebugTypeComposite Members operand.
166174
for (auto *subDecl : decl->decls()) {
167175
if (const auto *methodDecl = dyn_cast<FunctionDecl>(subDecl)) {
@@ -174,6 +182,7 @@ void DebugTypeVisitor::lowerDebugTypeMembers(
174182
}
175183
}
176184
}
185+
#endif
177186
}
178187

179188
SpirvDebugTypeTemplate *DebugTypeVisitor::lowerDebugTypeTemplate(

‎tools/clang/lib/SPIRV/SpirvEmitter.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,12 @@ void SpirvEmitter::HandleTranslationUnit(ASTContext &context) {
781781
if (context.getDiagnostics().hasErrorOccurred())
782782
return;
783783

784+
if (spirvOptions.debugInfoRich) {
785+
emitWarning("Member functions will not be linked to their class in the debug information. "
786+
"See https://github.com/KhronosGroup/SPIRV-Registry/issues/203",
787+
{});
788+
}
789+
784790
TranslationUnitDecl *tu = context.getTranslationUnitDecl();
785791
uint32_t numEntryPoints = 0;
786792

‎tools/clang/test/CodeGenSPIRV/rich.debug.member.function.param.hlsl‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ struct foo {
1919
bool c;
2020
};
2121

22+
// Note:
23+
// For member functions, the scope in the debug info should be the encapsulating composite.
24+
// Doing this (as specified in the NonSemantic.Shader.DebugInfo.100) would break the SPIR-V and NonSemantic
25+
// spec. For this reason, DXC is emitting the wrong scope.
26+
// See https://github.com/KhronosGroup/SPIRV-Registry/issues/203
27+
2228
// CHECK: [[set:%\d+]] = OpExtInstImport "OpenCL.DebugInfo.100"
2329

2430
// CHECK: [[fooName:%\d+]] = OpString "foo"
@@ -31,19 +37,26 @@ struct foo {
3137
// CHECK: [[arg:%\d+]] = OpString "arg"
3238

3339
// CHECK: [[bool:%\d+]] = OpExtInst %void [[set]] DebugTypeBasic {{%\d+}} %uint_32 Boolean
40+
// CHECK: [[unit:%\d+]] = OpExtInst %void [[set]] DebugCompilationUnit 1 4 {{%\d+}} HLSL
3441
// CHECK: [[foo:%\d+]] = OpExtInst %void [[set]] DebugTypeComposite [[fooName]] Structure {{%\d+}} 3 8
3542

3643
// CHECK: [[float:%\d+]] = OpExtInst %void [[set]] DebugTypeBasic {{%\d+}} %uint_32 Float
3744
// CHECK: [[int:%\d+]] = OpExtInst %void [[set]] DebugTypeBasic {{%\d+}} %uint_32 Signed
3845

3946
// CHECK: [[func1Type:%\d+]] = OpExtInst %void [[set]] DebugTypeFunction FlagIsProtected|FlagIsPrivate [[int]] [[foo]] [[int]] [[float]] [[bool]]
40-
// CHECK: [[f1:%\d+]] = OpExtInst %void [[set]] DebugFunction [[func1]] [[func1Type]] {{%\d+}} 12 3 [[foo]] {{%\d+}} FlagIsProtected|FlagIsPrivate 12 %foo_func1
47+
48+
// CHECK-NOT: [[f1:%\d+]] = OpExtInst %void [[set]] DebugFunction [[func1]] [[func1Type]] {{%\d+}} 12 3 [[foo]] {{%\d+}} FlagIsProtected|FlagIsPrivate 12 %foo_func1
49+
// CHECK: [[f1:%\d+]] = OpExtInst %void [[set]] DebugFunction [[func1]] [[func1Type]] {{%\d+}} 12 3 [[unit]] {{%\d+}} FlagIsProtected|FlagIsPrivate 12 %foo_func1
50+
4151
// CHECK: {{%\d+}} = OpExtInst %void [[set]] DebugLocalVariable [[arg2]] {{%\d+}} {{%\d+}} 12 40 [[f1]] FlagIsLocal 4
4252
// CHECK: {{%\d+}} = OpExtInst %void [[set]] DebugLocalVariable [[arg1]] {{%\d+}} {{%\d+}} 12 29 [[f1]] FlagIsLocal 3
4353
// CHECK: {{%\d+}} = OpExtInst %void [[set]] DebugLocalVariable [[arg0]] {{%\d+}} {{%\d+}} 12 17 [[f1]] FlagIsLocal 2
4454
// CHECK: {{%\d+}} = OpExtInst %void [[set]] DebugLocalVariable [[this]] [[foo]] {{%\d+}} 12 3 [[f1]] FlagArtificial|FlagObjectPointer 1
4555
// CHECK: [[func0Type:%\d+]] = OpExtInst %void [[set]] DebugTypeFunction FlagIsProtected|FlagIsPrivate %void [[foo]] [[float]]
46-
// CHECK: [[f0:%\d+]] = OpExtInst %void [[set]] DebugFunction [[func0]] [[func0Type]] {{%\d+}} 6 3 [[foo]] {{%\d+}} FlagIsProtected|FlagIsPrivate 6 %foo_func0
56+
57+
// CHECK-NOT: [[f0:%\d+]] = OpExtInst %void [[set]] DebugFunction [[func0]] [[func0Type]] {{%\d+}} 6 3 [[foo]] {{%\d+}} FlagIsProtected|FlagIsPrivate 6 %foo_func0
58+
// CHECK: [[f0:%\d+]] = OpExtInst %void [[set]] DebugFunction [[func0]] [[func0Type]] {{%\d+}} 6 3 [[unit]] {{%\d+}} FlagIsProtected|FlagIsPrivate 6 %foo_func0
59+
4760
// CHECK: {{%\d+}} = OpExtInst %void [[set]] DebugLocalVariable [[arg]] {{%\d+}} {{%\d+}} 6 20 [[f0]] FlagIsLocal 2
4861
// CHECK: {{%\d+}} = OpExtInst %void [[set]] DebugLocalVariable [[this]] [[foo]] {{%\d+}} 6 3 [[f0]] FlagArtificial|FlagObjectPointer 1
4962

‎tools/clang/test/CodeGenSPIRV/rich.debug.type.composite.hlsl‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,14 @@ struct foo {
1919
bool c;
2020
};
2121

22+
// Note:
23+
// For member functions, the scope in the debug info should be the encapsulating composite.
24+
// Doing this (as specified in the NonSemantic.Shader.DebugInfo.100) would break the SPIR-V and NonSemantic
25+
// spec. For this reason, DXC is emitting the wrong scope.
26+
// See https://github.com/KhronosGroup/SPIRV-Registry/issues/203
27+
2228
// CHECK: [[set:%\d+]] = OpExtInstImport "OpenCL.DebugInfo.100"
29+
2330
// CHECK: [[bool_name:%\d+]] = OpString "bool"
2431
// CHECK: [[foo:%\d+]] = OpString "foo"
2532
// CHECK: [[c_name:%\d+]] = OpString "c"
@@ -31,16 +38,22 @@ struct foo {
3138
// CHECK: [[func0:%\d+]] = OpString "foo.func0"
3239

3340
// CHECK: [[bool:%\d+]] = OpExtInst %void %1 DebugTypeBasic [[bool_name]] %uint_32 Boolean
34-
// CHECK: [[parent:%\d+]] = OpExtInst %void [[set]] DebugTypeComposite [[foo]] Structure {{%\d+}} 3 8 {{%\d+}} {{%\d+}} %uint_192 FlagIsProtected|FlagIsPrivate [[a:%\d+]] [[b:%\d+]] [[c:%\d+]] [[f0:%\d+]] [[f1:%\d+]]
41+
// CHECK: [[unit:%\d+]] = OpExtInst %void [[set]] DebugCompilationUnit 1 4 {{%\d+}} HLSL
42+
43+
// CHECK-NOT: [[parent:%\d+]] = OpExtInst %void [[set]] DebugTypeComposite [[foo]] Structure {{%\d+}} 3 8 {{%\d+}} {{%\d+}} %uint_192 FlagIsProtected|FlagIsPrivate [[a:%\d+]] [[b:%\d+]] [[c:%\d+]] {{%\d+}} {{%\d+}}
44+
// CHECK: [[parent:%\d+]] = OpExtInst %void [[set]] DebugTypeComposite [[foo]] Structure {{%\d+}} 3 8 {{%\d+}} {{%\d+}} %uint_192 FlagIsProtected|FlagIsPrivate [[a:%\d+]] [[b:%\d+]] [[c:%\d+]]
3545

3646
// CHECK: [[c]] = OpExtInst %void [[set]] DebugTypeMember [[c_name]] [[bool]] {{%\d+}} 19 8 [[parent]] %uint_160 %uint_32 FlagIsProtected|FlagIsPrivate
3747
// CHECK: [[float:%\d+]] = OpExtInst %void %1 DebugTypeBasic [[float_name]] %uint_32 Float
3848
// CHECK: [[v4f:%\d+]] = OpExtInst %void %1 DebugTypeVector [[float]] 4
3949
// CHECK: [[b]] = OpExtInst %void [[set]] DebugTypeMember [[b_name]] [[v4f]] {{%\d+}} 10 10 [[parent]] %uint_32 %uint_128 FlagIsProtected|FlagIsPrivate
4050
// CHECK: [[int:%\d+]] = OpExtInst %void %1 DebugTypeBasic [[int_name]] %uint_32 Signed
4151
// CHECK: [[a]] = OpExtInst %void [[set]] DebugTypeMember [[a_name]] [[int]] {{%\d+}} 4 7 [[parent]] %uint_0 %uint_32 FlagIsProtected|FlagIsPrivate
42-
// CHECK: [[f1]] = OpExtInst %void [[set]] DebugFunction [[func1]] {{%\d+}} {{%\d+}} 12 3 [[parent]] {{%\d+}} FlagIsProtected|FlagIsPrivate 12 %foo_func1
43-
// CHECK: [[f0]] = OpExtInst %void [[set]] DebugFunction [[func0]] {{%\d+}} {{%\d+}} 6 3 [[parent]] {{%\d+}} FlagIsProtected|FlagIsPrivate 6 %foo_func0
52+
53+
// CHECK-NOT: [[f1:%\d+]] = OpExtInst %void [[set]] DebugFunction [[func1]] {{%\d+}} {{%\d+}} 12 3 [[parent]] {{%\d+}} FlagIsProtected|FlagIsPrivate 12 %foo_func1
54+
// CHECK-NOT: [[f0:%\d+]] = OpExtInst %void [[set]] DebugFunction [[func0]] {{%\d+}} {{%\d+}} 6 3 [[parent]] {{%\d+}} FlagIsProtected|FlagIsPrivate 6 %foo_func0
55+
// CHECK: [[f1:%\d+]] = OpExtInst %void [[set]] DebugFunction [[func1]] {{%\d+}} {{%\d+}} 12 3 [[unit]] {{%\d+}} FlagIsProtected|FlagIsPrivate 12 %foo_func1
56+
// CHECK: [[f0:%\d+]] = OpExtInst %void [[set]] DebugFunction [[func0]] {{%\d+}} {{%\d+}} 6 3 [[unit]] {{%\d+}} FlagIsProtected|FlagIsPrivate 6 %foo_func0
4457

4558
float4 main(float4 color : COLOR) : SV_TARGET {
4659
foo a;

‎tools/clang/test/CodeGenSPIRV/rich.debug.type.composite.warning.hlsl‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// RUN: %dxc -T ps_6_0 -E main -fspv-debug=rich
2+
3+
struct foo {
4+
void method() { }
5+
};
6+
7+
float4 main(float4 color : COLOR) : SV_TARGET {
8+
return color;
9+
}
10+
11+
// CHECK: warning: Member functions will not be linked to their class in the debug information. See https://github.com/KhronosGroup/SPIRV-Registry/issues/203

‎tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3026,6 +3026,9 @@ TEST_F(FileTest, DISABLED_RichDebugInfoMemberFunctionWithoutCall) {
30263026
TEST_F(FileTest, RichDebugInfoTypeComposite) {
30273027
runFileTest("rich.debug.type.composite.hlsl");
30283028
}
3029+
TEST_F(FileTest, RichDebugInfoTypeCompositeEmitsWarning) {
3030+
runFileTest("rich.debug.type.composite.warning.hlsl", Expect::Warning);
3031+
}
30293032
TEST_F(FileTest, RichDebugInfoTypeCompositeEmpty) {
30303033
runFileTest("rich.debug.type.composite.empty.hlsl");
30313034
}

0 commit comments

Comments
 (0)