Skip to content

Use fewer scratch locals in IRBuilder #8608

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 2 commits into from
Apr 16, 2026
Merged

Use fewer scratch locals in IRBuilder #8608

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
38c5f27
Use fewer scratch locals in IRBuilder
tlively Apr 15, 2026
1a3544b
valIndex => hoistIndex
tlively Apr 15, 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
11 changes: 8 additions & 3 deletions src/wasm-ir-builder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,15 +713,20 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
Result<Index> addScratchLocal(Type);

struct HoistedVal {
// The index in the stack of the original value-producing expression.
// The index in the stack of the deepest expression to be popped. This can
// be the original value-producing expression, or if we are popping
// greedily, it might be the deepest none-typed expression under the
// value-producing expression.
Index valIndex;
// The local.get placed on the stack, if any.
LocalGet* get;
};

// Find the last value-producing expression, if any, and hoist its value to
// the top of the stack using a scratch local if necessary.
MaybeResult<HoistedVal> hoistLastValue();
// the top of the stack using a scratch local if necessary. If `greedy`, then
// also include none-typed expressions and the value-producing expression in
// the hoisted range of expressions.
MaybeResult<HoistedVal> hoistLastValue(bool greedy = false);
// Transform the stack as necessary such that the original producer of the
// hoisted value will be popped along with the final expression that produces
// the value, if they are different. May only be called directly after
Expand Down
52 changes: 36 additions & 16 deletions src/wasm/wasm-ir-builder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,29 +61,37 @@ Result<Index> IRBuilder::addScratchLocal(Type type) {
return Builder::addVar(func, name, type);
}

MaybeResult<IRBuilder::HoistedVal> IRBuilder::hoistLastValue() {
MaybeResult<IRBuilder::HoistedVal> IRBuilder::hoistLastValue(bool greedy) {
auto& stack = getScope().exprStack;
int index = stack.size() - 1;
for (; index >= 0; --index) {
if (stack[index]->type != Type::none) {
int valIndex = stack.size() - 1;
for (; valIndex >= 0; --valIndex) {
if (stack[valIndex]->type != Type::none) {
break;
}
}
if (index < 0) {
if (valIndex < 0) {
// There is no value-producing or unreachable expression.
return {};
}
if (unsigned(index) == stack.size() - 1) {

int hoistIndex = valIndex;
if (greedy) {
while (hoistIndex > 0 && stack[hoistIndex - 1]->type == Type::none) {
--hoistIndex;
}
}

if (unsigned(valIndex) == stack.size() - 1) {
// Value-producing expression already on top of the stack.
return HoistedVal{Index(index), nullptr};
return HoistedVal{Index(hoistIndex), nullptr};
}
auto*& expr = stack[index];
auto*& expr = stack[valIndex];
if (expr->type == Type::unreachable) {
// Make sure the top of the stack also has an unreachable expression.
if (stack.back()->type != Type::unreachable) {
pushSynthetic(builder.makeUnreachable());
}
return HoistedVal{Index(index), nullptr};
return HoistedVal{Index(hoistIndex), nullptr};
}
// Hoist with a scratch local. Normally the scratch local is the same type as
// the hoisted expression, but we may need to adjust it given the enabled
Expand All @@ -99,7 +107,7 @@ MaybeResult<IRBuilder::HoistedVal> IRBuilder::hoistLastValue() {
expr = builder.makeLocalSet(*scratchIdx, expr);
auto* get = builder.makeLocalGet(*scratchIdx, type);
pushSynthetic(get);
return HoistedVal{Index(index), get};
return HoistedVal{Index(hoistIndex), get};
}

Result<> IRBuilder::packageHoistedValue(const HoistedVal& hoisted,
Expand All @@ -123,7 +131,7 @@ Result<> IRBuilder::packageHoistedValue(const HoistedVal& hoisted,
auto type = scope.exprStack.back()->type;

if (type.size() == sizeHint || type.size() <= 1) {
if (hoisted.get) {
if (hoisted.valIndex < scope.exprStack.size() - 1) {
Copy link
Copy Markdown
Member

@kripken kripken Apr 15, 2026

Choose a reason for hiding this comment

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

Why is this correct?

Copy link
Copy Markdown
Member Author

@tlively tlively Apr 15, 2026

Choose a reason for hiding this comment

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

Basically we need to packageAsBlock whenever we want to treat multiple expressions as a unit, rather than just using the expression on top of the stack. This used to be the case precisely when we also had a get of a scratch local. Now it's possible for the value-producing expression to be on top of the stack, so we don't need a scratch local, but for us to greedily be consuming other expressions as well, so we still need a block. We handle this by more explicitly checking whether the hoisted index is something other than the top of the stack.

(I'll rename it from valIndex, since that name no longer gives the right idea.)

Copy link
Copy Markdown
Member

@kripken kripken Apr 15, 2026

Choose a reason for hiding this comment

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

Makes sense, thanks!

packageAsBlock(type);
}
return Ok{};
Expand Down Expand Up @@ -379,8 +387,10 @@ struct IRBuilder::ChildPopper
continue;
}

// Pop a child normally.
auto val = pop(children[i].constraint.size());
// Pop a child normally. Pop greedily for children other than the first
// (i.e. the last to be popped).
bool greedy = i > 0;
auto val = pop(children[i].constraint.size(), greedy);
CHECK_ERR(val);
*children[i].childp = *val;
}
Expand Down Expand Up @@ -458,12 +468,22 @@ struct IRBuilder::ChildPopper
return false;
}

Result<Expression*> pop(size_t size) {
// If `greedy`, then we will pop additional none-typed expressions that come
// before the value-producing expression. The additional expressions will be
// packaged into a block with the value-producing expression. This is better
// than leaving them on top of the stack, where they will force the use of a
// scratch local when the next operand is popped. `greedy` should be used when
// popping all children of an expression except the first (i.e. the
// last child to be popped). Not being greedy for the last popped child defers
// the creation of a block to hold its none-typed predecessors. It may turn
// out that such a block is not necessary, for example when the none-typed
// expressions can be included directly into a parent block scope.
Result<Expression*> pop(size_t size, bool greedy = false) {
assert(size >= 1);
auto& scope = builder.getScope();

// Find the suffix of expressions that do not produce values.
auto hoisted = builder.hoistLastValue();
auto hoisted = builder.hoistLastValue(greedy);
CHECK_ERR(hoisted);
if (!hoisted) {
// There are no expressions that produce values.
Expand All @@ -489,7 +509,7 @@ struct IRBuilder::ChildPopper
std::vector<Expression*> elems;
elems.resize(size);
for (int i = size - 1; i >= 0; --i) {
auto elem = pop(1);
auto elem = pop(1, greedy || i > 0);
CHECK_ERR(elem);
elems[i] = *elem;
}
Expand Down
44 changes: 18 additions & 26 deletions test/lit/basic/extra-branch-values.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2383,17 +2383,15 @@
;; CHECK-NEXT: (local $scratch_6 i32)
;; CHECK-NEXT: (local $scratch_7 i32)
;; CHECK-NEXT: (local $scratch_8 (ref any))
;; CHECK-NEXT: (local $scratch_9 i32)
;; CHECK-NEXT: (local $scratch_10 anyref)
;; CHECK-NEXT: (local $scratch_11 i32)
;; CHECK-NEXT: (local $scratch_12 (ref any))
;; CHECK-NEXT: (local $scratch_13 i32)
;; CHECK-NEXT: (local $scratch_14 eqref)
;; CHECK-NEXT: (local $scratch_9 anyref)
;; CHECK-NEXT: (local $scratch_10 i32)
;; CHECK-NEXT: (local $scratch_11 (ref any))
;; CHECK-NEXT: (local $scratch_12 eqref)
;; CHECK-NEXT: (local.set $scratch
;; CHECK-NEXT: (local.get $0)
;; CHECK-NEXT: )
;; CHECK-NEXT: (block $label (type $0) (result i32 eqref)
;; CHECK-NEXT: (local.set $scratch_14
;; CHECK-NEXT: (local.set $scratch_12
;; CHECK-NEXT: (block $label0 (result eqref)
;; CHECK-NEXT: (br $label
;; CHECK-NEXT: (if (type $0) (result i32 eqref)
Expand All @@ -2416,46 +2414,40 @@
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (tuple.make 2
;; CHECK-NEXT: (block (result i32)
;; CHECK-NEXT: (local.set $scratch_9
;; CHECK-NEXT: (local.get $scratch_6)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $scratch_6)
;; CHECK-NEXT: (block (result (ref eq))
;; CHECK-NEXT: (global.set $any
;; CHECK-NEXT: (local.get $scratch_8)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $scratch_9)
;; CHECK-NEXT: (global.get $eq)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.get $eq)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (else
;; CHECK-NEXT: (local.set $scratch_6
;; CHECK-NEXT: (block (result i32)
;; CHECK-NEXT: (local.set $scratch_11
;; CHECK-NEXT: (local.set $scratch_10
;; CHECK-NEXT: (local.get $scratch)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $scratch_10
;; CHECK-NEXT: (local.set $scratch_9
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $scratch_11)
;; CHECK-NEXT: (local.get $scratch_10)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $scratch_12
;; CHECK-NEXT: (local.set $scratch_11
;; CHECK-NEXT: (br_on_cast $label0 anyref eqref
;; CHECK-NEXT: (local.get $scratch_10)
;; CHECK-NEXT: (local.get $scratch_9)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (tuple.make 2
;; CHECK-NEXT: (block (result i32)
;; CHECK-NEXT: (local.set $scratch_13
;; CHECK-NEXT: (local.get $scratch_6)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $scratch_6)
;; CHECK-NEXT: (block (result eqref)
;; CHECK-NEXT: (global.set $any
;; CHECK-NEXT: (local.get $scratch_12)
;; CHECK-NEXT: (local.get $scratch_11)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $scratch_13)
;; CHECK-NEXT: (global.get $eqref)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.get $eqref)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
Expand All @@ -2464,7 +2456,7 @@
;; CHECK-NEXT: )
;; CHECK-NEXT: (tuple.make 2
;; CHECK-NEXT: (local.get $scratch_6)
;; CHECK-NEXT: (local.get $scratch_14)
;; CHECK-NEXT: (local.get $scratch_12)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
Expand Down
44 changes: 0 additions & 44 deletions test/lit/passes/roundtrip-gc.wast
View file
Open in desktop

This file was deleted.

Loading
Loading