Fix 8601 check for RayQuery user type - #8610
Fix 8601 check for RayQuery user type#8610Brendan Duncan (brendan-duncan) wants to merge 4 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
@microsoft-github-policy-service agree |
Chris B (llvm-beanz)
left a comment
Chris B (llvm-beanz)
left a comment
There was a problem hiding this comment.
Thank you for this contribution!
I'm happy with this PR as-is, but I do (oddly) recommend removing the comments. I realize the comments do explain well the code and why it differs from the code around it, but all the code around it is also a mess of bugs that should change because string-matching type names is bad.
Co-authored-by: Chris B <beanz@abolishcrlf.org>
Co-authored-by: Chris B <beanz@abolishcrlf.org>
|
Agreed with the comment removal. I was debating that myself. |
|
Chris B (@llvm-beanz) Just checking that I'm not missing something from my side for this PR. Not sure what the pending check is "waiting for status to be reported". I looked around for buttons or TODOs I should be doing. |
|
Chris B (@llvm-beanz) sorry for the poke--do you think you can run the azp tests command so this PR can move forward, before I forget about it again? |
|
I just saw that since the change was made I needed to request a re-review, so I did that. |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Fixes SPIR-V handling of user-defined types named RayQuery.
Changes:
- Uses semantic RayQuery detection instead of name matching.
- Adds a regression test for struct shadowing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
AstTypeProbe.cpp |
Refines opaque-type detection. |
LowerTypeVisitor.cpp |
Refines RayQuery lowering. |
type.rayquery.user-defined-shadow.hlsl |
Adds shadowing coverage. |
|
Joshua Batista (@bob80905) I pushed some updates addressing copilot's notes. |
Joshua Batista (bob80905)
left a comment
Joshua Batista (bob80905)
left a comment
There was a problem hiding this comment.
Cool, didn't know you could do implicit check nots on invocation!
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Fixes #8601
Compiling to SPIR-V crashes when a user-defined struct or class is named
RayQuery(a reserved intrinsic name), e.g. a ray-tracing abstraction thatdeclares its own
RayQuerytype inside a namespace.HLSL has no rule forbidding this; the local declaration should shadow the
reserved name, and the struct should compile like any other.
Added tools/clang/test/CodeGenSPIRV/type.rayquery.user-defined-shadow.hlsl,
which reproduces the issue and checks that the shadowing struct lowers to a
normal OpTypeStruct and that no OpTypeRayQueryKHR is emitted.
Verified that the intrinsic RayQuery still lowers to OpTypeRayQueryKHR
and that existing rayquery CodeGenSPIRV tests continue to pass.