Skip to content

[spirv] generate OpenCL.DebugInfo.100 instructions#3155

Merged
jaebaek merged 81 commits intomicrosoft:masterfrom
google:debug_info
Sep 22, 2020
Merged

[spirv] generate OpenCL.DebugInfo.100 instructions#3155
jaebaek merged 81 commits intomicrosoft:masterfrom
google:debug_info

Conversation

@jaebaek
Copy link
Copy Markdown
Collaborator

@jaebaek jaebaek commented Sep 21, 2020

OpenCL.DebugInfo.100
is a SPIR-V extended instruction set that provides DWARF style debug information.
This PR allows DXC SPIR-V backend to generate the rich HLSL debug information
using OpenCL.DebugInfo.100 instructions.

ehsannas and others added 30 commits March 4, 2020 11:53
Our new methodology keeps names and line numbers as member variables of
all SpirvInstruction objects, and therefore we do not have separate
instruction classes for OpLine and OpName. These enums were leftover
from the previous improvements.
Adds a new possible value for `-fspv-debug=` option. If `rich` is
specified, Rich Debug Info should be generated.

Also added base class for debug instructions as well as DebugScope and
DebugCompilationUnit classes.
All debug instructions are expressed as OpExtInst in SPIR-V and they
should all have OpTypeVoid as their result type.

Taking this (OpTypeVoid) in the constructor of all debug instructions is
cumbersome and can be confusing for cases such as DebugLocalVariable or
DebugGlobalVariable or DebugFunction where we also need the variable
type or function type. For example: The caller of these constructors could
accidentally pass the variable type instead of OpTypeVoid.
When there are multiple header files included, we need multiple
DebugSource. This commit includes heavy refactoring to support it.
We should emit debug info instructions in the order of
1. DebugOperation
2. DebugExpression
3. For each DebugSource
4. DebugCompilationUnit
5. DebugTypeXXX
6. DebugGlobalVariable
7. For each DebugFunction
   7.1. DebugTypeXXX defined in this function
   7.2. DebugGlobalVariable or DebugLocalVariable defined in
        this function
   7.3. For each DebugLexicalBlock defined in this function
        7.3.1. DebugTypeXXX defined in block
        7.3.2. DebugLocalVariable defined in block

If we do not follow this order, generated SPIR-V code would be
invalid because of forward references. To generate the SPIR-V code
with the valid layout, this change maintains each type of debug info
instructions separately.
Without this commit, no debug info instruction points to
DebugFunction. At least, its DebugLexical block must reference it as a
parent. This commit fixes the bug.
OpenCL.DebugInfo.100 Information Extended Instruction Set does not
support a debug type for a matrix type. Therefore, we do not actually
emit a matrix debug type, but this commit handles the matrix case to
avoid crash. We temporarily emit DebugTypeArray instead. This must be
updated when the spec adds DebugTypeMatrix later.
Both SpirvBuilder and SpirvEmitter managed the parent lexical scope
info. We want to move all parent lexical scope info and
DebugSource/DebugCompilationUnit info to SpirvContext. This information
will be used by debug type lowering for composite type and member
function. In other words, it will be used not only by SpirvBuilder and
SpirvEmitter but also by DebugTypeVisitor.
ben-clayton and others added 19 commits June 2, 2020 13:19
Rename `offset` variables to `offsetInBits` to try avoiding this sort of mistake in the future.
The pseudo code for lowering an array type in DebugTypeVisitor is
```
elementType = arrayType->element
if elementType is an array type
  (DebugTypeArray for elementType).appendOneMoreDimensionAtTheEnd(count)
else
  new DebugTypeArray(lower(arrayType->element), count)
```

When it appends one more dimension at the end of previously created
DebugTypeArray, it reuses the existing DebugTypeArray object, which
result in a wrong DebugTypeArray for a previously shown array type.

For example,
```
float foo[2];  --> dbg_type_array = DebugTypeArray(float, 2)
float bar[2][3]; --> dbg_type_array.appendOneMoreDimensionAtTheEnd(3)

// When emitting the SPIRV, dbg_type_array is
// DebugTypeArray(float, 2, 3) which is incorrect for "foo"
```
This commit lets DebugTypeVisitor create a new DebugTypeArray object for
new array type to fix this bug.
We emit DebugInfoNone for Value operand of DebugTypeTemplateParam
when DebugTypeTemplateParam is used for a type, not a value. Previously,
we did not consider the case in SortDebugInfoVisitor.
Even when a local variable does not have the initialization, we have to
map its storage e.g., OpVariable to its DebugLocalVariable.
The following two things have impact on the debug type generation:
1. The delivered debug information must be correct in the view of the debugger.
For example, if a variable in HLSL has a different size in SPIR-V, we have to use
the SPIR-V one because it is what the GPU will use.
2. We cannot recover the HLSL type information only using SPIR-V type information.
For example, if a struct is inherited from a base, the struct type in SPIR-V contains
the base in addition to the struct itself and we cannot separate the base from the struct itself.

In other words, the information of both SPIR-V type and HLSL type is needed
to generate debug types. We previously added some code to partially lower
HLSL types to debug types in LowerTypeVisitor, but it turns out that the code
complexity increases a lot because we keep adding some code for exceptional
cases and finally we cannot understand the code by ourselves.

We decided to redesign it and conducted a large refactoring. Key ideas are
- Do not change LowerTypeVisitor. Add only changes when there is no other option.
- Keep Clang Decl information to SpirvContext class (See spvStructTypeToDecl and declToDebugFunction of SpirvContext class).
OpenCL.DebugInfo.100 DebugFunction's 'Name' operand must be the name of
the function given by the HLSL code. DXC adds "src." as a prefix of the
enty function, but DebugFunction must not have it.

In addition, the entry function only called by a wrapper is externally
invisible. We have to set an empty string for the linkage name.

Fixes #324
We have to add DebugScope when we *switch* the lexical scope, not when
we create DebugLexicalBlock.
When the physical layout of a local variable is unknown, we cannot
properly set Offset and Size of DebugTypeMember based on the current
spec. We need the spec update.

Since we need to test SwiftShader debugger (the first Vulkan shader
debugger that can be used as a reference implementation), we want to
temporarily emit Offset and Size of DebugTypeMember even for local
variables with the unknown physical layout. We assume it uses the
tight data filling.
@jaebaek jaebaek added the spirv Work related to SPIR-V label Sep 21, 2020
@jaebaek jaebaek requested a review from ehsannas September 21, 2020 21:42
@jaebaek jaebaek self-assigned this Sep 21, 2020
@AppVeyorBot
Copy link
Copy Markdown

@jaebaek jaebaek merged commit 7f985ff into microsoft:master Sep 22, 2020
sudonatalie added a commit to sudonatalie/DirectXShaderCompiler that referenced this pull request May 17, 2024
This test was marked DISABLED_ in Gtest at the time it was added in PR
 microsoft#3155, so it appears that it was never passing. I don't think it's a
high-priority to implement at this point, so opting to simply remove it.

This is the last test in the unsupported directory so it can now be
removed entirely.

Fixes microsoft#6616
sudonatalie added a commit that referenced this pull request May 21, 2024
This test was marked DISABLED_ in gtest at the time it was added in PR
#3155, so it appears that it was never passing. Specifically, the CHECK
for `DebugFunction [[func1]]` fails. I don't think it's a priority to
implement debug info for unreferenced functions at this point, so opting
to simply remove it.

This is the last test in the unsupported directory so it can now be
removed entirely.

Fixes #6616
SjMxr233 pushed a commit to ShaderHelper/DirectXShaderCompiler that referenced this pull request Jul 24, 2025
This test was marked DISABLED_ in gtest at the time it was added in PR
microsoft#3155, so it appears that it was never passing. Specifically, the CHECK
for `DebugFunction [[func1]]` fails. I don't think it's a priority to
implement debug info for unreferenced functions at this point, so opting
to simply remove it.

This is the last test in the unsupported directory so it can now be
removed entirely.

Fixes microsoft#6616
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

spirv Work related to SPIR-V

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants