-
Notifications
You must be signed in to change notification settings - Fork 973
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
Naga features for platform dependent compilation of msl & hlsl #5919
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the backends won't be enabled if they won't be used on the target but it seems not because the conditional target compilation is setup in wgpu-core
and happens after feature selection. It doesn't feel right that naga gets those new cfgs though; I would say that wgpu-hal
is at fault here for not conditionally enabling naga
's features based on the target. We already have some target cfgs in wgpu-hal/Cargo.toml
, I think we should add naga in those instead of adding new features to naga
. Thoughts?
agreed, if possible we should solve it on that level instead 🤔 |
|
it works if a feature should be dragged in or not on a per-target basis. The problem here is that it's a combination of feature plus target condition that we want this to be active upon. I.e. what we want is: iff BOTH The left hand side is easy since cargo supports that directly: [target.'cfg(not(target_arch = "wasm32"))'.dependencies]
khronos-egl = { version = "6", features = ["dynamic"], optional = true }
#...
[target.'cfg(target_os = "emscripten")'.dependencies]
khronos-egl = { version = "6", features = ["static", "no-pkg-config"] } The thing I'm struggling here with is to evaluate both conditions at the same time |
Ah yeah, that's problematic. I previously didn't fully grasp what you meant in your previous comment (#5919 (comment)). I can't come up with a better solution either. I will take another look at this tomorrow. |
Rather than `feature = "blah"`, use the new `cfg` identifiers introduced by the `cfg_aliases` invocation in `naga/build.rs` to decide whether to compile the `naga::back::continue_forward` module, which is only used by the GLSL and HLSL backends. The `hlsl_out` `cfg` identifer has a more complex condition than just `feature = "hlsl-out"`, introduced by gfx-rs#5919.
Rather than `feature = "blah"`, use the new `cfg` identifiers introduced by the `cfg_aliases` invocation in `naga/build.rs` to decide whether to compile the `naga::back::continue_forward` module, which is only used by the GLSL and HLSL backends. The `hlsl_out` `cfg` identifer has a more complex condition than just `feature = "hlsl-out"`, introduced by gfx-rs#5919. Fixes gfx-rs#6063.
Rather than `feature = "blah"`, use the new `cfg` identifiers introduced by the `cfg_aliases` invocation in `naga/build.rs` to decide whether to compile the `naga::back::continue_forward` module, which is only used by the GLSL and HLSL backends. The `hlsl_out` `cfg` identifer has a more complex condition than just `feature = "hlsl-out"`, introduced by gfx-rs#5919. Fixes gfx-rs#6063.
Rather than `feature = "blah"`, use the new `cfg` identifiers introduced by the `cfg_aliases` invocation in `naga/build.rs` to decide whether to compile the `naga::back::continue_forward` module, which is only used by the GLSL and HLSL backends. The `hlsl_out` `cfg` identifer has a more complex condition than just `feature = "hlsl-out"`, introduced by gfx-rs#5919. Fixes gfx-rs#6063.
Rather than `feature = "blah"`, use the new `cfg` identifiers introduced by the `cfg_aliases` invocation in `naga/build.rs` to decide whether to compile the `naga::back::continue_forward` module, which is only used by the GLSL and HLSL backends. The `hlsl_out` `cfg` identifer has a more complex condition than just `feature = "hlsl-out"`, introduced by #5919. Fixes #6063.
Connections
Description
Adds
cfg_alias
indirection to Naga's x_out features which allows us to introduce new features for platform dependent compilation. This in turn allows wgpu-hal to request shading language output more conditionally.Drawbacks:
Testing
Eyeballing on Windows. Single sample of
cargo clean && cargo build -p wgpu --timings
looked even more promising than I would have imagined:Before:
After:
1.4s cpu time saved on this setup for not actually touching anything 😮. When building wgpu this is even on the hot path, meaning the the wgpu build as a whole got that much faster!
Edit: as expected lots of noise here. Running again on trunk I got 7.0s and 6.2s here, so above trunk compilation is likely with cold file caches. ymmv.
TODO:
Checklist
cargo fmt
.cargo clippy
. If applicable, add:--target wasm32-unknown-unknown
--target wasm32-unknown-emscripten
cargo xtask test
to run tests.CHANGELOG.md
. See simple instructions inside file.