Skip to content

[naga] Process overrides selectively for the active entry point #7652

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

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

andyleiserson
Copy link
Contributor

@andyleiserson andyleiserson commented Apr 30, 2025

This PR is intended to fix #5885. As that issue says, a missing override value should only be an error if that value is needed by the active entry point. Currently, we raise an error if any override does not have at least one of a default value in the shader or a pipeline-specified value.

This adds an argument to process_overrides to specify the desired entry point, and then modifies override processing to tolerate missing overrides if (and hopefully only if) they are not used by that entry point.

During that processing, lists are constructed of the functions, globals, and entry points that cannot be used due to missing overrides. The MSL backend is changed to skip over those items while writing.

Opening as a draft to confirm the approach. Still some work left:

  • update the other backends
  • make save_overrides_resolved a bit nicer
  • add some more tests

An alternate approach would be to rewrite the module after evaluating overrides. This would enable a more robust validation pass to confirm there are no remaining overrides, and if structured based on writing items reachable from the entrypoint rather than excluding items referencing unresolved overrides, would allow the backend to write a more minimal shader (as @cwfitzgerald mentioned here). But the current rewriting process for override evaluation does not do what is needed -- it requires all overrides be resolved, so there's never a need to remove any functions, entry points, or globals. The existing compaction operation also does not provide what we need, because it treats all functions, entry points, and globals as implicitly used. (Also, we compact the module when initially parsing the shader, before applying per-pipeline overrides, so we'd either need to adjust when compaction happens, or add a new compaction pass.)

It is also possible for there to be types (override-sized arrays) that depend on an unresolved override. Array sizes are resolved in the backend (see #6787). I've currently made the msl backend silently skip emitting unresolved override-sized arrays, but it might be better to add a fourth member in UnresolvedOverrides for types, to make this safer.

Testing
Adds tests to the naga validation suite and a snapshot test.

Squash or Rebase? Squash, tentatively

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

This adds an argument to `process_overrides` to specify the desired
entry point, and then modifies override processing to tolerate
missing overrides if (and hopefully only if) they are not used
by that entry point.

During that processing, lists are constructed of the functions, globals,
and entry points that cannot be used due to missing overrides. The MSL
backend is changed to skip over those items while writing.

- [ ] update the other backends
- [ ] make save_overrides_resolved a bit nicer
- [ ] add some more tests
@andyleiserson
Copy link
Contributor Author

Discussing with @jimblandy, we think it is probably better to do this through compaction, even if it comes at the cost of adding another compaction pass.

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.

Naga shouldn't evaluate overrides unless they're used by the entry point
1 participant