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

Make the WGSL frontend optional #2549

Closed
relrelb opened this issue Mar 21, 2022 · 2 comments
Closed

Make the WGSL frontend optional #2549

relrelb opened this issue Mar 21, 2022 · 2 comments
Labels
area: ecosystem Help the connected projects grow and prosper

Comments

@relrelb
Copy link

relrelb commented Mar 21, 2022

Is your feature request related to a problem? Please describe.
As of today, wgpu supports only the WGSL frontend by default, and other frontends are gated behind Cargo features. This is great, because this reduces the binary sizes and build times significantly, but I wonder why there's no way to disable the WGSL frontend as well. I understand that WebGPU supports exclusively WGSL, but I don't see a reason for desktop targets to be required to pay the extra binary size and build time, even if they only use the SPIR-V frontend for instance.

Describe the solution you'd like
Disable the WGSL backend by default, and introduce a wgsl Cargo feature to enable it, pretty much as done in #1518 for SPIR-V. Since this will make all frontends optional, a static assert can be added to enforce that at least one is enabled.

Describe alternatives you've considered
This would have been a less critical problem if the Rust compiler was able to eliminate the WGSL frontend code when it's unused, because then only a longer build time remains. But from a simple experiment this doesn't work, at least not trivially, even with lto = "fat". Functions like naga::front::wgsl::parse_str still appear in the executable. I suspect this optimization opportunity is missed because of the use of enums and matches throughout the code that handles shaders (e.g. wgpu::ShaderSource and create_shader_module).

Additional context
I very appreciate your work, it's truly awesome.

@kvark
Copy link
Member

kvark commented Mar 21, 2022

Thank you for filing this!

My stance here is that WGSL should be kept always enabled. Support for WGSL is essential to the library - it's a blessed import format, unlike SPIR-V and GLSL. For example, currently atomics only work on WGSL.
Secondly, Naga's IR is very much inspired by WGSL, and therefore parsing is very straightforward. This is much unlike SPIR-V and GLSL, which have to do wonders in order to parse correctly.

That is to say, discussion is open. If you want to provide more concrete numbers on the binary size (that could be saved), that would help.

@kvark kvark added the area: ecosystem Help the connected projects grow and prosper label Mar 21, 2022
@daxpedda
Copy link
Contributor

daxpedda commented Dec 1, 2023

I believe this was already done in #2890.
As stated by kvark, we didn't actually remove it by default, but if you use default-features = false you can get rid of it if you don't need/want it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ecosystem Help the connected projects grow and prosper
Projects
None yet
Development

No branches or pull requests

4 participants