Skip to content

Fix Flatten on br_table with a more refined value #9319

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

Closed
kripken wants to merge 3 commits into from
Closed

Fix Flatten on br_table with a more refined value #9319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
acc2665
Fix Flatten on br_table with subtyping
kripken Oct 28, 2026
72366ec
simpl
kripken Oct 28, 2026
4dbc96b
comment
kripken Oct 28, 2026
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
15 changes: 7 additions & 8 deletions src/passes/Flatten.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,7 @@ struct Flatten
auto type = br->value->type;
if (type.isConcrete()) {
// we are sending a value. use a local instead
Type blockType = findBreakTarget(br->name)->type;
Index temp = getTempForBreakTarget(br->name, blockType);
Index temp = getTempForBreakTarget(br->name);
ourPreludes.push_back(builder.makeLocalSet(temp, br->value));

// br_if leaves a value on the stack if not taken, which later can
Expand All @@ -283,7 +282,7 @@ struct Flatten
// with anyref type in case for flowing out. Here we create the
// second 'flowing out' local in case two block's types are
// different.
if (type != blockType) {
if (type != findBreakTarget(br->name)->type) {

@gemini-code-assist gemini-code-assist Bot Oct 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Instead of calling findBreakTarget(br->name)->type again, which performs a redundant AST traversal up the parent stack, you can query the type of the local variable temp directly from the function using getFunction()->getLocalType(temp). Since temp was just retrieved or allocated using getTempForBreakTarget(br->name), its local type is guaranteed to match the target block's type.

Suggested change
if (type != findBreakTarget(br->name)->type) {
if (type != getFunction()->getLocalType(temp)) {

@kripken kripken Oct 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That is true, but more complicated (harder to refactor, in particular), and would require a comment.

temp = builder.addVar(getFunction(), type);
ourPreludes.push_back(builder.makeLocalSet(
temp, ExpressionManipulator::copy(br->value, *getModule())));
Expand Down Expand Up @@ -318,9 +317,8 @@ struct Flatten
// we don't know which break target will be hit - assign to them all
auto names = BranchUtils::getUniqueTargets(sw);
for (auto name : names) {
ourPreludes.push_back(
builder.makeLocalSet(getTempForBreakTarget(name, type),
builder.makeLocalGet(temp, type)));
ourPreludes.push_back(builder.makeLocalSet(
getTempForBreakTarget(name), builder.makeLocalGet(temp, type)));
}
sw->value = nullptr;
sw->finalize();
Expand Down Expand Up @@ -408,13 +406,14 @@ struct Flatten

// get the temp local to be used for breaks to that target. allocates
// one if there isn't one yet
Index getTempForBreakTarget(Name name, Type type) {
Index getTempForBreakTarget(Name name) {
auto iter = breakTemps.find(name);
if (iter != breakTemps.end()) {
return iter->second;
} else {
return breakTemps[name] =
Builder(*getModule()).addVar(getFunction(), type);
Builder(*getModule())
.addVar(getFunction(), findBreakTarget(name)->type);
}
}
};
Expand Down
34 changes: 34 additions & 0 deletions test/lit/passes/flatten.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,38 @@
;; modified
(local.get $0)
)

;; CHECK: (func $br_table-subtype (type $2) (result funcref)
;; CHECK-NEXT: (local $0 nullfuncref)
;; CHECK-NEXT: (local $1 funcref)
;; CHECK-NEXT: (local $2 funcref)
;; CHECK-NEXT: (block $block
;; CHECK-NEXT: (local.set $0
;; CHECK-NEXT: (ref.null nofunc)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $1
;; CHECK-NEXT: (local.get $0)
;; CHECK-NEXT: )
;; CHECK-NEXT: (br_table $block $block
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: )
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $2
;; CHECK-NEXT: (local.get $1)
;; CHECK-NEXT: )
;; CHECK-NEXT: (return
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $br_table-subtype (result funcref)
;; The br_table's value is more refined than the place we branch to, so we
;; need to be careful with the types of our temp locals.
(block $block (result funcref)
(br_table $block $block
(ref.null nofunc)
(i32.const 0)
)
)
)
)
Loading