Skip to content
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

Jump to bottom

Heap2Local: Fix an ordering issue with children having different interactions with a parent #6089

Merged
merged 2 commits into from
Nov 9, 2023
Merged

Heap2Local: Fix an ordering issue with children having different interactions with a parent #6089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0c33e44
fix
kripken Nov 8, 2023
ade831a
tpo
kripken Nov 8, 2023
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
54 changes: 33 additions & 21 deletions src/passes/Heap2Local.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,38 +541,50 @@ struct Heap2LocalOptimizer {
auto* child = flow.first;
auto* parent = flow.second;

auto interaction = getParentChildInteraction(allocation, parent, child);
if (interaction == ParentChildInteraction::Escapes ||
interaction == ParentChildInteraction::Mixes) {
// If the parent may let us escape, or the parent mixes other values
// up with us, give up.
return;
}

// The parent either fully consumes us, or flows us onwards; either way,
// we can proceed here, hopefully.
assert(interaction == ParentChildInteraction::FullyConsumes ||
interaction == ParentChildInteraction::Flows);

// If we've already seen an expression, stop since we cannot optimize
// things that overlap in any way (see the notes on exclusivity, above).
// Note that we use a nonrepeating queue here, so we already do not visit
// the same thing more than once; what this check does is verify we don't
// look at something that another allocation reached, which would be in a
// different call to this function and use a different queue (any overlap
// between calls would prove non-exclusivity).
//
// Note that we do this after the check for Escapes/Mixes above: it is
// possible for a parent to receive two children and handle them
// differently:
//
// (struct.set
// (local.get $ref)
// (local.get $value)
// )
//
// The value escapes, but the ref does not, and might be optimized. If we
// added the parent to |seen| for both children, the reference would get
// blocked from being optimized.
if (!seen.emplace(parent).second) {
return;
}

switch (getParentChildInteraction(allocation, parent, child)) {
case ParentChildInteraction::Escapes: {
// If the parent may let us escape then we are done.
return;
}
case ParentChildInteraction::FullyConsumes: {
// If the parent consumes us without letting us escape then all is
// well (and there is nothing flowing from the parent to check).
break;
}
case ParentChildInteraction::Flows: {
// The value flows through the parent; we need to look further at the
// grandparent.
flows.push({parent, parents.getParent(parent)});
break;
}
case ParentChildInteraction::Mixes: {
// Our allocation is not used exclusively via the parent, as other
// values are mixed with it. Give up.
return;
}
// We can proceed, as the parent interacts with us properly, and we are
// the only allocation to get here.

if (interaction == ParentChildInteraction::Flows) {
// The value flows through the parent; we need to look further at the
// grandparent.
flows.push({parent, parents.getParent(parent)});
}

if (auto* set = parent->dynCast<LocalSet>()) {
Expand Down
116 changes: 116 additions & 0 deletions test/lit/passes/heap2local.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2078,3 +2078,119 @@
)
)
)

(module
;; CHECK: (type $struct (struct (field (mut anyref))))
(type $struct (struct (field (mut anyref))))

;; CHECK: (func $multiple-interactions (type $1)
;; CHECK-NEXT: (local $temp (ref $struct))
;; CHECK-NEXT: (local $1 anyref)
;; CHECK-NEXT: (local.set $temp
;; CHECK-NEXT: (struct.new_default $struct)
;; CHECK-NEXT: )
;; CHECK-NEXT: (block
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (local.set $1
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $1
;; CHECK-NEXT: (local.get $temp)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $multiple-interactions
(local $temp (ref $struct))
(local.set $temp
(struct.new_default $struct)
)
;; This expression interacts with its children in two different ways: the
;; reference does not escape from the function, so we can optimize it into
;; locals, while the value read from the local is written to the heap, so it
;; does escape. However, we can optimize it after we optimize the first
;; allocation away, which would happen if we ran another pass of heap2local
;; (but we do not here).
(struct.set $struct 0
(struct.new_default $struct)
(local.get $temp)
)
)

;; CHECK: (func $multiple-interactions-both-locals (type $1)
;; CHECK-NEXT: (local $temp (ref $struct))
;; CHECK-NEXT: (local $temp2 (ref $struct))
;; CHECK-NEXT: (local $2 anyref)
;; CHECK-NEXT: (local.set $temp
;; CHECK-NEXT: (struct.new_default $struct)
;; CHECK-NEXT: )
;; 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: (block
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $2
;; CHECK-NEXT: (local.get $temp)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $multiple-interactions-both-locals
(local $temp (ref $struct))
(local $temp2 (ref $struct))
(local.set $temp
(struct.new_default $struct)
)
;; Now both allocations are written to locals. We can still optimize the
;; second.
(local.set $temp2
(struct.new_default $struct)
)
(struct.set $struct 0
(local.get $temp2)
(local.get $temp)
)
)

;; CHECK: (func $multiple-interactions-escapes (type $2) (result anyref)
;; CHECK-NEXT: (local $temp (ref $struct))
;; CHECK-NEXT: (local $temp2 (ref $struct))
;; CHECK-NEXT: (local.set $temp
;; CHECK-NEXT: (struct.new_default $struct)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $temp2
;; CHECK-NEXT: (struct.new_default $struct)
;; CHECK-NEXT: )
;; CHECK-NEXT: (struct.set $struct 0
;; CHECK-NEXT: (local.get $temp2)
;; CHECK-NEXT: (local.get $temp)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $temp2)
;; CHECK-NEXT: )
(func $multiple-interactions-escapes (result anyref)
(local $temp (ref $struct))
(local $temp2 (ref $struct))
(local.set $temp
(struct.new_default $struct)
)
(local.set $temp2
(struct.new_default $struct)
)
(struct.set $struct 0
(local.get $temp2)
(local.get $temp)
)
;; As above, but now the second allocation escapes, so nothing is
;; optimized.
(local.get $temp2)
)
)
Loading