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

Preserve multivalue drops in IRBuilder #6150

Merged
merged 7 commits into from
Dec 13, 2023
Merged

Preserve multivalue drops in IRBuilder #6150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
86b2508
Preserve multivalue drops in IRBuilder
tlively Dec 7, 2023
066426b
fix
tlively Dec 7, 2023
1451c34
test
tlively Dec 7, 2023
19a89ad
update tests
tlively Dec 13, 2023
8758620
fix memory error
tlively Dec 13, 2023
257a807
fix
tlively Dec 13, 2023
9ab99a7
thank you ashley for the suggestion
tlively Dec 13, 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
12 changes: 9 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 @@ -200,6 +200,8 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {

// Private functions that must be public for technical reasons.
[[nodiscard]] Result<> visitExpression(Expression*);
[[nodiscard]] Result<>
visitDrop(Drop*, std::optional<uint32_t> arity = std::nullopt);
[[nodiscard]] Result<> visitIf(If*);
[[nodiscard]] Result<> visitReturn(Return*);
[[nodiscard]] Result<> visitStructNew(StructNew*);
Expand Down Expand Up @@ -463,7 +465,7 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
[[nodiscard]] Result<Name> getLabelName(Index label);
[[nodiscard]] Result<Name> getDelegateLabelName(Index label);
[[nodiscard]] Result<Index> addScratchLocal(Type);
[[nodiscard]] Result<Expression*> pop();
[[nodiscard]] Result<Expression*> pop(size_t size = 1);

struct HoistedVal {
// The index in the stack of the original value-producing expression.
Expand All @@ -478,8 +480,12 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
// 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
// hoistLastValue().
[[nodiscard]] Result<> packageHoistedValue(const HoistedVal&);
// hoistLastValue(). `sizeHint` is the size of the type we ultimately want to
// consume, so if the hoisted value has `sizeHint` elements, it is left intact
// even if it is a tuple. Otherwise, hoisted tuple values will be broken into
// pieces.
[[nodiscard]] Result<> packageHoistedValue(const HoistedVal&,
size_t sizeHint = 1);

[[nodiscard]] Result<Expression*> getBranchValue(Name labelName,
std::optional<Index> label);
Expand Down
44 changes: 37 additions & 7 deletions src/wasm/wasm-ir-builder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ MaybeResult<IRBuilder::HoistedVal> IRBuilder::hoistLastValue() {
return HoistedVal{Index(index), get};
}

Result<> IRBuilder::packageHoistedValue(const HoistedVal& hoisted) {
Result<> IRBuilder::packageHoistedValue(const HoistedVal& hoisted,
size_t sizeHint) {
auto& scope = getScope();
assert(!scope.exprStack.empty());

Expand All @@ -106,7 +107,7 @@ Result<> IRBuilder::packageHoistedValue(const HoistedVal& hoisted) {

auto type = scope.exprStack.back()->type;

if (!type.isTuple()) {
if (type.size() == sizeHint) {
if (hoisted.get) {
packageAsBlock(type);
}
Expand Down Expand Up @@ -158,7 +159,8 @@ void IRBuilder::push(Expression* expr) {
DBG(dump());
}

Result<Expression*> IRBuilder::pop() {
Result<Expression*> IRBuilder::pop(size_t size) {
assert(size >= 1);
auto& scope = getScope();

// Find the suffix of expressions that do not produce values.
Expand All @@ -173,11 +175,25 @@ Result<Expression*> IRBuilder::pop() {
return Err{"popping from empty stack"};
}

CHECK_ERR(packageHoistedValue(*hoisted));
CHECK_ERR(packageHoistedValue(*hoisted, size));

auto* ret = scope.exprStack.back();
scope.exprStack.pop_back();
return ret;
if (ret->type.size() == size) {
scope.exprStack.pop_back();
return ret;
}

// The last value-producing expression did not produce exactly the right
// number of values, so we need to construct a tuple piecewise instead.
assert(size > 1);
std::vector<Expression*> elems;
elems.resize(size);
for (int i = size - 1; i >= 0; --i) {
auto elem = pop();
CHECK_ERR(elem);
elems[i] = *elem;
}
return builder.makeTupleMake(elems);
}

Result<Expression*> IRBuilder::build() {
Expand Down Expand Up @@ -319,6 +335,20 @@ Result<> IRBuilder::visitExpression(Expression* curr) {
return Ok{};
}

Result<> IRBuilder::visitDrop(Drop* curr, std::optional<uint32_t> arity) {
// Multivalue drops must remain multivalue drops.
if (!arity) {
arity = curr->value->type.size();
}
if (*arity >= 2) {
auto val = pop(*arity);
CHECK_ERR(val);
curr->value = *val;
return Ok{};
}
return visitExpression(curr);
}

Result<> IRBuilder::visitIf(If* curr) {
// Only the condition is popped from the stack. The ifTrue and ifFalse are
// self-contained so we do not modify them.
Expand Down Expand Up @@ -1183,7 +1213,7 @@ Result<> IRBuilder::makeSelect(std::optional<Type> type) {

Result<> IRBuilder::makeDrop() {
Drop curr;
CHECK_ERR(visitDrop(&curr));
CHECK_ERR(visitDrop(&curr, 1));
push(builder.makeDrop(curr.value));
return Ok{};
}
Expand Down
52 changes: 26 additions & 26 deletions test/lit/passes/outlining.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -729,20 +729,16 @@
)

;; Test outlining works with call_indirect
;; 0 results, 2 params, 3 operands
;; 0 results, 0 params, 1 operand
(module
(table funcref)
;; CHECK: (type $0 (func))

;; CHECK: (type $1 (func (param i32 i32)))

;; CHECK: (table $0 0 funcref)

;; CHECK: (func $outline$ (type $0)
;; CHECK-NEXT: (call_indirect $0 (type $1)
;; CHECK-NEXT: (call_indirect $0 (type $0)
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: (i32.const 1)
;; CHECK-NEXT: (i32.const 2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )

Expand All @@ -752,31 +748,29 @@
;; CHECK-NEXT: )
(func $a
(call_indirect
(param i32 i32)
(i32.const 0)
(i32.const 1)
(i32.const 2)
)
(call_indirect
(param i32 i32)
(i32.const 0)
(i32.const 1)
(i32.const 2)
)
)
)

;; Test outlining works with call_indirect
;; 0 results, 0 params, 1 operand
;; 1 result, 0 params, 1 operand
(module
(table funcref)
;; CHECK: (type $0 (func))

;; CHECK: (type $1 (func (result i32)))

;; CHECK: (table $0 0 funcref)

;; CHECK: (func $outline$ (type $0)
;; CHECK-NEXT: (call_indirect $0 (type $0)
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (call_indirect $0 (type $1)
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )

Expand All @@ -785,27 +779,33 @@
;; CHECK-NEXT: (call $outline$)
;; CHECK-NEXT: )
(func $a
(call_indirect
(i32.const 0)
(drop
(call_indirect
(result i32)
(i32.const 0)
)
)
(call_indirect
(i32.const 0)
(drop
(call_indirect
(result i32)
(i32.const 0)
)
)
)
)

;; Test outlining works with call_indirect
;; 1 result, 0 params, 1 operand
;; 2 results, 0 params, 1 operand
(module
(table funcref)
;; CHECK: (type $0 (func))

;; CHECK: (type $1 (func (result i32)))
;; CHECK: (type $1 (func (result i32 i32)))

;; CHECK: (table $0 0 funcref)

;; CHECK: (func $outline$ (type $0)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (tuple.drop 2
;; CHECK-NEXT: (call_indirect $0 (type $1)
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: )
Expand All @@ -817,15 +817,15 @@
;; CHECK-NEXT: (call $outline$)
;; CHECK-NEXT: )
(func $a
(drop
(tuple.drop 2
(call_indirect
(result i32)
(result i32 i32)
(i32.const 0)
)
)
(drop
(tuple.drop 2
(call_indirect
(result i32)
(result i32 i32)
(i32.const 0)
)
)
Expand Down
Loading