Skip to content

Commit

Permalink
[Parser] Parse pops (by doing nothing) (WebAssembly#6252)
Browse files Browse the repository at this point in the history 
Parse pop expressions and check that they have the expected types, but do not
actually create new Pop expressions or push anything onto the stack because we
already create Pop expressions as necessary when visiting the beginning of catch
blocks.

Unlike the legacy text parser, the new text parser is not capable of parsing
pops in invalid locations in the IR. This means that the new text parser will
never be able to parse test/lit/catch-pop-fixup-eh-old.wast, which deliberately
parses invalid IR to check that the pops can be fixed up and moved to the
correct locations. It should be acceptable to delete that test when we turn on
the new parser by default, though, so that won't be a problem.
  • Loading branch information
@tlively @radekdoulik
tlively authored and radekdoulik committed Jul 12, 2024
1 parent a4c047f commit 2e43c0c
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 26 deletions.
5 changes: 5 additions & 0 deletions src/parser/contexts.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ struct NullInstrParserCtx {

Result<> makeMemoryCopy(Index, MemoryIdxT*, MemoryIdxT*) { return Ok{}; }
Result<> makeMemoryFill(Index, MemoryIdxT*) { return Ok{}; }
template<typename TypeT> Result<> makePop(Index, TypeT) { return Ok{}; }
Result<> makeCall(Index, FuncIdxT, bool) { return Ok{}; }
template<typename TypeUseT>
Result<> makeCallIndirect(Index, TableIdxT*, TypeUseT, bool) {
Expand Down Expand Up @@ -1649,6 +1650,10 @@ struct ParseDefsCtx : TypeParserCtx<ParseDefsCtx> {
return withLoc(pos, irBuilder.makeMemoryFill(*m));
}

Result<> makePop(Index pos, Type type) {
return withLoc(pos, irBuilder.makePop(type));
}

Result<> makeCall(Index pos, Name func, bool isReturn) {
return withLoc(pos, irBuilder.makeCall(func, isReturn));
}
Expand Down
4 changes: 3 additions & 1 deletion src/parser/parsers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1555,7 +1555,9 @@ template<typename Ctx> Result<> makeMemoryFill(Ctx& ctx, Index pos) {
}

template<typename Ctx> Result<> makePop(Ctx& ctx, Index pos) {
return ctx.in.err("unimplemented instruction");
auto type = valtype(ctx);
CHECK_ERR(type);
return ctx.makePop(pos, *type);
}

template<typename Ctx> Result<> makeCall(Ctx& ctx, Index pos, bool isReturn) {
Expand Down
3 changes: 2 additions & 1 deletion src/wasm-ir-builder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
[[nodiscard]] Result<> makeMemorySize(Name mem);
[[nodiscard]] Result<> makeMemoryGrow(Name mem);
[[nodiscard]] Result<> makeUnreachable();
// [[nodiscard]] Result<> makePop();
[[nodiscard]] Result<> makePop(Type type);
[[nodiscard]] Result<> makeRefNull(HeapType type);
[[nodiscard]] Result<> makeRefIsNull();
[[nodiscard]] Result<> makeRefFunc(Name func);
Expand Down Expand Up @@ -232,6 +232,7 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
[[nodiscard]] Result<>
visitTupleExtract(TupleExtract*,
std::optional<uint32_t> arity = std::nullopt);
[[nodiscard]] Result<> visitPop(Pop*);

private:
Module& wasm;
Expand Down
24 changes: 23 additions & 1 deletion src/wasm/wasm-ir-builder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,12 @@ Result<> IRBuilder::visitTupleExtract(TupleExtract* curr,
return Ok{};
}

Result<> IRBuilder::visitPop(Pop*) {
// Do not actually push this pop onto the stack since we generate our own pops
// as necessary when visiting the beginnings of try blocks.
return Ok{};
}

Result<> IRBuilder::visitFunctionStart(Function* func) {
if (!scopeStack.empty()) {
return Err{"unexpected start of function"};
Expand Down Expand Up @@ -1300,7 +1306,23 @@ Result<> IRBuilder::makeUnreachable() {
return Ok{};
}

// Result<> IRBuilder::makePop() {}
Result<> IRBuilder::makePop(Type type) {
// We don't actually want to create a new Pop expression here because we
// already create them automatically when starting a legacy catch block that
// needs one. Just verify that the Pop we are being asked to make is the same
// type as the Pop we have already made.
auto& scope = getScope();
if (!scope.getCatch() || scope.exprStack.size() != 1 ||
!scope.exprStack[0]->is<Pop>()) {
return Err{
"pop instructions may only appear at the beginning of catch blocks"};
}
auto expectedType = scope.exprStack[0]->type;
if (type != expectedType) {
return Err{std::string("Expected pop of type ") + expectedType.toString()};
}
return Ok{};
}

Result<> IRBuilder::makeRefNull(HeapType type) {
push(builder.makeRefNull(type));
Expand Down
67 changes: 44 additions & 23 deletions test/lit/wat-kitchen-sink.wast
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
(type $ret2 (func (result i32 i32)))

(rec
;; CHECK: (type $pair (struct (field (mut i32)) (field (mut i64))))
;; CHECK: (type $3 (func (result i32 i64)))

;; CHECK: (type $4 (func (param i32 i64)))
;; CHECK: (type $pair (struct (field (mut i32)) (field (mut i64))))

;; CHECK: (type $5 (func (result i32 i64)))
;; CHECK: (type $5 (func (param i32 i64)))

;; CHECK: (type $a2 (array (mut f32)))

Expand Down Expand Up @@ -1815,8 +1815,8 @@
end
)

;; CHECK: (func $try-catch-params (type $5) (result i32 i64)
;; CHECK-NEXT: (try (type $5) (result i32 i64)
;; CHECK: (func $try-catch-params (type $3) (result i32 i64)
;; CHECK-NEXT: (try (type $3) (result i32 i64)
;; CHECK-NEXT: (do
;; CHECK-NEXT: (tuple.make 2
;; CHECK-NEXT: (i32.const 0)
Expand All @@ -1836,6 +1836,27 @@
end
)

;; CHECK: (func $try-catch-pop (type $3) (result i32 i64)
;; CHECK-NEXT: (try (type $3) (result i32 i64)
;; CHECK-NEXT: (do
;; CHECK-NEXT: (tuple.make 2
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: (i64.const 1)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (catch $tag-pair
;; CHECK-NEXT: (pop (tuple i32 i64))
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $try-catch-pop (result i32 i64)
try (result i32 i64)
i32.const 0
i64.const 1
catch $tag-pair
pop (tuple i32 i64)
end
)

;; CHECK: (func $try-catch_all (type $void)
;; CHECK-NEXT: (try
Expand Down Expand Up @@ -2674,8 +2695,8 @@
br 0
)

;; CHECK: (func $br-multivalue (type $5) (result i32 i64)
;; CHECK-NEXT: (block $label (type $5) (result i32 i64)
;; CHECK: (func $br-multivalue (type $3) (result i32 i64)
;; CHECK-NEXT: (block $label (type $3) (result i32 i64)
;; CHECK-NEXT: (br $label
;; CHECK-NEXT: (tuple.make 2
;; CHECK-NEXT: (i32.const 0)
Expand All @@ -2690,9 +2711,9 @@
br 0
)

;; CHECK: (func $br-multivalue-drop (type $5) (result i32 i64)
;; CHECK-NEXT: (block $label (type $5) (result i32 i64)
;; CHECK-NEXT: (block (type $5) (result i32 i64)
;; CHECK: (func $br-multivalue-drop (type $3) (result i32 i64)
;; CHECK-NEXT: (block $label (type $3) (result i32 i64)
;; CHECK-NEXT: (block (type $3) (result i32 i64)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (f32.const 0)
;; CHECK-NEXT: )
Expand Down Expand Up @@ -2892,9 +2913,9 @@
end
)

;; CHECK: (func $br-table-multivalue (type $5) (result i32 i64)
;; CHECK-NEXT: (block $a (type $5) (result i32 i64)
;; CHECK-NEXT: (block $b (type $5) (result i32 i64)
;; CHECK: (func $br-table-multivalue (type $3) (result i32 i64)
;; CHECK-NEXT: (block $a (type $3) (result i32 i64)
;; CHECK-NEXT: (block $b (type $3) (result i32 i64)
;; CHECK-NEXT: (br_table $a $b
;; CHECK-NEXT: (tuple.make 2
;; CHECK-NEXT: (i32.const 42)
Expand Down Expand Up @@ -3049,7 +3070,7 @@
drop
)

;; CHECK: (func $memory-grow (type $4) (param $0 i32) (param $1 i64)
;; CHECK: (func $memory-grow (type $5) (param $0 i32) (param $1 i64)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (memory.grow $mimport$0
;; CHECK-NEXT: (local.get $0)
Expand Down Expand Up @@ -3107,7 +3128,7 @@
global.set $pair
)

;; CHECK: (func $load (type $4) (param $0 i32) (param $1 i64)
;; CHECK: (func $load (type $5) (param $0 i32) (param $1 i64)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (i32.load $mimport$0 offset=42
;; CHECK-NEXT: (local.get $0)
Expand Down Expand Up @@ -3136,7 +3157,7 @@
drop
)

;; CHECK: (func $store (type $4) (param $0 i32) (param $1 i64)
;; CHECK: (func $store (type $5) (param $0 i32) (param $1 i64)
;; CHECK-NEXT: (i32.store $mimport$0 offset=42 align=1
;; CHECK-NEXT: (local.get $0)
;; CHECK-NEXT: (i32.const 0)
Expand All @@ -3162,7 +3183,7 @@
f32.store $mem-i64
)

;; CHECK: (func $atomic-rmw (type $4) (param $0 i32) (param $1 i64)
;; CHECK: (func $atomic-rmw (type $5) (param $0 i32) (param $1 i64)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (i32.atomic.rmw16.add_u $mimport$0
;; CHECK-NEXT: (local.get $0)
Expand All @@ -3187,7 +3208,7 @@
drop
)

;; CHECK: (func $atomic-cmpxchg (type $4) (param $0 i32) (param $1 i64)
;; CHECK: (func $atomic-cmpxchg (type $5) (param $0 i32) (param $1 i64)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (i32.atomic.rmw8.cmpxchg_u $mem
;; CHECK-NEXT: (local.get $0)
Expand Down Expand Up @@ -3216,7 +3237,7 @@
drop
)

;; CHECK: (func $atomic-wait (type $4) (param $0 i32) (param $1 i64)
;; CHECK: (func $atomic-wait (type $5) (param $0 i32) (param $1 i64)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (memory.atomic.wait32 $mimport$0
;; CHECK-NEXT: (local.get $0)
Expand Down Expand Up @@ -3245,7 +3266,7 @@
drop
)

;; CHECK: (func $atomic-notify (type $4) (param $0 i32) (param $1 i64)
;; CHECK: (func $atomic-notify (type $5) (param $0 i32) (param $1 i64)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (memory.atomic.notify $mimport$0 offset=8
;; CHECK-NEXT: (local.get $0)
Expand Down Expand Up @@ -3337,7 +3358,7 @@
i8x16.shl
)

;; CHECK: (func $simd-load (type $4) (param $0 i32) (param $1 i64)
;; CHECK: (func $simd-load (type $5) (param $0 i32) (param $1 i64)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (v128.load8x8_s $mimport$0 offset=8
;; CHECK-NEXT: (local.get $0)
Expand Down Expand Up @@ -3453,7 +3474,7 @@
memory.copy $mem-i64 5
)

;; CHECK: (func $memory-fill (type $4) (param $0 i32) (param $1 i64)
;; CHECK: (func $memory-fill (type $5) (param $0 i32) (param $1 i64)
;; CHECK-NEXT: (memory.fill $mimport$0
;; CHECK-NEXT: (local.get $0)
;; CHECK-NEXT: (i32.const 1)
Expand Down Expand Up @@ -3568,7 +3589,7 @@
(func $ref-func
ref.func $ref-func
drop
ref.func 153
ref.func 154
drop
)

Expand Down

0 comments on commit 2e43c0c

Please sign in to comment.