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

Refactor WGSL front end's MathFunction handling #7442

Open
wants to merge 20 commits into
base: trunk
Choose a base branch
from

Conversation

jimblandy
Copy link
Member

This depends on #6833.

To prepare for further use of the OverloadSet trait, refactor the code in naga::front::wgsl::lower::Lowerer::call that handles calls to builtin functions in the MathFunctions enum. This results in three new methods on Lowerer:

  • resolve_overloads, which does the interesting work of selecting an overload based on the argument types at the call, generating diagnostics if something goes wrong, or returning a specific overload if all goes well;
  • apply_automatic_conversions_for_call, which takes care of actually applying whatever automatic conversions are needed to apply those arguments to that overload;
  • and math_function_helper, which uses the above two to build an Expression::Math expression.

@jimblandy jimblandy requested a review from a team as a code owner March 27, 2025 03:09
@jimblandy jimblandy added naga Shader Translator kind: refactor Making existing function faster or nicer area: naga front-end lang: WGSL WebGPU Shading Language labels Mar 27, 2025
@cwfitzgerald cwfitzgerald marked this pull request as draft April 2, 2025 15:18
jimblandy added 20 commits April 2, 2025 14:12
Make the associated function `TypeInner::automatically_converts_to`
available outside the WGSL front end by moving it into the
`proc::type_methods` module.
Add a new module, `naga::common::predeclared`, which implements a new
`struct_name` method on `naga::ir::PredeclaredType`, to produce the
name of the struct type corresponding to that `PredeclaredType` value.
Use this new method in `naga::front::type_gen`.
In the implementation of `TryToWgsl` for `Scalar`, order types within
each representation from smallest to largest. This is just a cleanup;
the change has no effect on behavior.
Define two new types in `naga::common`, `DiagnosticDisplay` and
`DiagnosticDebug`, that allow Naga IR types to be formatted using
`core::fmt::Display` and `core::fmt::Debug`.

In `naga::common::wgsl`, add supporting implementations of
`TypeContext` for `GlobalCtx` and `UniqueArena<Type>`
In `naga::front::wgsl::lower`, add a new method,
`ExpressionContext::is_const`, for looking up the constness of a given
expression. Change existing code to use it.
Define a new helper function on `naga::ir::TypeInner`,
`scalar_for_conversions`, that returns the leaf scalar of a type as
relevant for WGSL-style automatic conversions. Specifically, this
means that, unlike `TypeInner::scalar`, arrays are considered to have
leaf scalars if their elements do.
Introduce traits to add `for_debug` methods to Naga IR types that can
be formatted with `core::fmt::Debug`, given adequate context.
Define a new trait, `proc::builtins::OverloadSet`, for types that
represent a Naga IR builtin function's set of overloads. The
`OverloadSet` trait includes operations needed to validate calls,
choose automatic type conversions, and generate diagnostics.

Add a new function, `ir::MathFunction::overloads`, which returns the
given `MathFunction`'s set of overloads as an `impl OverloadSet`
value. Use this in the WGSL front end, the validator, and the
typifier.

To support `MathFunction::overloads`, provide two implementations
of `OverloadSet`:

- `List` is flexible but verbose.

- `Regular` is concise but more restrictive.

Some snapshot output is affected because `TypeResolution::Handle`
values turn into `TypeResolution::Value`, since the function database
constructs the return type directly.

To work around gfx-rs#7405, avoid offering abstract-typed overloads of some
functions.

This addresses gfx-rs#6443 for `MathFunction`, although that issue covers
other categories of operations as well.
In `naga::front::wgsl::lower::Lowerer::call`, use only the
`unconverted_arguments` vector, rather that using both `arguments` and
`unconverted_arguments`.
In `naga::front::wgsl::lower::Lowerer::call`, when building `Math`
expressions, build the vector of lowered but not yet converted
argument expressions completely before starting overload resolution.
Then iterate over those lowered arguments, not the ast arguments.

This is a minor refactor that prepares for later cleanups.
In `naga::front::wgsl::lower::Lowerer::call`, when building `Math`
expressions, replace some uses of the `arguments` vector with
uses of `unconverted_arguments`.

This is a minor refactor that prepares for later cleanups.
In `naga::front::wgsl::lower::Lowerer::call`, when building `Math`
expressions, use `unconverted_arguments` to check the number of
arguments to the call. This means that `arguments` is never used in
this branch once we have built `unconverted_arguments`.

This is a minor refactor that prepares for later cleanups.
In `naga::front::wgsl::lower::Lowerer::call`, when building `Math`
expressions, rename `unconverted_arguments` to `lowered_arguments`.

No change to behavior. This is a minor refactor that prepares for
later cleanups.
In `naga::front::wgsl::lower::Lowerer::call`, when building `Math`
expressions, use `lowered_arguments` for both the original Naga IR
argument expressions and their counterparts after applying automatic
conversions.

This is a minor refactor that prepares for later cleanups.
In `naga::front::wgsl::lower::Lowerer`, add a new helper method,
`apply_automatic_conversions_for_call`, that applies the conversions
necessary to pass a given set of arguments to a given overload.

Use this helper in the `call` method when building `Math`
expressions.

This is just code motion, and should have no effect on behavior.
In `naga::front::wgsl::lower::Lowerer`, add a new helper method,
`resolve_overloads`, that selects the appropriate rule for a given
list of of arguments from an overload set.

Use this helper in the `call` method when building `Math`
expressions.

This is just code motion, and should have no effect on behavior.
Since we have moved some code out into separate functions, we have
less context to compete with, so we can use simpler names for the
arguments.

This commit is just renaming.
In `naga::front::wgsl`, add a new helper method,
`Lowerer::math_function_helper`, that constructs `Expression::Math`
expressions for calls to `MathFunction` builtins. Use this helper in
`Lowerer::call`.
@jimblandy jimblandy force-pushed the naga-wgsl-in-math-function-helper branch from 5a9bc00 to ad2e936 Compare April 2, 2025 22:01
@jimblandy
Copy link
Member Author

This has been rebased on the latest version of #6833.

@jimblandy jimblandy marked this pull request as ready for review April 2, 2025 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: naga front-end kind: refactor Making existing function faster or nicer lang: WGSL WebGPU Shading Language naga Shader Translator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant