Skip to content

Builtin function database, for automatic conversions #6833

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

Merged
merged 10 commits into from
Apr 2, 2025

Conversation

jimblandy
Copy link
Member

@jimblandy jimblandy commented Dec 27, 2024

Depends on #7406.
Rebase without squashing.

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.

This addresses #6443 for MathFunction, although that issue covers other categories of operations as well.

Remaining work:

  • Hook up concrete_only.
  • Properly check that sufficient arguments have been passed.

@jimblandy

This comment was marked as outdated.

@jimblandy jimblandy force-pushed the naga-function-database branch from 4b0303d to 0218277 Compare March 15, 2025 21:03
@jimblandy jimblandy force-pushed the naga-function-database branch 12 times, most recently from 55026f3 to d5f7caf Compare March 23, 2025 00:09
@jimblandy

This comment was marked as resolved.

@jimblandy jimblandy force-pushed the naga-function-database branch from d5f7caf to e9c2aaa Compare March 23, 2025 00:18
@jimblandy jimblandy marked this pull request as ready for review March 23, 2025 00:18
@jimblandy jimblandy force-pushed the naga-function-database branch 5 times, most recently from 5753066 to e7f8127 Compare April 1, 2025 17:19
@jimblandy
Copy link
Member Author

@ErichDonGubler @andyleiserson Okay, I believe I've addressed all comments with the current text of this PR. I've left some comments unresolved where I pushed back on the suggestion; please mark them resolved if you're okay with that.

@jimblandy jimblandy force-pushed the naga-function-database branch from c887b92 to 98899c8 Compare April 2, 2025 02:13
@jimblandy
Copy link
Member Author

jimblandy commented Apr 2, 2025

Maybe it would be worth adding some tests from the bugs we're expecting this to fix? I think pow, min/max, and clamp with a mix of abstract and concrete arguments are common offenders.

I'd overlooked this comment earlier. I've added these tests now.

@jimblandy jimblandy force-pushed the naga-function-database branch from 131184f to 6b55a0b Compare April 2, 2025 19:11
Copy link
Member

@ErichDonGubler ErichDonGubler left a comment

Choose a reason for hiding this comment

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

AFTER MANY EXHAUSTING HOURS OF EPIC WORK BY THE JIM

Here's yer green checkmark. ✨✅😀👍🏻✨

@ErichDonGubler
Copy link
Member

@andyleiserson, I presume your feedback is satisfied. If so, could you please indicate, and optionally rebase-and-merge?

@jimblandy jimblandy force-pushed the naga-function-database branch from 6b55a0b to fce1ef4 Compare April 2, 2025 21:11
@jimblandy
Copy link
Member Author

I've squashed all the changes that should have been squashed, so this can be landed with a rebase.

If you want to see the last few revisions as separate commits, they're at https://github.com/jimblandy/wgpu/commits/naga-function-database-final-revisions/

jimblandy added 10 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.
@jimblandy jimblandy force-pushed the naga-function-database branch from fce1ef4 to 93aa210 Compare April 2, 2025 21:15
Copy link
Contributor

@andyleiserson andyleiserson left a comment

Choose a reason for hiding this comment

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

LGTM as well, excited to see this land!

@cwfitzgerald cwfitzgerald merged commit 7da6996 into gfx-rs:trunk Apr 2, 2025
65 of 66 checks passed
@jimblandy jimblandy deleted the naga-function-database branch April 3, 2025 14:04
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.

4 participants