[wasm, codegen] Add skipped write barrier verification to Liftoff [v8/v8 : main]
1 view
Skip to first unread message
Dominik Inführ (Gerrit)
Sep 2, 2025, 9:29:07 AM (5 days ago) Sep 2
to Matthias Liedtke, V8 LUCI CQ, AyeAye, Hannes Payer, dmercadi...@chromium.org, v8-ppc...@googlegroups.com, v8-risc...@chromium.org, v8-mip...@googlegroups.com, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Matthias Liedtke
Dominik Inführ added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention is currently required from:
- Matthias Liedtke
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominik Inführ (Gerrit)
Sep 2, 2025, 9:51:38 AM (5 days ago) Sep 2
to Darius Mercadier, Matthias Liedtke, V8 LUCI CQ, AyeAye, Hannes Payer, dmercadi...@chromium.org, v8-ppc...@googlegroups.com, v8-risc...@chromium.org, v8-mip...@googlegroups.com, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Darius Mercadier and Matthias Liedtke
Dominik Inführ added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Dominik Inführ . resolved
Adding Darius as reviewer as well, since we've discussed Turbofan support together.
Related details
Attention is currently required from:
- Darius Mercadier
- Matthias Liedtke
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Matthias Liedtke (Gerrit)
Sep 2, 2025, 10:13:23 AM (5 days ago) Sep 2
to Dominik Inführ, Clemens Backes, Darius Mercadier, V8 LUCI CQ, AyeAye, Hannes Payer, dmercadi...@chromium.org, v8-ppc...@googlegroups.com, v8-risc...@chromium.org, v8-mip...@googlegroups.com, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Clemens Backes, Darius Mercadier and Dominik Inführ
Matthias Liedtke added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 9:
Matthias Liedtke . resolved
@clem...@chromium.org: Could you please take this review to reduce my review workload?
Related details
Attention is currently required from:
- Clemens Backes
- Darius Mercadier
- Dominik Inführ
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Clemens Backes (Gerrit)
Sep 2, 2025, 10:33:36 AM (5 days ago) Sep 2
to Dominik Inführ, Matthias Liedtke, Darius Mercadier, V8 LUCI CQ, AyeAye, Hannes Payer, dmercadi...@chromium.org, v8-ppc...@googlegroups.com, v8-risc...@chromium.org, v8-mip...@googlegroups.com, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Darius Mercadier and Dominik Inführ
Clemens Backes added 1 comment![]( "Open in Gerrit")
File src/compiler/turboshaft/memory-optimization-reducer.h
Line 557, Patchset 10 (Latest):
WasmTrustedInstanceData::kLastYoungAllocationAddressOffset);Clemens Backes . unresolved
Is this additional field on the `WasmTrustedInstanceData` needed? We have the roots register, from which we can get to the isolate address (constant offset, `kIsolateRootBias`?), right? So in the end we would emit a single store relative to the root register.
(and we can probably do the same for the `stress_deopt_counter_address`, but that's orthogonal)
Related details
Attention is currently required from:
- Darius Mercadier
- Dominik Inführ
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominik Inführ (Gerrit)
Sep 2, 2025, 10:39:48 AM (5 days ago) Sep 2
to Clemens Backes, Matthias Liedtke, Darius Mercadier, V8 LUCI CQ, AyeAye, Hannes Payer, dmercadi...@chromium.org, v8-ppc...@googlegroups.com, v8-risc...@chromium.org, v8-mip...@googlegroups.com, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Clemens Backes and Darius Mercadier
Dominik Inführ voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dominik Inführ
| Commit-Queue | +1 |
1 comment
File src/compiler/turboshaft/memory-optimization-reducer.h
Line 557, Patchset 10 (Latest):
WasmTrustedInstanceData::kLastYoungAllocationAddressOffset);Clemens Backes . unresolved
Is this additional field on the `WasmTrustedInstanceData` needed? We have the roots register, from which we can get to the isolate address (constant offset, `kIsolateRootBias`?), right? So in the end we would emit a single store relative to the root register.
(and we can probably do the same for the `stress_deopt_counter_address`, but that's orthogonal)
Dominik Inführ
Hmm, good point. I have to check this. I simply copied the solution we use from NewSpaceAllocationTop. If that works, we should be able to use the same mechanism for this as well and maybe get some performance benefits from this as well.
Related details
Attention is currently required from:
- Clemens Backes
- Darius Mercadier
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominik Inführ (Gerrit)
Sep 2, 2025, 1:46:35 PM (5 days ago) Sep 2
to Clemens Backes, Matthias Liedtke, Darius Mercadier, V8 LUCI CQ, AyeAye, Hannes Payer, dmercadi...@chromium.org, v8-ppc...@googlegroups.com, v8-risc...@chromium.org, v8-mip...@googlegroups.com, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Clemens Backes and Darius Mercadier
Dominik Inführ added 2 comments![]( "Open in Gerrit")
Patchset-level comments
File src/compiler/turboshaft/memory-optimization-reducer.h
Line 557, Patchset 10:
WasmTrustedInstanceData::kLastYoungAllocationAddressOffset);Clemens Backes . resolved
Dominik InführIs this additional field on the `WasmTrustedInstanceData` needed? We have the roots register, from which we can get to the isolate address (constant offset, `kIsolateRootBias`?), right? So in the end we would emit a single store relative to the root register.
(and we can probably do the same for the `stress_deopt_counter_address`, but that's orthogonal)
Hmm, good point. I have to check this. I simply copied the solution we use from NewSpaceAllocationTop. If that works, we should be able to use the same mechanism for this as well and maybe get some performance benefits from this as well.
Dominik Inführ
This indeed seems to work, thanks! PTALA
Related details
Attention is currently required from:
- Clemens Backes
- Darius Mercadier
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Clemens Backes (Gerrit)
Sep 3, 2025, 4:53:01 AM (4 days ago) Sep 3
to Dominik Inführ, Matthias Liedtke, Darius Mercadier, V8 LUCI CQ, AyeAye, Hannes Payer, dmercadi...@chromium.org, v8-ppc...@googlegroups.com, v8-risc...@chromium.org, v8-mip...@googlegroups.com, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Darius Mercadier and Dominik Inführ
Clemens Backes voted and added 1 comment![]( "Open in Gerrit")
Votes added by Clemens Backes
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- Darius Mercadier
- Dominik Inführ
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Darius Mercadier (Gerrit)
Sep 3, 2025, 4:56:50 AM (4 days ago) Sep 3
to Dominik Inführ, Clemens Backes, Matthias Liedtke, V8 LUCI CQ, AyeAye, Hannes Payer, dmercadi...@chromium.org, v8-ppc...@googlegroups.com, v8-risc...@chromium.org, v8-mip...@googlegroups.com, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Dominik Inführ
Darius Mercadier voted and added 1 comment![]( "Open in Gerrit")
Votes added by Darius Mercadier
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- Dominik Inführ
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominik Inführ (Gerrit)
Sep 3, 2025, 7:10:49 AM (4 days ago) Sep 3
to Darius Mercadier, Clemens Backes, Matthias Liedtke, V8 LUCI CQ, AyeAye, Hannes Payer, dmercadi...@chromium.org, v8-ppc...@googlegroups.com, v8-risc...@chromium.org, v8-mip...@googlegroups.com, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Dominik Inführ added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 12 (Latest):
Dominik Inführ . resolved
Hey, in PS12 I've updated the CL to only use the root relative code path as it should work both if isolate() is null or non-null. It should also basically create the same generated code. PTALA to see whether that's okay for you.
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Clemens Backes (Gerrit)
Sep 3, 2025, 7:42:28 AM (4 days ago) Sep 3
to Dominik Inführ, Darius Mercadier, Matthias Liedtke, V8 LUCI CQ, AyeAye, Hannes Payer, dmercadi...@chromium.org, v8-ppc...@googlegroups.com, v8-risc...@chromium.org, v8-mip...@googlegroups.com, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Attention needed from Dominik Inführ
Clemens Backes voted and added 1 comment![]( "Open in Gerrit")
Votes added by Clemens Backes
| Code-Review | +1 |
1 comment
Patchset-level comments
Dominik Inführ . resolved
Hey, in PS12 I've updated the CL to only use the root relative code path as it should work both if isolate() is null or non-null. It should also basically create the same generated code. PTALA to see whether that's okay for you.
Clemens Backes
Ah, I was already wondering if we actually need the other code :)
LGTM.
Related details
Attention is currently required from:
- Dominik Inführ
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominik Inführ (Gerrit)
Sep 3, 2025, 8:31:20 AM (4 days ago) Sep 3
to Darius Mercadier, Clemens Backes, Matthias Liedtke, V8 LUCI CQ, AyeAye, Hannes Payer, dmercadi...@chromium.org, v8-ppc...@googlegroups.com, v8-risc...@chromium.org, v8-mip...@googlegroups.com, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
Dominik Inführ voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dominik Inführ
| Commit-Queue | +2 |
1 comment
Patchset-level comments
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
Sep 3, 2025, 9:01:25 AM (4 days ago) Sep 3
to Dominik Inführ, Darius Mercadier, Clemens Backes, Matthias Liedtke, AyeAye, Hannes Payer, dmercadi...@chromium.org, v8-ppc...@googlegroups.com, v8-risc...@chromium.org, v8-mip...@googlegroups.com, mlippau...@chromium.org, v8-re...@googlegroups.com, was...@google.com
V8 LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[wasm, codegen] Add skipped write barrier verification to Liftoff
This CL adds support to Liftoff for skipped write barrier
verification. When this verification mode is enabled, skipped write
barriers call a C function in order to check whether skipping the
barrier was valid.
In order for this check to work, allocations in generated code
need to set the last_young_allocation_ field either to the new
object in new space or reset it to 0. For Liftoff, allocations
are generated through Turbofan/Turboshaft. So this CL also
extends MemoryOptimizationReducer.
Bug: 437096305
Change-Id: I4265e1ba38f545c121b7575f98d28d3ea6af49dc
Cq-Include-Trybots: luci.v8.try:v8_mac_arm64_dbg
Cq-Include-Trybots: luci.v8.try:v8_linux_arm_dbg
Cq-Include-Trybots: luci.v8.try:v8_linux_dbg
Reviewed-by: Darius Mercadier <dmerc...@chromium.org>
Reviewed-by: Clemens Backes <clem...@chromium.org>
Commit-Queue: Dominik Inführ <dinf...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#102218}
Files:
- M src/codegen/arm/macro-assembler-arm.cc
- M src/codegen/arm/macro-assembler-arm.h
- M src/codegen/arm64/macro-assembler-arm64.cc
- M src/codegen/arm64/macro-assembler-arm64.h
- M src/codegen/code-stub-assembler.cc
- M src/codegen/external-reference.cc
- M src/codegen/external-reference.h
- M src/codegen/ia32/macro-assembler-ia32.cc
- M src/codegen/ia32/macro-assembler-ia32.h
- M src/codegen/x64/macro-assembler-x64.cc
- M src/codegen/x64/macro-assembler-x64.h
- M src/compiler/turboshaft/memory-optimization-reducer.h
- M src/heap/heap.cc
- M src/heap/heap.h
- M src/wasm/baseline/arm/liftoff-assembler-arm-inl.h
- M src/wasm/baseline/arm64/liftoff-assembler-arm64-inl.h
- M src/wasm/baseline/ia32/liftoff-assembler-ia32-inl.h
- M src/wasm/baseline/x64/liftoff-assembler-x64-inl.h
Change size: M
Delta: 18 files changed, 180 insertions(+), 4 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Clemens Backes, +1 by Darius Mercadier
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kasper Lund (Gerrit)
Sep 4, 2025, 1:01:17 AM (3 days ago) Sep 4
to Yahan Lu (LuYahan), V8 LUCI CQ, Ji Qiu, AyeAye, v8-re...@googlegroups.com, was...@google.com
Attention needed from Ji Qiu and Yahan Lu (LuYahan)
Kasper Lund voted and added 1 comment![]( "Open in Gerrit")
Votes added by Kasper Lund
| Code-Review | +1 |
1 comment
File src/codegen/riscv/macro-assembler-riscv.cc
Line 739, Patchset 1 (Latest):
mv(kCArgRegs[0], object);Kasper Lund . resolved
Are we sure these cannot overlap? Can we DCHECK?
Related details
Attention is currently required from:
- Ji Qiu
- Yahan Lu (LuYahan)
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kasper Lund (Gerrit)
Sep 4, 2025, 1:01:35 AM (3 days ago) Sep 4
to Yahan Lu (LuYahan), V8 LUCI CQ, Ji Qiu, AyeAye, v8-re...@googlegroups.com, was...@google.com
Attention needed from Ji Qiu and Yahan Lu (LuYahan)
Kasper Lund added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention is currently required from:
- Ji Qiu
- Yahan Lu (LuYahan)
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Florian Loitsch (Gerrit)
Sep 4, 2025, 2:51:03 AM (3 days ago) Sep 4
to Yahan Lu (LuYahan), Kasper Lund, V8 LUCI CQ, Ji Qiu, AyeAye, v8-re...@googlegroups.com, was...@google.com
Attention needed from Ji Qiu and Yahan Lu (LuYahan)
Florian Loitsch voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Ji Qiu
- Yahan Lu (LuYahan)
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ji Qiu (Gerrit)
Sep 4, 2025, 4:38:36 AM (3 days ago) Sep 4
to Yahan Lu (LuYahan), Florian Loitsch, Kasper Lund, V8 LUCI CQ, AyeAye, v8-re...@googlegroups.com, was...@google.com
Attention needed from Yahan Lu (LuYahan)
Ji Qiu voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Yahan Lu (LuYahan)
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yahan Lu (LuYahan) (Gerrit)
Sep 4, 2025, 8:50:20 PM (3 days ago) Sep 4
to Ji Qiu, Florian Loitsch, Kasper Lund, V8 LUCI CQ, AyeAye, v8-re...@googlegroups.com, was...@google.com
Yahan Lu (LuYahan) voted![]( "Open in Gerrit")
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
Sep 4, 2025, 8:52:29 PM (3 days ago) Sep 4
to Yahan Lu (LuYahan), Ji Qiu, Florian Loitsch, Kasper Lund, AyeAye, v8-re...@googlegroups.com, was...@google.com
V8 LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[riscv][wasm, codegen] Add skipped write barrier verification to Liftoff
Port commit bc103f35c846906fe57914a9846c12b955ff9e46
Change-Id: Ib9130ddd5e88688a0bbdd1ee96bd8e3da127dc82
Reviewed-by: Kasper Lund <kas...@rivosinc.com>
Reviewed-by: Ji Qiu <qi...@iscas.ac.cn>
Commit-Queue: Yahan Lu (LuYahan) <ya...@iscas.ac.cn>
Reviewed-by: Florian Loitsch <floi...@rivosinc.com>
Auto-Submit: Yahan Lu (LuYahan) <ya...@iscas.ac.cn>
Cr-Commit-Position: refs/heads/main@{#102255}
Files:
- M src/codegen/riscv/macro-assembler-riscv.cc
- M src/codegen/riscv/macro-assembler-riscv.h
- M src/wasm/baseline/riscv/liftoff-assembler-riscv32-inl.h
- M src/wasm/baseline/riscv/liftoff-assembler-riscv64-inl.h
Change size: S
Delta: 4 files changed, 40 insertions(+), 2 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Kasper Lund, +1 by Florian Loitsch, +1 by Ji Qiu
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages