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

Shader module unchecked doesn't disable bounds checking codegen #3483

Closed
unfolding18 opened this issue Feb 13, 2023 · 1 comment · Fixed by #3603
Closed

Shader module unchecked doesn't disable bounds checking codegen #3483

unfolding18 opened this issue Feb 13, 2023 · 1 comment · Fixed by #3603
Labels
api: metal Issues with Metal help required We need community help to make this happen. type: bug Something isn't working

Comments

@unfolding18
Copy link

Description
Naga adds additional bounds checking code to WGSL shaders prior to compilation. It is configurable via Naga, but does not appear to work in wgpu-rs itself. In particular, create_shader_module_unchecked() does not disable bounds checking. Note that I have only checked this on the Metal backend, as that is the hardware I am using/targeting.

Repro steps
Create a WGSL shader that indexes into any array, buffer, matrix, or vec. Set wgpu to use the Metal backend. Run the executable through Xcode's shader debugger. You should see that, regardless of whether you use create_shader_module or create_shader_module_unchecked, the resulting Metal shader code contains various conditionals and bounds checking routines.

Expected vs observed behavior
Ideally, create_shader_module_unchecked would disable bounds checking (or this would be configurable somehow via the API).

Platform
MacOS Ventura 13.2, wgpu version 0.14.0, Metal backend.

@cwfitzgerald cwfitzgerald added type: bug Something isn't working help required We need community help to make this happen. api: metal Issues with Metal labels Feb 15, 2023
@cwfitzgerald
Copy link
Member

cwfitzgerald commented Feb 15, 2023

This appears to be a two sided problem. First is that we just are never changing the bounds check policies we pass to naga. See

bounds_check_policies: naga::proc::BoundsCheckPolicies {
index: naga::proc::BoundsCheckPolicy::ReadZeroSkipWrite,
buffer: naga::proc::BoundsCheckPolicy::ReadZeroSkipWrite,
image: naga::proc::BoundsCheckPolicy::ReadZeroSkipWrite,
// TODO: support bounds checks on binding arrays
binding_array: naga::proc::BoundsCheckPolicy::Unchecked,
},
for where they are hard coded. Second we are setting them in the pipline layout, when the unchecked-ness comes on a per-module basis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: metal Issues with Metal help required We need community help to make this happen. type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants