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

spirv-val: Add SPV_KHR_ray_tracing storage class #4868

Merged
merged 5 commits into from
Aug 29, 2022

Conversation

sjfricke
Copy link
Contributor

Adds validation to make sure all the ray tracing Storage Class are using the correct execution model. Also HitAttributeKHR and ShaderRecordBufferKHR have read/write limitations. There is already validation for the "cannot have initializers" marked as VUID-StandaloneSpirv-OpVariable-04651

(tasklist KhronosGroup/Vulkan-Docs#1402)

Copy link
Contributor

@alan-baker alan-baker left a comment

Choose a reason for hiding this comment

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

Are the VUIDs not attachable in RegisterStorageClassConsumer?

@sjfricke
Copy link
Contributor Author

So right now waiting on KhronosGroup/Vulkan-Docs#1895 to add the VUs, if they decide to keep/add the missing ones I will come back and label them

@alan-baker
Copy link
Contributor

Maybe I'm confused but https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-StandaloneSpirv-IncomingRayPayloadKHR-04699 and other around there seem just like this. Restrictions on the execution models that can use a storage class.

@sjfricke
Copy link
Contributor Author

I guess my logic was these VUs language (and a few that are missing) are already listed in the SPV_KHR_ray_tracing spec

@sjfricke
Copy link
Contributor Author

sjfricke commented Aug 4, 2022

@alan-baker added the VU and when the KhronosGroup/Vulkan-Docs#1895 is merged and upstreamed will add the final 2 labels

@sjfricke
Copy link
Contributor Author

Had to fix that the Intersection stage can be read from with HitAttribute storage class

Also think I fixed the CI issue with clang-format, not sure what version the CI is using and the utils/clang-format-diff.py script it calls it in the .gitignore and I can't run locally

@sjfricke
Copy link
Contributor Author

ERROR: /Volumes/BuildData/tmpfs/src/github/SPIRV-Tools/BUILD.bazel:550:10: Compiling test/val/val_decoration_test.cpp failed: (Aborted): wrapped_clang_pp failed: error executing command external/local_config_cc/wrapped_clang_pp '-D_FORTIFY_SOURCE=1' -fstack-protector -fcolor-diagnostics -Wall -Wthread-safety -Wself-assign -fno-omit-frame-pointer -O0 -DDEBUG '-std=c++11' ... (remaining 101 arguments skipped)

yet I didn't touch that file, rebased, hope this finally fixes/pleases CI (cc @alan-baker )

@alan-baker alan-baker merged commit f76431c into KhronosGroup:master Aug 29, 2022
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.

None yet

3 participants