Skip to content

Commit 359392a

Browse files
tlivelyradekdoulik
authored andcommitted
Refactor IRBuilder to build blocks internally (WebAssembly#5901)
The initial PR introducing IRBuilder kept the interface the same as the previous internal interface in the new wat parser. This PR updates that interface to avoid exposing implementation details of the IRBuilder and to provide an API that matches the binary format. For example, after calling `makeBlock` or `visitBlock` at the beginning of a block, users now call `visitEnd()` at the end of the block without having to manually install the block's contents. Providing this improved interface requires refactoring some of the IRBuilder internals. While we are refactoring things anyway, put in extra effort to avoid unnecessarily splitting up and recombining tuples that could simply be returned from a multivalue block.
1 parent fb6146f commit 359392a

File tree

4 files changed

+475
-335
lines changed

4 files changed

+475
-335
lines changed

src/wasm-ir-builder.h

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,7 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
4242
// Get the valid Binaryen IR expression representing the sequence of visited
4343
// instructions. The IRBuilder is reset and can be used with a fresh sequence
4444
// of instructions after this is called.
45-
Expression* build();
46-
47-
[[nodiscard]] Result<std::vector<Expression*>> finishInstrs();
45+
[[nodiscard]] Result<Expression*> build();
4846

4947
// Call visit() on an existing Expression with its non-child fields
5048
// initialized to initialize the child fields and refinalize it. The specific
@@ -56,11 +54,13 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
5654
[[nodiscard]] Result<> visitStructNew(StructNew*);
5755
[[nodiscard]] Result<> visitArrayNew(ArrayNew*);
5856

57+
[[nodiscard]] Result<> visitEnd();
58+
5959
// Alternatively, call makeXYZ to have the IRBuilder allocate the nodes. This
6060
// is generally safer than calling `visit` because the function signatures
6161
// ensure that there are no missing fields.
6262
[[nodiscard]] Result<> makeNop();
63-
[[nodiscard]] Result<> makeBlock();
63+
[[nodiscard]] Result<> makeBlock(Name label, Type type);
6464
// [[nodiscard]] Result<> makeIf();
6565
// [[nodiscard]] Result<> makeLoop();
6666
// [[nodiscard]] Result<> makeBreak();
@@ -168,9 +168,6 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
168168
// [[nodiscard]] Result<> makeStringSliceWTF();
169169
// [[nodiscard]] Result<> makeStringSliceIter();
170170

171-
// TODO: make this private.
172-
void pushScope(Type type) { scopeStack.push_back({{}, type}); }
173-
174171
void setFunction(Function* func) { this->func = func; }
175172

176173
private:
@@ -183,24 +180,42 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
183180
// to have.
184181
struct BlockCtx {
185182
std::vector<Expression*> exprStack;
186-
Type type;
183+
Block* block;
184+
// Whether we have seen an unreachable instruction and are in
185+
// stack-polymorphic unreachable mode.
186+
bool unreachable = false;
187187
};
188188

189189
// The stack of block contexts currently being parsed.
190190
std::vector<BlockCtx> scopeStack;
191-
std::vector<Expression*>& getExprStack();
192-
Type getResultType() {
193-
assert(!scopeStack.empty());
194-
return scopeStack.back().type;
191+
192+
BlockCtx& getScope() {
193+
if (scopeStack.empty()) {
194+
// We are not in a block context, so push a dummy scope.
195+
scopeStack.push_back({{}, nullptr});
196+
}
197+
return scopeStack.back();
195198
}
196199

197-
// Whether we have seen an unreachable instruction and are in
198-
// stack-polymorphic unreachable mode.
199-
bool unreachable = false;
200+
[[nodiscard]] Result<Index> addScratchLocal(Type);
201+
[[nodiscard]] Result<Expression*> pop();
202+
void push(Expression*);
203+
204+
struct HoistedVal {
205+
// The index in the stack of the original value-producing expression.
206+
Index valIndex;
207+
// The local.get placed on the stack, if any.
208+
LocalGet* get;
209+
};
200210

201-
Result<Index> addScratchLocal(Type);
202-
[[nodiscard]] Result<> push(Expression*);
203-
Result<Expression*> pop();
211+
// Find the last value-producing expression, if any, and hoist its value to
212+
// the top of the stack using a scratch local if necessary.
213+
[[nodiscard]] MaybeResult<HoistedVal> hoistLastValue();
214+
// Transform the stack as necessary such that the original producer of the
215+
// hoisted value will be popped along with the final expression that produces
216+
// the value, if they are different. May only be called directly after
217+
// hoistLastValue().
218+
[[nodiscard]] Result<> packageHoistedValue(const HoistedVal&);
204219
};
205220

206221
} // namespace wasm

0 commit comments

Comments
 (0)