Allow unchecked shader modules to also skip (naga's) uniformity checks #3175
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3175 +/- ##
==========================================
- Coverage 64.77% 64.75% -0.02%
==========================================
Files 81 81
Lines 38724 38756 +32
==========================================
+ Hits 25084 25098 +14
- Misses 13640 13658 +18
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
wgpu/src/lib.rs
Outdated
| /// the caller responsibility to pass a shader which doesn't perform any of these | ||
| /// operations. | ||
| /// | ||
| /// This is equivalent to `create_shader_module` effect on web. |
There was a problem hiding this comment.
This isn't actually equivalent. With disabling bounds checks, it doesn't actually change the validity of the shader. This method makes otherwise invalid shaders valid. I think we need a proper feature for this. Maybe UNCHECKED_SHADERS to cover any future validation restrictions too.
There was a problem hiding this comment.
Ah, I was coming at this from 'web=webgpu', which should have uniformity analysis, forgetting the 'web=webgl' case. Thanks for the reminder
There was a problem hiding this comment.
My point is more that, when calling *_unchecked, it doesn't matter if there are bounds checks in the shader or not, so the user doesn't need to know if eliding bounds checks are supported.
When calling *_non_uniform, it does matter if eliding uniformity analysis is supported, because some shaders won't compile. Because of this, we need some way to inform the user if eliding uniformity analysis is supported on the device.
There was a problem hiding this comment.
Possibly? I'd hope use of this API would be relatively rare.
More precisely, I think that the non-uniform part of this API is kind of inherently tied to working around naga, with knowledge of the code it emits to the backend. It's unclear to me what cases are valid wgsl if they don't pass uniformity analysis. That is, if you have a version of your code which uses the fact that uniformity analysis can be disabled, and another one which doesn't, you should always use the one which doesn't.
There was a problem hiding this comment.
If the workgroupBroadcast proposal is accepted (and implemented in naga), then that would be my preferred way to avoid triggering uniformity analysis errors or warnings, and then I would not use this API. However, it might still make sense as a short term solution.
DJMcNab
changed the title
DJMcNab
changed the title
|
Closing this for now. We will revisit if and when naga's uniformity check is improved such that we need the workaround, but at the moment we can just make use of the fact naga doesn't actually check uniformity for the scenario we need. |
Checklist
cargo clippyRan but couldn't find any non-pre-existing errors. Didn't do a complete diff as that felt like a waste of time.
RUSTFLAGS=--cfg=web_sys_unstable_apis cargo clippy --target wasm32-unknown-unknownif applicable.I think not applicable, but ran anyway - no warnings. Please clarify what "if applicable" means here.
Connections
Modelled after #1978, using the same struct this already created.
Fixes #3135
The changes might be removable after gpuweb/gpuweb#3479 is resolved and filters through
Description
When using shaders with
wgpu, it may be necessary to work around gpuweb/gpuweb#3479.To allow this,
Device::create_shader_module_uncheckedhas gained a parameter for configuration. This allows This disablingValidationFlags::CONTROL_FLOW_UNIFORMITYwhen used, but may allow maintaining bounds checks.Testing
I don't believe that we should expect much use of this API; it should primarily be used for exploratory work.
TODO, awaiting results from @raphlinus