Skip to content

[Custom Descriptors] Heap2Local: Handle definitely-failing descriptor casts #7919

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
merged 3 commits into from
Sep 24, 2025
Merged

[Custom Descriptors] Heap2Local: Handle definitely-failing descriptor casts #7919

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
86d4f97
go?
kripken Sep 23, 2025
40f64c1
work
kripken Sep 24, 2025
f2e5a48
comment
kripken Sep 24, 2025
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
25 changes: 16 additions & 9 deletions src/passes/Heap2Local.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -892,15 +892,22 @@ struct Struct2Local : PostWalker<Struct2Local> {
assert(allocIsCastRef);
// The cast succeeds iff the optimized allocation's descriptor is the
// same as the given descriptor and traps otherwise.
auto type = allocation->desc->type;
replaceCurrent(builder.blockify(
builder.makeDrop(curr->ref),
builder.makeIf(
builder.makeRefEq(
curr->desc,
builder.makeLocalGet(localIndexes[fields.size()], type)),
builder.makeRefNull(allocation->type.getHeapType()),
builder.makeUnreachable())));
if (curr->type.isNonNullable()) {
// No null is possible; trap.
replaceCurrent(builder.blockify(builder.makeDrop(curr->ref),
builder.makeDrop(curr->desc),
builder.makeUnreachable()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we should be able to optimize out an allocation even if it flows into a successful non-null descriptor cast. This code is not correct because it replaces such a successful cast with an unreachable, though. If we did the correct optimization, the cast (which returns a non-null reference) would be replaced with a propagation of the null value. Since this is not a simple type refinement, we would have to guarantee that whatever location receives the null value will also be updated to accept it.

} else {
auto type = allocation->desc->type;
replaceCurrent(builder.blockify(
builder.makeDrop(curr->ref),
builder.makeIf(
builder.makeRefEq(
curr->desc,
builder.makeLocalGet(localIndexes[fields.size()], type)),
builder.makeRefNull(allocation->type.getHeapType()),
builder.makeUnreachable())));
}
}
} else {
// We know this RefCast receives our allocation, so we can see whether it
Expand Down
107 changes: 71 additions & 36 deletions test/lit/passes/heap2local-desc.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@
;; CHECK-NEXT: (struct.new_default $super.desc)
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (block
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (local.set $2
Expand All @@ -261,18 +261,10 @@
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (if (result nullref)
;; CHECK-NEXT: (ref.eq
;; CHECK-NEXT: (local.get $desc)
;; CHECK-NEXT: (local.get $1)
;; CHECK-NEXT: )
;; CHECK-NEXT: (then
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (else
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (local.get $desc)
;; CHECK-NEXT: )
;; CHECK-NEXT: (unreachable)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems bad that this successful cast is being optimized to unreachable!

;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
Expand All @@ -295,7 +287,7 @@
;; CHECK-NEXT: (local $1 (ref (exact $super.desc)))
;; CHECK-NEXT: (local $2 (ref (exact $super.desc)))
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (block
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (local.set $2
Expand All @@ -307,18 +299,10 @@
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (if (result nullref)
;; CHECK-NEXT: (ref.eq
;; CHECK-NEXT: (local.get $desc)
;; CHECK-NEXT: (local.get $1)
;; CHECK-NEXT: )
;; CHECK-NEXT: (then
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (else
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (local.get $desc)
;; CHECK-NEXT: )
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
Expand All @@ -337,7 +321,7 @@
;; CHECK-NEXT: (local $1 (ref null (exact $super.desc)))
;; CHECK-NEXT: (local $2 (ref null (exact $super.desc)))
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (block
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (local.set $2
Expand All @@ -351,20 +335,12 @@
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (if (result nullref)
;; CHECK-NEXT: (ref.eq
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $1)
;; CHECK-NEXT: )
;; CHECK-NEXT: (then
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (else
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
Expand Down Expand Up @@ -898,3 +874,62 @@
)
)

;; A descriptor chain, and the cast is non-nullable. All the allocations can be
;; moved to locals, and must be handled properly (trapping, in this case;
;; returning a null would lead to a validation error).
(module
(rec
;; CHECK: (rec
;; CHECK-NEXT: (type $A (descriptor $B (struct)))
(type $A (descriptor $B (struct)))
;; CHECK: (type $B (describes $A (descriptor $C (struct))))
(type $B (describes $A (descriptor $C (struct))))
;; CHECK: (type $C (sub (describes $B (struct))))
(type $C (sub (describes $B (struct))))
)

;; CHECK: (type $3 (func (result (ref (exact $A)))))

;; CHECK: (func $A (type $3) (result (ref (exact $A)))
;; CHECK-NEXT: (local $0 nullref)
;; CHECK-NEXT: (local $1 nullref)
;; CHECK-NEXT: (local $2 (ref (exact $C)))
;; CHECK-NEXT: (local $3 (ref (exact $C)))
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (local.set $1
;; CHECK-NEXT: (ref.as_non_null
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $0
;; CHECK-NEXT: (local.get $1)
;; CHECK-NEXT: )
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (local.set $3
;; CHECK-NEXT: (struct.new_default $C)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $2
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
(func $A (result (ref (exact $A)))
(ref.cast_desc (ref (exact $A))
(struct.new_default $B
(ref.null none)
)
(struct.new_default $B
(struct.new_default $C)
)
)
)
)

Loading