Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use line and column from node on DebugLexicalBlock. #3737

Merged
merged 4 commits into from
Oct 4, 2024

Conversation

sajjadmirzanv
Copy link
Contributor

Old implementation had every DebugLexicalBlock hardcoded to (0, 0), but IR nodes know their source location so we may as well attach it to the DebugLexicalBlock.

@arcady-lunarg
Copy link
Contributor

You should add a test that exercises this functionality.

@sajjadmirzanv
Copy link
Contributor Author

You should add a test that exercises this functionality.

I discovered that there was a bug in the test runner, which is why the existing nonsemantic tests were not already triggering this code. Running glslang -gVS on those files emits DebugLexicalLine, but it was missing when run through glslangtests.

In TestFixture.h, the code was only generating debug info for the 2nd stage of compilation, from IR -> SPIRV. I enabled the flag so it also emits debug info during Source Language -> IR compilation. The result is that we get EOpScope nodes in the tree, so the GlslangToSpv converter can emit DebugLexicalBlock instructions.

I had to make a few other small changes to some tests as well.

if (enableDebug) {
shader.setDebugInfo(true);
}
if (enableNonSemanticShaderDebugInfo) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this forcing of correct usage. We might want to force enableNonSemanticShaderDebugInfo to set enableDebug at a lower level, but I will investigate that in a separate PR.

@arcady-lunarg arcady-lunarg added the kokoro:run Trigger Google bot runs label Sep 30, 2024
@kokoro-team kokoro-team removed the kokoro:run Trigger Google bot runs label Sep 30, 2024
Copy link
Contributor

@arcady-lunarg arcady-lunarg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I think this looks reasonable.

@arcady-lunarg arcady-lunarg merged commit f69d276 into KhronosGroup:main Oct 4, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants