-
Couldn't load subscription status.
- Fork 1.8k
[GR-48908] Implement the Typed Function References proposal in GraalWasm. #12423
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
Conversation
This file contains hidden or 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
Also fixed up code style in typed function references implementation.
Make the type checks around typed functions, global and table writes more precise.
This makes sure we always dispatch on a compilation final constant.
The funcref type only admits wasm functions. External functions must go through the import object, where they are wrapped in ExecuteHostFunctionNode.
According to the wasm JS API spec, exnref is not a valid ValueType. However, we will need to enforce that the types v128 and exnref never cross the wasm and JS boundary and we will need to do so in JS (we allow v128 values crossing from wasm to the spec test harness and other embedders). Since we use ValueType as a simple form of type reflection, we will need to have exnref so that GraalJS can detect exnref in wasm type signatures.
…_indirect This reverts back to using type equivalence classes. We use them for fast type equality checks. In the optimistic case (expected type == actual type), this is enough and leads to a fast path that avoids invoking the subtype matching on every indirect call. If types are not equal, subtype matching is still performed.
These string values show up in root node names and ease debugging.
a59ac51 to
ea73176
Compare
This is called for by the official spec test "linking". We keep the check only for modules eval'ed with EvalReturnsInstance, where it is possible for the embedder to obtain a WasmInstance that will fail linking. Without EvalReturnsInstance, instances must pass linking before they are returned to the embedder.
These actions have side effects and their ordering can matter insofar as one link action can fail and only the effects of those that came before should be observable.
ea73176 to
538847e
Compare
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 PR adds an implementation of the typed function references proposal (https://github.com/WebAssembly/function-references) behind the
--wasm.TypedFunctionReferencesoption.The biggest change is to GraalWasm's type system. The type system now, instead of being a closed enumeration of a few value types, is now open-ended and includes user-defined composite types (function types for now). In the engine, we still use scalars to represent types, but we switch from
bytes toints. Negative values represent predefined types (numerics, vectors and predefined abstract heap types), while non-negative values are type indices which point to a symbol table that holds the type's definition (see theWasmTypejavadoc for more details).Aside from changing the data representation of value types, the way we compare them also changes. Since GraalWasm now has subtyping (typed function references are a subtype of
funcref), it is not possible to compare types simply by equality but a subtype matching check must be made. At runtime, this happens in two places: when unchecked values come in from interop/JS and when calling a function usingcall_indirect. The subtype check is implemented by recursively traversing and comparing the structure of the types and the recursion does not sit well with PE, so I put it behind a@TruffleBoundary. In current benchmarks, we should not be hitting this, since we only invoke subtype matching when the types are not equal. I originally implemented the subtype check by constructingfinaltree-like data structures for the composite value types and then calling a virtualmatchesmethod (basically a mini AST interpreter) and I was hoping this would PE nicely, but I still ended up with non-inlined invokes in the compiled code so I dropped this. The code for that is still in the commit history.Speaking of the commit history, I recommend reviewing the entire changeset. The commit history includes 3 reversions:
call_indirectfor cases where the types are equal.exnreffromValueType, since the Wasm JS API spec does not listexnrefas one of the possible values ofValueType. However, we will need to forbid JS from passing and receivingexnrefvalues to/from wasm (that's part of the spec). For that, we will need JS to know whether something has anexnrefvalue type and so that we are not forced to introduce another form of type reflection, I putexnrefback intoValueType.I originally implemented subtype matching by constructing tree representations of closed value types (with all type indices resolved). To avoid having to allocate and maintain more objects, I changed this back to using scalars for types.This is now restored.As part of the PR, I also updated the revision of the official spec tests that we run and I fixed some minor unrelated issues. Since the
mainbranch of thespecrepo has now caught up withwasm-3.0, we now run only on a single revision of the spec tests.