[wasm][cleanup] Remove serialized signature from tag [v8/v8 : main]
Thibaud Michaud (Gerrit)
Thibaud Michaud voted and added 1 comment![]( "Open in Gerrit")
Votes added by Thibaud Michaud
| Commit-Queue | +1 |
1 comment
Related details
- Clemens Backes
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Clemens Backes (Gerrit)
Clemens Backes added 2 comments![]( "Open in Gerrit")
Nice! Thanks for following through with this cleanup!
One question though. Maybe I miss something.
// We need the FunctionSig. If the CanonicalSig contains indexed types, weReally? Why can't we use the canonical signature in `GetTypeForFunction`?
Related details
- Thibaud Michaud
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thibaud Michaud (Gerrit)
Thibaud Michaud added 1 comment![]( "Open in Gerrit")
// We need the FunctionSig. If the CanonicalSig contains indexed types, weReally? Why can't we use the canonical signature in `GetTypeForFunction`?
If the signature contains indexed types, the behavior would be different. The function would return something like `(ref null 5)` where 5 would be the canonical index instead of the module-relative index.
So I did this to preserve the old behavior, which makes sense to me. But I can't find the expected behavior in the spec, I think it has not been rebased on top of WasmGC yet...
Related details
- Clemens Backes
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Clemens Backes (Gerrit)
Clemens Backes voted and added 2 comments![]( "Open in Gerrit")
Votes added by Clemens Backes
| Code-Review | +1 |
2 comments
Thanks, LGTM!
// We need the FunctionSig. If the CanonicalSig contains indexed types, weThibaud MichaudReally? Why can't we use the canonical signature in `GetTypeForFunction`?
If the signature contains indexed types, the behavior would be different. The function would return something like `(ref null 5)` where 5 would be the canonical index instead of the module-relative index.
So I did this to preserve the old behavior, which makes sense to me. But I can't find the expected behavior in the spec, I think it has not been rebased on top of WasmGC yet...
Oh, I see, this is hidden in `ValueType::name()`.
Yeah, this is not really spec'ed and we discussed removing the implementation of type reflection already. So for now it shouldn't really matter what we return here, but it also doesn't hurt to be slow and iterate all types in the module.
Related details
- Thibaud Michaud
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thibaud Michaud (Gerrit)
Thibaud Michaud voted Commit-Queue+2![]( "Open in Gerrit")
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
V8 LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
[wasm][cleanup] Remove serialized signature from tag
We already store the index of the canonical signature, so use that
instead.
R=clem...@chromium.org
- M src/wasm/baseline/liftoff-compiler.cc
- M src/wasm/turboshaft-graph-interface.cc
- M src/wasm/wasm-js.cc
- M src/wasm/wasm-objects.cc
- M src/wasm/wasm-objects.h
- M src/wasm/wasm-objects.tq
Code-Review: +1 by Clemens Backes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |