-
Notifications
You must be signed in to change notification settings - Fork 1k
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
jimblandy
wants to merge
20
commits into
gfx-rs:trunk
Choose a base branch
from
jimblandy:naga-wgsl-in-math-function-helper
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Refactor WGSL front end's MathFunction
handling
#7442
jimblandy
wants to merge
20
commits into
gfx-rs:trunk
from
jimblandy:naga-wgsl-in-math-function-helper
+3,602
−1,040
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 tasks
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`.
5a9bc00
to
ad2e936
Compare
This has been rebased on the latest version of #6833. |
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This depends on #6833.
To prepare for further use of the
OverloadSet
trait, refactor the code innaga::front::wgsl::lower::Lowerer::call
that handles calls to builtin functions in theMathFunctions
enum. This results in three new methods onLowerer
: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;math_function_helper
, which uses the above two to build anExpression::Math
expression.