Skip to content

[naga] Ensure that FooResult expressions are correctly populated.#5771

Merged
teoxoy merged 1 commit intogfx-rs:trunkfrom
jimblandy:naga-result-population
Jun 4, 2024
Merged

[naga] Ensure that FooResult expressions are correctly populated.#5771
teoxoy merged 1 commit intogfx-rs:trunkfrom
jimblandy:naga-result-population

Conversation

@jimblandy
Copy link
Copy Markdown
Member

Make Naga module validation require that CallResult and AtomicResult expressions are indeed visited by exactly one Call / Atomic statement.

@jimblandy jimblandy added type: bug Something isn't working area: validation Issues related to validation, diagnostics, and error handling naga Shader Translator labels Jun 3, 2024
@jimblandy jimblandy requested a review from a team June 3, 2024 15:22
@jimblandy jimblandy force-pushed the naga-result-population branch from 100d101 to b9a6730 Compare June 3, 2024 15:25
Make Naga module validation require that `CallResult` and
`AtomicResult` expressions are indeed visited by exactly one `Call` /
`Atomic` statement.
@jimblandy jimblandy force-pushed the naga-result-population branch from b9a6730 to f83ffed Compare June 3, 2024 15:26
Copy link
Copy Markdown
Member

@teoxoy teoxoy left a comment

Choose a reason for hiding this comment

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

Looks good!

@teoxoy teoxoy merged commit 583cc6a into gfx-rs:trunk Jun 4, 2024
@jimblandy jimblandy deleted the naga-result-population branch June 4, 2024 21:05
jimblandy added a commit to atlv24/wgpu that referenced this pull request Jun 4, 2024
Rather than introducing an entirely new statement variant
`AtomicNoResult`, make `Statement::Atomic::result` an `Option`. This
reduces the patch size by about a third, and has no effect on snapshot
output other than the direct IR dumps.

In HLSL output, don't bother to generated "discard" temporaries for
functions where the final `original_value` parameter is not required.
This improves snapshot output.

Improve documentation for `Expression::AtomicResult`,
`Statement::Atomic`, and the new `valid::Capabilities` flags.

Rewrite `Atomic` statement validation. Consolidate validation of
`AtomicResult` expressions with this; gfx-rs#5771 ensures that there is
indeed some `Atomic` statement referring to every `AtomicResult`
expression, so we can be sure it will be validated.

Fixes gfx-rs#5742.
@jimblandy jimblandy mentioned this pull request Jun 4, 2024
6 tasks
jimblandy added a commit to atlv24/wgpu that referenced this pull request Jun 4, 2024
Rather than introducing an entirely new statement variant
`AtomicNoResult`, make `Statement::Atomic::result` an `Option`. This
reduces the patch size by about a third, and has no effect on snapshot
output other than the direct IR dumps.

In HLSL output, don't bother to generated "discard" temporaries for
functions where the final `original_value` parameter is not required.
This improves snapshot output.

Improve documentation for `Expression::AtomicResult`,
`Statement::Atomic`, and the new `valid::Capabilities` flags.

Rewrite `Atomic` statement validation. Consolidate validation of
`AtomicResult` expressions with this; gfx-rs#5771 ensures that there is
indeed some `Atomic` statement referring to every `AtomicResult`
expression, so we can be sure it will be validated.

Fixes gfx-rs#5742.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: validation Issues related to validation, diagnostics, and error handling naga Shader Translator type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants