[wasmfx] Support arguments in stack wrapper [v8/v8 : main]
0 views
Skip to first unread message
Thibaud Michaud (Gerrit)
Dec 8, 2025, 4:10:23 AM12/8/25
to Clemens Backes, V8 LUCI CQ, dmercadi...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Clemens Backes
Thibaud Michaud added 2 comments![]( "Open in Gerrit")
Patchset-level comments
File src/wasm/module-compiler.cc
Line 2275, Patchset 2 (Parent):
#if V8_ENABLE_TURBOFANThibaud Michaud . resolved
Drive-by cleanup: this should have been removed in the previous CL which introduces the stack wrapper cache:
https://chromium-review.googlesource.com/c/v8/v8/+/7210774
Related details
Attention is currently required from:
- Clemens Backes
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Clemens Backes (Gerrit)
Dec 8, 2025, 6:27:33 AM12/8/25
to Thibaud Michaud, V8 LUCI CQ, dmercadi...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Thibaud Michaud
Clemens Backes voted and added 2 comments![]( "Open in Gerrit")
Votes added by Clemens Backes
| Code-Review | +1 |
2 comments
Patchset-level comments
Clemens Backes . resolved
Nice, not even that complex!
One nit / minor question, otherwise LGTM.
File src/compiler/wasm-compiler.cc
Line 1125, Patchset 2 (Latest):
PrintSignature(base::VectorOf(func_name, kMaxNameLen) + name_prefix_len, sig,Clemens Backes . unresolved
Why this change? Isn't this identical to the previous `base::ArrayVector` (which is also still used one line above)?
Related details
Attention is currently required from:
- Thibaud Michaud
Submit Requirements:
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)
Dec 8, 2025, 6:46:52 AM12/8/25
to Jakob Kummerow, Clemens Backes, V8 LUCI CQ, dmercadi...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Jakob Kummerow
Thibaud Michaud voted and added 1 comment![]( "Open in Gerrit")
Votes added by Thibaud Michaud
| Commit-Queue | +2 |
1 comment
File src/compiler/wasm-compiler.cc
Line 1125, Patchset 2:
PrintSignature(base::VectorOf(func_name, kMaxNameLen) + name_prefix_len, sig,Clemens Backes . resolved
Why this change? Isn't this identical to the previous `base::ArrayVector` (which is also still used one line above)?
Thibaud Michaud
That's not needed actually, you're right, I reverted it.
I think I copied this from the import wrapper compilation above.
Related details
Attention is currently required from:
- Jakob Kummerow
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
Dec 8, 2025, 7:31:10 AM12/8/25
to Thibaud Michaud, Jakob Kummerow, Clemens Backes, dmercadi...@chromium.org, v8-re...@googlegroups.com, was...@google.com
V8 LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
2 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/compiler/wasm-compiler.cc
Insertions: 1, Deletions: 2.
@@ -1122,8 +1122,7 @@
char func_name[kMaxNameLen];
int name_prefix_len =
SNPrintF(base::ArrayVector(func_name), "wasm-continuation-");
- PrintSignature(base::VectorOf(func_name, kMaxNameLen) + name_prefix_len, sig,
- '-');
+ PrintSignature(base::ArrayVector(func_name) + name_prefix_len, sig, '-');
wasm::WasmCompilationResult result =
Pipeline::GenerateCodeForWasmNativeStubFromTurboshaft(
sig, wasm::WrapperCompilationInfo{CodeKind::WASM_STACK_ENTRY},
```
Change information
Commit message:
[wasmfx] Support arguments in stack wrapper
Now that stack wrappers are cached per signature, allow them to have
arguments.
The wrapper receives a pointer to the arg buffer, unpacks it and
forwards the arguments to the initial func ref.
R=jkum...@chromium.org
Bug: 388533754
Change-Id: I8f8b90360902a2eabd73e861fff5b9b3122675cb
Commit-Queue: Thibaud Michaud <thib...@chromium.org>
Reviewed-by: Clemens Backes <clem...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#104167}
Files:
- M src/compiler/wasm-compiler-definitions.cc
- M src/compiler/wasm-compiler.cc
- M src/compiler/wasm-compiler.h
- M src/wasm/module-compiler.cc
- M src/wasm/wasm-code-manager.cc
- M src/wasm/wasm-code-manager.h
- M src/wasm/wasm-stack-wrapper-cache.cc
- M src/wasm/wrappers-inl.h
- M test/mjsunit/wasm/stack-switching-params.js
Change size: M
Delta: 9 files changed, 111 insertions(+), 106 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Clemens Backes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Manos Koukoutos (Gerrit)
Dec 8, 2025, 8:57:13 AM12/8/25
to Thibaud Michaud, V8 LUCI CQ, Jakob Kummerow, Clemens Backes, dmercadi...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Manos Koukoutos has created a revert of this change![]( "Open in Gerrit")
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jakob Kummerow (Gerrit)
Dec 9, 2025, 9:04:41 AM12/9/25
to Thibaud Michaud, V8 LUCI CQ, Jakob Kummerow, Clemens Backes, dmercadi...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Thibaud Michaud
Jakob Kummerow added 1 comment![]( "Open in Gerrit")
File src/wasm/wrappers-inl.h
Line 618, Patchset 4 (Latest):
for (size_t i = 0; i < sig_->parameter_count(); ++i) {Jakob Kummerow . unresolved
DBC: please find a way to reuse `IterateWasmFXArgBuffer` (probably by making that a free function declared in `turboshaft-graph-interface.h`).
Related details
Attention is currently required from:
- Thibaud Michaud
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages