Fix function definition in debug info, and related cleanups #2807
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
isDefinition is always true, so createMethod could never be called. It is also unclear how useful calling createMethod is at this point. It seems the primary difference to createFunction is supporting vtable info and this-pointer adjustments, but neither concept seems to exist in NonSemantic.Shader.DebugInfo.100
DebugFunction is always a function definition, never a function declaration -- those are represented using DebugFunctionDeclaration. It's also unclear whether shader entry points should be considered "main subprograms" or not. First of all, it's unclear what that flag means. Second, entry points aren't exactly like C/C++ main() -- a better analogy would be the "main" function of a thread created with pthread_create or similar. Finally, the current test is simply incorrect because NonSemantic.Shader.DebugInfo.100 doesn't actually reference the OpFunction from the DebugFunction instruction.
Be honest about the operands of DebugFunction when reading the SPIR-V and cleanly handle the fact that (unlike in the OpenCL debug info) the DebugFunction instruction doesn't reference the OpFunction at all. Setting the LLVM debug metadata can only be done after reading the DebugFunctionDefinition instruction which links the two.
Test summary for commit a57987cCTS tests (Failed: 0/138184)
Ubuntu navi3x, SrdcvkUbuntu navi2x, Srdcvk |
|
Thanks, LGTM , it cleans a lot of code. |
Flakebi
reviewed
Nov 10, 2023
The -trim-debug-info option should also trim this more advanced debug info when set (which is the default).
nhaehnle
force-pushed
the
pub-debuginfo
branch
from
a57987c to
909e511
Compare
Test summary for commit 909e511CTS tests (Failed: 0/138362)
Ubuntu navi3x, SrdcvkUbuntu navi2x, Srdcvk |
Flakebi
approved these changes
Nov 14, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The debug info support was originally written for the OpenCL debug info extension. We don't actually support kernel SPIR-V, though, and the shader debug info extension has existed for a while now, is produced by glslang and dxc, and is subtly different. The biggest relevant difference is in how function definitions work: the shader debug info extension avoids forward references.
This PR is a sequence of commits that clean up some of the debug info code and ultimately change how function definitions are handled to map the shader debug info extension.