Skip to content

[Heap2Local] Fix cmpxchg with same ref and expected #9333

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.

Sign up for GitHub

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
tlively merged 1 commit into from
Nov 2, 2026
+87 −26
Merged

[Heap2Local] Fix cmpxchg with same ref and expected #9333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/passes/Heap2Local.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1131,6 +1131,18 @@ struct Struct2Local : PostWalker<Struct2Local> {
auto type = curr->type;
assert(type == field.type);
assert(!field.isPacked());
auto local = localIndexes[curr->index];

if (analyzer.getInteraction(curr->expected) ==
ParentChildInteraction::Flows) {
// Since the allocation does not escape, it cannot possibly match the
// value already in the struct. The cmpxchg will just do a read.
replaceCurrent(builder.makeBlock({builder.makeDrop(curr->ref),
builder.makeDrop(curr->expected),
builder.makeDrop(curr->replacement),
builder.makeLocalGet(local, type)}));
return;
}

// Hold everything in scratch locals, just like for other RMW ops and
// struct.new. Use a nullable (shared) eqref local for `expected` to
Expand All @@ -1143,7 +1155,6 @@ struct Struct2Local : PostWalker<Struct2Local> {
auto oldScratch = builder.addVar(func, type);
auto expectedScratch = builder.addVar(func, expectedType);
auto replacementScratch = builder.addVar(func, type);
auto local = localIndexes[curr->index];

auto* block = builder.makeBlock(
{builder.makeDrop(curr->ref),
Expand Down
100 changes: 75 additions & 25 deletions test/lit/passes/heap2local-rmw.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1442,16 +1442,16 @@
;; CHECK: (type $struct (struct (field (mut (ref null $struct)))))
(type $struct (struct (field (mut (ref null $struct)))))

;; CHECK: (type $1 (func))
;; CHECK: (type $1 (func (result (ref null $struct))))

;; CHECK: (type $2 (func (param (ref $struct)) (result (ref null $struct))))

;; CHECK: (export "test" (func $cmpxchg-ref-and-expected))

;; CHECK: (func $cmpxchg-ref-and-expected (type $1)
;; CHECK: (func $cmpxchg-ref-and-expected (type $1) (result (ref null $struct))
;; CHECK-NEXT: (local $local (ref $struct))
;; CHECK-NEXT: (local $1 (ref null $struct))
;; CHECK-NEXT: (local $2 (ref null $struct))
;; CHECK-NEXT: (local $3 eqref)
;; CHECK-NEXT: (local $4 (ref null $struct))
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result (ref null $struct))
;; CHECK-NEXT: (drop
Expand All @@ -1462,46 +1462,96 @@
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $3
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $4
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $2
;; CHECK-NEXT: (local.get $1)
;; CHECK-NEXT: )
;; CHECK-NEXT: (if
;; CHECK-NEXT: (ref.eq
;; CHECK-NEXT: (local.get $1)
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (local.set $2
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (then
;; CHECK-NEXT: (local.set $1
;; CHECK-NEXT: (local.get $4)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $1)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (block (result (ref null $struct))
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $1)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $cmpxchg-ref-and-expected (export "test") (result (ref null $struct))
(local $local (ref $struct))
(drop
;; The allocation flows to both `ref` and `expected`. Since it does not
;; escape, it cannot match the value in the field, so the cmpxchg must
;; just drop its operands and read the field local (rather than emitting a
;; struct.get on the nulled-out `ref` or comparing the field local against
;; the nulled-out `expected` via `ref.eq`). Because the replacement is
;; dropped, it does not escape either and can also be optimized.
(struct.atomic.rmw.cmpxchg $struct 0
(local.tee $local
(struct.new_default $struct)
)
(local.get $local)
(struct.new_default $struct)
)
)
(struct.get $struct 0
(local.get $local)
)
)

;; CHECK: (func $cmpxchg-ref-and-expected-non-null-replacement (type $2) (param $replacement (ref $struct)) (result (ref null $struct))
;; CHECK-NEXT: (local $local (ref $struct))
;; CHECK-NEXT: (local $2 (ref null $struct))
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result (ref null $struct))
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (local.set $2
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (local.get $replacement)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (block (result (ref null $struct))
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $cmpxchg-ref-and-expected (export "test")
(func $cmpxchg-ref-and-expected-non-null-replacement (param $replacement (ref $struct)) (result (ref null $struct))
(local $local (ref $struct))
(drop
;; The allocation flows to both `ref` and `expected` fields. We must
;; prioritize the optimization for flows through `ref`. Otherwise, if we
;; did the optimization for `expected` first, we would end up with a
;; struct.get of the ref value, but the ref value would have been changed
;; to a null and we would introduce a trap.
;; The replacement is non-null and cannot be optimized, so if the cmpxchg
;; spuriously succeeded (e.g. by comparing the null-initialized field
;; against an `expected` replaced with `null`), this would return
;; `$replacement` instead of `null`.
(struct.atomic.rmw.cmpxchg $struct 0
(local.tee $local
(struct.new_default $struct)
)
(local.get $local)
(ref.null none)
(local.get $replacement)
)
)
(struct.get $struct 0
(local.get $local)
)
)
)

Expand Down
Loading