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

Expose shader validation #4811

Merged
merged 2 commits into from
Dec 6, 2023
Merged

Expose shader validation #4811

merged 2 commits into from
Dec 6, 2023

Conversation

daxpedda
Copy link
Contributor

I was trying to pre-compile shaders while still running the exact same validation that wgpu does.
Would it be acceptable to expose this module for that purpose?

@daxpedda daxpedda requested a review from a team as a code owner November 30, 2023 14:55
@Wumpf
Copy link
Member

Wumpf commented Dec 2, 2023

I think the better way to go about this is to call https://docs.rs/naga/0.14.1/naga/valid/struct.Validator.html#method.validate
which is most of what wgpu does. Is there a reason this would miss something in your case?

let module =
    naga::front::wgsl::parse_str(&shader.source).expect("should parse :O");

let mut validator = naga::valid::Validator::new(
    naga::valid::ValidationFlags::all(),
    naga::valid::Capabilities::all(),
);

validator.validate(&module).expect("should be valid!");

The nice thing about this is that you can then at least skip the parsing on native wgpu since you're allowed to pass in a naga module for creating the shader module https://github.com/gfx-rs/wgpu/blob/trunk/wgpu/src/lib.rs#L576

@daxpedda
Copy link
Contributor Author

daxpedda commented Dec 2, 2023

What you are describing is a different use-case, one which I've used pretty successfully so far!

I'm talking specifically about passing raw SPIRV modules, which I would generate externally first. Wgpu currently does a bunch of validation, including using the Naga Validator, but that's not all it does. E.g. Interface::check_stage() does a lot of checking against the render pipeline, which is completely skipped when providing a raw SPIRV shader.

The goal is to replicate exactly what Wgpu does internally, most of this stuff is exposed and easy to re-use. But the whole validation going on in wgpu_core::validation will require me to almost copy the whole file. Which, alternatively, I can do as well.

I was told before that large parts of the wgpu-core and wgpu-hal API are not exactly considered stable, so I thought exposing this isn't an issue.

Cc #3103, which would make this pointless as well.

@Wumpf
Copy link
Member

Wumpf commented Dec 2, 2023

ah I see, thanks for elaborating, makes sense! I'd be in favor of exposing then as well, but I'd like to hear what @cwfitzgerald says

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love it, but can't argue against it.

wgpu-core/src/lib.rs Show resolved Hide resolved
wgpu-core/src/lib.rs Outdated Show resolved Hide resolved
wgpu-core/src/lib.rs Outdated Show resolved Hide resolved
@cwfitzgerald cwfitzgerald enabled auto-merge (squash) December 6, 2023 20:43
@cwfitzgerald cwfitzgerald merged commit 3e0fb2c into gfx-rs:trunk Dec 6, 2023
27 checks passed
bradwerth pushed a commit to bradwerth/wgpu that referenced this pull request Dec 8, 2023
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 9, 2023
…eb3c29ef785. r=webgpu-reviewers,supply-chain-reviewers,jimb

# Changelog

 * #4807 Bump wasm-bindgen-test from 0.3.38 to 0.3.39
   By dependabot[bot] in gfx-rs/wgpu#4807
 * #4830 Use Display instead of Debug to log errors.
   By nical in gfx-rs/wgpu#4830
 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers.
   By jimblandy in gfx-rs/wgpu#4755
 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers.
   By jimblandy in gfx-rs/wgpu#4755
 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers.
   By jimblandy in gfx-rs/wgpu#4755
 * #4832 vulkan: fix multi-planar texture creation
   By xiaopengli89 in gfx-rs/wgpu#4832
 * #4758 [vk] remove (old) unused blocklist for dual source blending
   By teoxoy in gfx-rs/wgpu#4758
 * #4828 Remove DX11 backend
   By valaphee in gfx-rs/wgpu#4828
 * #4836 [gl] add support for line and point polygon modes
   By valaphee in gfx-rs/wgpu#4836
 * #4820 Bump futures-lite from 2.0.1 to 2.1.0
   By dependabot[bot] in gfx-rs/wgpu#4820
 * #4811 Expose shader validation
   By daxpedda in gfx-rs/wgpu#4811
 * #3507 [wgpu-hal] Inline RayQuery Support
   By daniel-keitel in gfx-rs/wgpu#3507
 * #4726 Fix Javascript exception on repeated `BufferSlice::get_mapped_range` calls
   By DouglasDwyer in gfx-rs/wgpu#4726
 * #4841 Remove `expose-ids` Feature
   By cwfitzgerald in gfx-rs/wgpu#4841
 * #4843 Some Minor `wgpu-core` Cleanups
   By cwfitzgerald in gfx-rs/wgpu#4843
 * #4844 Work around cbindgen issue
   By nical in gfx-rs/wgpu#4844

Differential Revision: https://phabricator.services.mozilla.com/D195735
aosmond pushed a commit to aosmond/gecko that referenced this pull request Dec 10, 2023
…eb3c29ef785. r=webgpu-reviewers,supply-chain-reviewers,jimb

# Changelog

 * #4807 Bump wasm-bindgen-test from 0.3.38 to 0.3.39
   By dependabot[bot] in gfx-rs/wgpu#4807
 * #4830 Use Display instead of Debug to log errors.
   By nical in gfx-rs/wgpu#4830
 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers.
   By jimblandy in gfx-rs/wgpu#4755
 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers.
   By jimblandy in gfx-rs/wgpu#4755
 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers.
   By jimblandy in gfx-rs/wgpu#4755
 * #4832 vulkan: fix multi-planar texture creation
   By xiaopengli89 in gfx-rs/wgpu#4832
 * #4758 [vk] remove (old) unused blocklist for dual source blending
   By teoxoy in gfx-rs/wgpu#4758
 * #4828 Remove DX11 backend
   By valaphee in gfx-rs/wgpu#4828
 * #4836 [gl] add support for line and point polygon modes
   By valaphee in gfx-rs/wgpu#4836
 * #4820 Bump futures-lite from 2.0.1 to 2.1.0
   By dependabot[bot] in gfx-rs/wgpu#4820
 * #4811 Expose shader validation
   By daxpedda in gfx-rs/wgpu#4811
 * #3507 [wgpu-hal] Inline RayQuery Support
   By daniel-keitel in gfx-rs/wgpu#3507
 * #4726 Fix Javascript exception on repeated `BufferSlice::get_mapped_range` calls
   By DouglasDwyer in gfx-rs/wgpu#4726
 * #4841 Remove `expose-ids` Feature
   By cwfitzgerald in gfx-rs/wgpu#4841
 * #4843 Some Minor `wgpu-core` Cleanups
   By cwfitzgerald in gfx-rs/wgpu#4843
 * #4844 Work around cbindgen issue
   By nical in gfx-rs/wgpu#4844

Differential Revision: https://phabricator.services.mozilla.com/D195735
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Dec 12, 2023
…eb3c29ef785. r=webgpu-reviewers,supply-chain-reviewers,jimb

# Changelog

 * #4807 Bump wasm-bindgen-test from 0.3.38 to 0.3.39
   By dependabot[bot] in gfx-rs/wgpu#4807
 * #4830 Use Display instead of Debug to log errors.
   By nical in gfx-rs/wgpu#4830
 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers.
   By jimblandy in gfx-rs/wgpu#4755
 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers.
   By jimblandy in gfx-rs/wgpu#4755
 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers.
   By jimblandy in gfx-rs/wgpu#4755
 * #4832 vulkan: fix multi-planar texture creation
   By xiaopengli89 in gfx-rs/wgpu#4832
 * #4758 [vk] remove (old) unused blocklist for dual source blending
   By teoxoy in gfx-rs/wgpu#4758
 * #4828 Remove DX11 backend
   By valaphee in gfx-rs/wgpu#4828
 * #4836 [gl] add support for line and point polygon modes
   By valaphee in gfx-rs/wgpu#4836
 * #4820 Bump futures-lite from 2.0.1 to 2.1.0
   By dependabot[bot] in gfx-rs/wgpu#4820
 * #4811 Expose shader validation
   By daxpedda in gfx-rs/wgpu#4811
 * #3507 [wgpu-hal] Inline RayQuery Support
   By daniel-keitel in gfx-rs/wgpu#3507
 * #4726 Fix Javascript exception on repeated `BufferSlice::get_mapped_range` calls
   By DouglasDwyer in gfx-rs/wgpu#4726
 * #4841 Remove `expose-ids` Feature
   By cwfitzgerald in gfx-rs/wgpu#4841
 * #4843 Some Minor `wgpu-core` Cleanups
   By cwfitzgerald in gfx-rs/wgpu#4843
 * #4844 Work around cbindgen issue
   By nical in gfx-rs/wgpu#4844

Differential Revision: https://phabricator.services.mozilla.com/D195735

UltraBlame original commit: 0dfeaf41028d13f9dd98b106e262c1deb255398d
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Dec 12, 2023
…eb3c29ef785. r=webgpu-reviewers,supply-chain-reviewers,jimb

# Changelog

 * #4807 Bump wasm-bindgen-test from 0.3.38 to 0.3.39
   By dependabot[bot] in gfx-rs/wgpu#4807
 * #4830 Use Display instead of Debug to log errors.
   By nical in gfx-rs/wgpu#4830
 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers.
   By jimblandy in gfx-rs/wgpu#4755
 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers.
   By jimblandy in gfx-rs/wgpu#4755
 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers.
   By jimblandy in gfx-rs/wgpu#4755
 * #4832 vulkan: fix multi-planar texture creation
   By xiaopengli89 in gfx-rs/wgpu#4832
 * #4758 [vk] remove (old) unused blocklist for dual source blending
   By teoxoy in gfx-rs/wgpu#4758
 * #4828 Remove DX11 backend
   By valaphee in gfx-rs/wgpu#4828
 * #4836 [gl] add support for line and point polygon modes
   By valaphee in gfx-rs/wgpu#4836
 * #4820 Bump futures-lite from 2.0.1 to 2.1.0
   By dependabot[bot] in gfx-rs/wgpu#4820
 * #4811 Expose shader validation
   By daxpedda in gfx-rs/wgpu#4811
 * #3507 [wgpu-hal] Inline RayQuery Support
   By daniel-keitel in gfx-rs/wgpu#3507
 * #4726 Fix Javascript exception on repeated `BufferSlice::get_mapped_range` calls
   By DouglasDwyer in gfx-rs/wgpu#4726
 * #4841 Remove `expose-ids` Feature
   By cwfitzgerald in gfx-rs/wgpu#4841
 * #4843 Some Minor `wgpu-core` Cleanups
   By cwfitzgerald in gfx-rs/wgpu#4843
 * #4844 Work around cbindgen issue
   By nical in gfx-rs/wgpu#4844

Differential Revision: https://phabricator.services.mozilla.com/D195735

UltraBlame original commit: 0dfeaf41028d13f9dd98b106e262c1deb255398d
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Dec 12, 2023
…eb3c29ef785. r=webgpu-reviewers,supply-chain-reviewers,jimb

# Changelog

 * #4807 Bump wasm-bindgen-test from 0.3.38 to 0.3.39
   By dependabot[bot] in gfx-rs/wgpu#4807
 * #4830 Use Display instead of Debug to log errors.
   By nical in gfx-rs/wgpu#4830
 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers.
   By jimblandy in gfx-rs/wgpu#4755
 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers.
   By jimblandy in gfx-rs/wgpu#4755
 * #4755 [naga wgsl-in] Automatic conversions for `var` initializers.
   By jimblandy in gfx-rs/wgpu#4755
 * #4832 vulkan: fix multi-planar texture creation
   By xiaopengli89 in gfx-rs/wgpu#4832
 * #4758 [vk] remove (old) unused blocklist for dual source blending
   By teoxoy in gfx-rs/wgpu#4758
 * #4828 Remove DX11 backend
   By valaphee in gfx-rs/wgpu#4828
 * #4836 [gl] add support for line and point polygon modes
   By valaphee in gfx-rs/wgpu#4836
 * #4820 Bump futures-lite from 2.0.1 to 2.1.0
   By dependabot[bot] in gfx-rs/wgpu#4820
 * #4811 Expose shader validation
   By daxpedda in gfx-rs/wgpu#4811
 * #3507 [wgpu-hal] Inline RayQuery Support
   By daniel-keitel in gfx-rs/wgpu#3507
 * #4726 Fix Javascript exception on repeated `BufferSlice::get_mapped_range` calls
   By DouglasDwyer in gfx-rs/wgpu#4726
 * #4841 Remove `expose-ids` Feature
   By cwfitzgerald in gfx-rs/wgpu#4841
 * #4843 Some Minor `wgpu-core` Cleanups
   By cwfitzgerald in gfx-rs/wgpu#4843
 * #4844 Work around cbindgen issue
   By nical in gfx-rs/wgpu#4844

Differential Revision: https://phabricator.services.mozilla.com/D195735

UltraBlame original commit: 0dfeaf41028d13f9dd98b106e262c1deb255398d
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.

3 participants