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

Stale access permissions for overlapping descriptors #2351

Merged
merged 2 commits into from
Jul 15, 2024

Conversation

js6i
Copy link
Contributor

@js6i js6i commented Jul 10, 2024

No description provided.

@js6i
Copy link
Contributor Author

js6i commented Jul 10, 2024

Looks like there's a dependency in the other direction after all in the other tests sets I forgot about.. I'll look for a less invasive solution, maybe a force_recompile() will do.

@js6i js6i force-pushed the entry_point_decls branch from cb59527 to b37f159 Compare July 11, 2024 10:00
@js6i
Copy link
Contributor Author

js6i commented Jul 11, 2024

My explanation vanished with the old commit..

The issue is that emit_entry_point_declarations is called before fixups, and the function I added force_recompile to sets the name for aliasing descriptor that contains its access pattern. If the access gets changed, and there are no further recompilations, emit_entry_point_declarations might use the old name.

Here's a repro (non-minimal, sorry): https://gist.github.com/js6i/cb407e7f15b71cf59713106ff5fede12

./spirv-cross --msl REPRO.spv --msl-argument-buffers --msl-argument-buffer-tier 1 --msl-version 30100 --msl-force-active-argument-buffer-resources --msl-device-argument-buffer 0 --msl-device-argument-buffer 3 --msl-device-argument-buffer 5

@HansKristian-Work
Copy link
Contributor

I tried a cleaner method, but that doesn't work either, since we depend on the qualified alias of the base resource. Guess this is the best we can do for now.

@HansKristian-Work HansKristian-Work merged commit d17a2d6 into KhronosGroup:main Jul 15, 2024
6 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.

2 participants