-
Notifications
You must be signed in to change notification settings - Fork 379
Fix missing debug line numbers for repacked struct members in SPIR-V #9028
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
Conversation
…cked struct members Co-authored-by: zangold-nv <[email protected]>
Co-authored-by: zangold-nv <[email protected]>
jkwak-work
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
| // CHECK: DebugTypeMember %{{.*}} %{{.*}} %{{.*}} %uint_6 | ||
| // CHECK: DebugTypeMember %{{.*}} %{{.*}} %{{.*}} %uint_7 | ||
| // CHECK: DebugTypeMember %{{.*}} %{{.*}} %{{.*}} %uint_8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can use [[# @LINE+1]] magic like this.
struct ShaderParameters
{
// CHECK: DebugTypeMember %{{.*}} %{{.*}} %{{.*}} %uint_[[# @LINE+1]]
float4 fillColor;
// CHECK: DebugTypeMember %{{.*}} %{{.*}} %{{.*}} %uint_[[# @LINE+1]]
float edgeThickness;
// CHECK: DebugTypeMember %{{.*}} %{{.*}} %{{.*}} %uint_[[# @LINE+1]]
float4 edgeColor;
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot to modify the test code, according to the above suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the test to use [[# @LINE+1]] magic with CHECK-DAG directives for better maintainability. The test now dynamically computes line numbers, so it won't break if lines are added or removed above the struct definition. Commit: 934a1d0
Co-authored-by: zangold-nv <[email protected]>
jkwak-work
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
Task: Fix missing debug line numbers for repacked struct members ✅
Problem
When compiling shader code that includes structures which get repacked (e.g., _std140), slangc emits SPIR-V code with missing line numbers (set to 0) for the DebugTypeMember instructions of repacked structures.
Root Cause
In
slang-ir-lower-buffer-element-type.cpp, when creating fields for the lowered/repacked struct type, the code reuses the original field keys but those keys didn't have debug location decorations attached. The SPIR-V emitter was always using the struct type's debug location instead of checking the field keys.Solution Implemented
Changes Made
maybeAddDebugLocationDecoration(context, irFieldKey)when creating struct keys so they retain source location information[[# @LINE+1]]magic for dynamic line number checking, making it more maintainableVerification
[[# @LINE+1]]magicThe fix ensures that when structures are repacked for buffer layout (std140, std430, etc.), the debug information preserves the original source line numbers for each struct member instead of using 0 or the struct type's line number.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.