Refactor WGSL front end's MathFunction handling
#7442
Merged
+244
−199
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
jimblandy
added
naga
jimblandy
force-pushed
the
naga-wgsl-in-math-function-helper
branch
from
5a9bc00 to
ad2e936
Compare
|
This has been rebased on the latest version of #6833. |
jimblandy
force-pushed
the
naga-wgsl-in-math-function-helper
branch
from
ad2e936 to
83430f4
Compare
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
force-pushed
the
naga-wgsl-in-math-function-helper
branch
from
83430f4 to
87d79c6
Compare
ErichDonGubler
approved these changes
Apr 7, 2025
|
Commits made this so easy to review. 😊 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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. Rebase (but squash is fine too).
This PR is a nothingburger: just code motion, renames, and minor refactorings. Commits are split for easy review.
To prepare for further use of the
OverloadSettrait, refactor the code innaga::front::wgsl::lower::Lowerer::callthat handles calls to builtin functions in theMathFunctionsenum. 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::Mathexpression.