[wasm][shared] Handle shared Wasm objects in GotoIfCannotBeHeldWeakly [v8/v8 : main]
0 views
Skip to first unread message
Manos Koukoutos (Gerrit)
Mar 10, 2026, 11:46:42 AMMar 10
to Matthias Liedtke, V8 LUCI CQ, v8-re...@googlegroups.com
Attention needed from Matthias Liedtke
Manos Koukoutos voted and added 3 comments![]( "Open in Gerrit")
Votes added by Manos Koukoutos
| Commit-Queue | +1 |
3 comments
Commit Message
Line 7, Patchset 1:
[wasm][shared] Shared Wasm objects in GotoIfCannotBeHeldWeaklyMatthias Liedtke . resolved
Manos KoukoutosWhat's with them? This message misses something imo. 😊
Maybe
```suggestion
[wasm][shared] Add handling of shared Wasm objects in GotoIfCannotBeHeldWeakly
```
I was trying to fit it in one line, but I guess we can fit another word.
File src/builtins/builtins-collections-gen.cc
Line 495, Patchset 1:
GotoIf(InstanceTypeEqual(instance_type, WASM_ARRAY_TYPE),
&check_not_in_shared_space);
GotoIf(InstanceTypeEqual(instance_type, WASM_STRUCT_TYPE),
&check_not_in_shared_space);Matthias Liedtke . resolved
Manos KoukoutosFor performance reasons, maybe it would be faster to use `FIRST_WASM_OBJECT_TYPE` and `LAST_WASM_OBJECT_TYPE`, similarly to what we are doing in `IsDataRefMap` in `wasm-lowering-reducer.h`, so we only have a single comparison?
Done
Line 504, Patchset 1:
TNode<IntPtrT> page_flags = LoadMemoryChunkFlags(CAST(obj));
GotoIf(WordNotEqual(
WordAnd(page_flags,
IntPtrConstant(MemoryChunk::IN_WRITABLE_SHARED_SPACE)),
IntPtrConstant(0)),Matthias Liedtke . resolved
Manos KoukoutosThis part already appears a few tiems in `code-stub-assembler.cc`. Can we add a utility function on the assembler for this and reuse it here?
Done
Related details
Attention is currently required from:
- Matthias Liedtke
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Matthias Liedtke (Gerrit)
Mar 10, 2026, 11:49:06 AMMar 10
to Manos Koukoutos, V8 LUCI CQ, v8-re...@googlegroups.com
Attention needed from Manos Koukoutos
Matthias Liedtke voted and added 1 comment![]( "Open in Gerrit")
Votes added by Matthias Liedtke
| Code-Review | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Manos Koukoutos
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Manos Koukoutos (Gerrit)
Mar 10, 2026, 12:16:34 PMMar 10
to Matthias Liedtke, V8 LUCI CQ, v8-re...@googlegroups.com
Manos Koukoutos voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Manos Koukoutos (Gerrit)
Mar 11, 2026, 1:36:34 AMMar 11
to Jakob Kummerow, Matthias Liedtke, V8 LUCI CQ, v8-re...@googlegroups.com
Attention needed from Jakob Kummerow
Manos Koukoutos added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Manos Koukoutos . resolved
Jakob, please approve code-stub-assembler.cc.
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. |
Manos Koukoutos (Gerrit)
Mar 11, 2026, 2:09:07 AMMar 11
to Jakob Kummerow, Matthias Liedtke, V8 LUCI CQ, v8-re...@googlegroups.com
Attention needed from Jakob Kummerow and Matthias Liedtke
Manos Koukoutos added 1 comment![]( "Open in Gerrit")
Attention is currently required from:
- Jakob Kummerow
- Matthias Liedtke
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Matthias Liedtke (Gerrit)
Mar 11, 2026, 5:30:12 AMMar 11
to Manos Koukoutos, Jakob Kummerow, V8 LUCI CQ, v8-re...@googlegroups.com
Attention needed from Jakob Kummerow and Manos Koukoutos
Matthias Liedtke voted and added 1 comment![]( "Open in Gerrit")
Votes added by Matthias Liedtke
| Code-Review | +1 |
1 comment
Patchset-level comments
Matthias Liedtke . resolved
LGTM
Attention is currently required from:
- Jakob Kummerow
- Manos Koukoutos
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)
Mar 11, 2026, 8:48:47 AMMar 11
to Manos Koukoutos, Jakob Kummerow, Matthias Liedtke, V8 LUCI CQ, v8-re...@googlegroups.com
Attention needed from Manos Koukoutos
Jakob Kummerow voted and added 2 comments![]( "Open in Gerrit")
Votes added by Jakob Kummerow
| Code-Review | +1 |
2 comments
Patchset-level comments
Jakob Kummerow . resolved
LGTM with optional nit.
File src/builtins/builtins-collections-gen.cc
Line 501, Patchset 4 (Latest):
GotoIf(IsPageFlagSet(BitcastTaggedToWord(obj),Jakob Kummerow . unresolved
FYI, a `GotoIf/Goto` sequence could also be expressed slightly more concisely with `Branch`:
```
Branch(IsPageFlagSet(...), if_cannot_be_held_weakly, &end);
```
Related details
Attention is currently required from:
- Manos Koukoutos
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. |
Manos Koukoutos (Gerrit)
Mar 11, 2026, 9:20:24 AMMar 11
to Jakob Kummerow, Matthias Liedtke, V8 LUCI CQ, v8-re...@googlegroups.com
Attention needed from Jakob Kummerow and Matthias Liedtke
Manos Koukoutos added 2 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 5 (Latest):
Manos Koukoutos . resolved
Jakob, can you please refresh your vote?
File src/builtins/builtins-collections-gen.cc
FYI, a `GotoIf/Goto` sequence could also be expressed slightly more concisely with `Branch`:
```
Branch(IsPageFlagSet(...), if_cannot_be_held_weakly, &end);
```
Attention is currently required from:
- Jakob Kummerow
- Matthias Liedtke
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)
Mar 12, 2026, 4:23:35 AMMar 12
to Manos Koukoutos, Jakob Kummerow, Matthias Liedtke, V8 LUCI CQ, v8-re...@googlegroups.com
Attention needed from Manos Koukoutos and Matthias Liedtke
Jakob Kummerow voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Manos Koukoutos
- Matthias Liedtke
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Manos Koukoutos (Gerrit)
Mar 12, 2026, 4:24:41 AMMar 12
to Jakob Kummerow, Matthias Liedtke, V8 LUCI CQ, v8-re...@googlegroups.com
Attention needed from Matthias Liedtke
Manos Koukoutos voted Commit-Queue+2![]( "Open in Gerrit")
Attention is currently required from:
- Matthias Liedtke
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)
Mar 12, 2026, 5:47:43 AMMar 12
to Manos Koukoutos, Jakob Kummerow, Matthias Liedtke, v8-re...@googlegroups.com
V8 LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[wasm][shared] Handle shared Wasm objects in GotoIfCannotBeHeldWeakly
Bug: 490877757, 42204563
Change-Id: Id516b6d5de3eed9c2ac5e4168018d4799b9ce77b
Reviewed-by: Jakob Kummerow <jkum...@chromium.org>
Commit-Queue: Manos Koukoutos <mano...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#105748}
Files:
- M src/builtins/builtins-collections-gen.cc
- M src/codegen/code-stub-assembler.cc
- M src/codegen/code-stub-assembler.h
- A test/mjsunit/regress/wasm/regress-490877757.js
Change size: M
Delta: 4 files changed, 66 insertions(+), 14 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Jakob Kummerow
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages