Expose shader validation #4811

daxpedda · 2023-11-30T14:55:24Z

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?

Wumpf · 2023-12-02T09:25:08Z

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 · 2023-12-02T10:41:12Z

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 · 2023-12-02T11:58:42Z

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

cwfitzgerald

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

wgpu-core/src/lib.rs

Co-authored-by: Connor Fitzgerald <[email protected]>

…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

…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

daxpedda requested a review from a team as a code owner November 30, 2023 14:55

cwfitzgerald requested changes Dec 6, 2023

View reviewed changes

wgpu-core/src/lib.rs Show resolved Hide resolved

Expose shader validation

378e8fe

daxpedda force-pushed the patch-1 branch from acd5912 to 378e8fe Compare December 6, 2023 20:26

daxpedda commented Dec 6, 2023

View reviewed changes

wgpu-core/src/lib.rs Outdated Show resolved Hide resolved

cwfitzgerald reviewed Dec 6, 2023

View reviewed changes

wgpu-core/src/lib.rs Outdated Show resolved Hide resolved

Update wgpu-core/src/lib.rs

b19ddb4

cwfitzgerald approved these changes Dec 6, 2023

View reviewed changes

cwfitzgerald enabled auto-merge (squash) December 6, 2023 20:43

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

Expose shader validation (gfx-rs#4811)

fa172e1

Co-authored-by: Connor Fitzgerald <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose shader validation #4811

Expose shader validation #4811

daxpedda commented Nov 30, 2023

Wumpf commented Dec 2, 2023 •

edited

Loading

daxpedda commented Dec 2, 2023

Wumpf commented Dec 2, 2023

cwfitzgerald left a comment

Expose shader validation #4811

Expose shader validation #4811

Conversation

daxpedda commented Nov 30, 2023

Wumpf commented Dec 2, 2023 • edited Loading

daxpedda commented Dec 2, 2023

Wumpf commented Dec 2, 2023

cwfitzgerald left a comment

Choose a reason for hiding this comment

Wumpf commented Dec 2, 2023 •

edited

Loading