Skip to content

[Parser] Parse BrOn expressions #6100

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 1 commit into from
Nov 15, 2023
Merged

[Parser] Parse BrOn expressions #6100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 11 additions & 0 deletions src/parser/contexts.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,12 @@ struct NullInstrParserCtx {
template<typename TypeT> Result<> makeRefTest(Index, TypeT) { return Ok{}; }
template<typename TypeT> Result<> makeRefCast(Index, TypeT) { return Ok{}; }

Result<> makeBrOn(Index, LabelIdxT, BrOnOp) { return Ok{}; }

template<typename TypeT> Result<> makeBrOn(Index, LabelIdxT, BrOnOp, TypeT) {
return Ok{};
}

template<typename HeapTypeT> Result<> makeStructNew(Index, HeapTypeT) {
return Ok{};
}
Expand Down Expand Up @@ -1290,6 +1296,11 @@ struct ParseDefsCtx : TypeParserCtx<ParseDefsCtx> {
return withLoc(pos, irBuilder.makeRefCast(type));
}

Result<>
makeBrOn(Index pos, Index label, BrOnOp op, Type castType = Type::none) {
return withLoc(pos, irBuilder.makeBrOn(label, op, castType));
}

Result<> makeStructNew(Index pos, HeapType type) {
return withLoc(pos, irBuilder.makeStructNew(type));
}
Expand Down
10 changes: 8 additions & 2 deletions src/parser/parsers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1315,11 +1315,17 @@ template<typename Ctx> Result<> makeRefCast(Ctx& ctx, Index pos) {
}

template<typename Ctx> Result<> makeBrOnNull(Ctx& ctx, Index pos, bool onFail) {
Copy link
Collaborator

@ashleynh ashleynh Nov 13, 2023

Choose a reason for hiding this comment

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

I'm missing where the Contexts are created and how they are passed into the "make" series of functions.

Copy link
Member Author

@tlively tlively Nov 15, 2023

Choose a reason for hiding this comment

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

The contexts are created and passed to the top-level parse function here: https://github.com/WebAssembly/binaryen/blob/main/src/parser/wat-parser.cpp#L96-L169

return ctx.in.err("unimplemented instruction");
auto label = labelidx(ctx);
CHECK_ERR(label);
return ctx.makeBrOn(pos, *label, onFail ? BrOnNonNull : BrOnNull);
}

template<typename Ctx> Result<> makeBrOnCast(Ctx& ctx, Index pos, bool onFail) {
return ctx.in.err("unimplemented instruction");
auto label = labelidx(ctx);
CHECK_ERR(label);
auto type = reftype(ctx);
CHECK_ERR(type);
return ctx.makeBrOn(pos, *label, onFail ? BrOnCastFail : BrOnCast, *type);
}

template<typename Ctx>
Expand Down
3 changes: 2 additions & 1 deletion src/wasm-ir-builder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
// [[nodiscard]] Result<> makeCallRef();
[[nodiscard]] Result<> makeRefTest(Type type);
[[nodiscard]] Result<> makeRefCast(Type type);
// [[nodiscard]] Result<> makeBrOn();
[[nodiscard]] Result<>
makeBrOn(Index label, BrOnOp op, Type castType = Type::none);
[[nodiscard]] Result<> makeStructNew(HeapType type);
[[nodiscard]] Result<> makeStructNewDefault(HeapType type);
[[nodiscard]] Result<>
Expand Down
1 change: 1 addition & 0 deletions src/wasm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1566,6 +1566,7 @@ class RefCast : public SpecificExpression<Expression::RefCastId> {

class BrOn : public SpecificExpression<Expression::BrOnId> {
public:
BrOn() = default;
BrOn(MixedArena& allocator) {}

BrOnOp op;
Expand Down
18 changes: 11 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 @@ -220,18 +220,15 @@ Result<> IRBuilder::visitExpression(Expression* curr) {
#define DELEGATE_FIELD_CHILD_VECTOR(id, field) \
WASM_UNREACHABLE("should have called visit" #id " because " #id \
" has child vector " #field);
#define DELEGATE_FIELD_SCOPE_NAME_USE(id, field) \
WASM_UNREACHABLE("should have called visit" #id " because " #id \
" has scope name use " #field);
#define DELEGATE_FIELD_SCOPE_NAME_USE_VECTOR(id, field) \
WASM_UNREACHABLE("should have called visit" #id " because " #id \
" has scope name use vector " #field);

#define DELEGATE_FIELD_INT(id, field)
#define DELEGATE_FIELD_INT_ARRAY(id, field)
#define DELEGATE_FIELD_LITERAL(id, field)
#define DELEGATE_FIELD_NAME(id, field)
#define DELEGATE_FIELD_NAME_VECTOR(id, field)
#define DELEGATE_FIELD_SCOPE_NAME_USE(id, field)
#define DELEGATE_FIELD_SCOPE_NAME_USE_VECTOR(id, field)
Copy link
Collaborator

@ashleynh ashleynh Nov 13, 2023

Choose a reason for hiding this comment

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

What does the vector version do that's different?

Copy link
Member Author

@tlively tlively Nov 15, 2023

Choose a reason for hiding this comment

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

br_table is the only instruction that takes a list of labels, so it is the only one that currently uses NAME_USE_VECTOR.

I'm removing the error when visiting NAME_USE because it turns out the default visitExpression implementation Just Works for BrOn, even though it includes NAME_USE. I'm making the same change for NAME_USE_VECTOR for consistency. There's nothing about having a NAME_USE_VECTOR that fundamentally means that this default implementation won't work.


#define DELEGATE_FIELD_TYPE(id, field)
#define DELEGATE_FIELD_HEAPTYPE(id, field)
#define DELEGATE_FIELD_ADDRESS(id, field)
Expand Down Expand Up @@ -933,7 +930,14 @@ Result<> IRBuilder::makeRefCast(Type type) {
return Ok{};
}

// Result<> IRBuilder::makeBrOn() {}
Result<> IRBuilder::makeBrOn(Index label, BrOnOp op, Type castType) {
BrOn curr;
CHECK_ERR(visitBrOn(&curr));
auto name = getLabelName(label);
CHECK_ERR(name);
push(builder.makeBrOn(op, *name, curr.ref, castType));
return Ok{};
}

Result<> IRBuilder::makeStructNew(HeapType type) {
StructNew curr(wasm.allocator);
Expand Down
108 changes: 96 additions & 12 deletions test/lit/wat-kitchen-sink.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@

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

;; CHECK: (type $8 (func (param i32)))
;; CHECK: (type $8 (func (param anyref)))

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

;; CHECK: (type $10 (func (param v128 i32) (result v128)))
;; CHECK: (type $10 (func (param i32 i32 i32)))

;; CHECK: (type $11 (func (param anyref)))
;; CHECK: (type $11 (func (param v128 i32) (result v128)))

;; CHECK: (rec
;; CHECK-NEXT: (type $s0 (struct ))
Expand Down Expand Up @@ -234,11 +234,11 @@
;; CHECK-NEXT: (nop)
;; CHECK-NEXT: )

;; CHECK: (func $f1 (type $8) (param $0 i32)
;; CHECK: (func $f1 (type $9) (param $0 i32)
;; CHECK-NEXT: (nop)
;; CHECK-NEXT: )
(func $f1 (param i32))
;; CHECK: (func $f2 (type $8) (param $x i32)
;; CHECK: (func $f2 (type $9) (param $x i32)
;; CHECK-NEXT: (nop)
;; CHECK-NEXT: )
(func $f2 (param $x i32))
Expand Down Expand Up @@ -1756,7 +1756,7 @@
drop
)

;; CHECK: (func $select (type $9) (param $0 i32) (param $1 i32) (param $2 i32)
;; CHECK: (func $select (type $10) (param $0 i32) (param $1 i32) (param $2 i32)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (select
;; CHECK-NEXT: (local.get $0)
Expand Down Expand Up @@ -2048,7 +2048,7 @@
i32x4.extract_lane 3
)

;; CHECK: (func $simd-replace (type $10) (param $0 v128) (param $1 i32) (result v128)
;; CHECK: (func $simd-replace (type $11) (param $0 v128) (param $1 i32) (result v128)
;; CHECK-NEXT: (i32x4.replace_lane 2
;; CHECK-NEXT: (local.get $0)
;; CHECK-NEXT: (local.get $1)
Expand Down Expand Up @@ -2086,7 +2086,7 @@
v128.bitselect
)

;; CHECK: (func $simd-shift (type $10) (param $0 v128) (param $1 i32) (result v128)
;; CHECK: (func $simd-shift (type $11) (param $0 v128) (param $1 i32) (result v128)
;; CHECK-NEXT: (i8x16.shl
;; CHECK-NEXT: (local.get $0)
;; CHECK-NEXT: (local.get $1)
Expand Down Expand Up @@ -2141,7 +2141,7 @@
v128.store64_lane 3 align=4 0
)

;; CHECK: (func $memory-init (type $9) (param $0 i32) (param $1 i32) (param $2 i32)
;; CHECK: (func $memory-init (type $10) (param $0 i32) (param $1 i32) (param $2 i32)
;; CHECK-NEXT: (memory.init $mem-i32 $passive
;; CHECK-NEXT: (local.get $0)
;; CHECK-NEXT: (local.get $1)
Expand Down Expand Up @@ -2376,7 +2376,7 @@
drop
)

;; CHECK: (func $ref-test (type $11) (param $0 anyref)
;; CHECK: (func $ref-test (type $8) (param $0 anyref)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (ref.test i31ref
;; CHECK-NEXT: (local.get $0)
Expand All @@ -2397,7 +2397,7 @@
drop
)

;; CHECK: (func $ref-cast (type $11) (param $0 anyref)
;; CHECK: (func $ref-cast (type $8) (param $0 anyref)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (ref.cast i31ref
;; CHECK-NEXT: (local.get $0)
Expand All @@ -2418,6 +2418,90 @@
drop
)

;; CHECK: (func $br-on-null (type $8) (param $0 anyref)
;; CHECK-NEXT: (block $label
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (br_on_null $label
;; CHECK-NEXT: (local.get $0)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $br-on-null (param anyref)
local.get 0
br_on_null 0
drop
)

;; CHECK: (func $br-on-non-null (type $8) (param $0 anyref)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block $label (result (ref any))
;; CHECK-NEXT: (br_on_non_null $label
;; CHECK-NEXT: (local.get $0)
;; CHECK-NEXT: )
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $br-on-non-null (param anyref)
block (result (ref any))
local.get 0
br_on_non_null 0
unreachable
end
drop
)

;; CHECK: (func $br-on-cast (type $8) (param $0 anyref)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block $label (result i31ref)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result (ref any))
;; CHECK-NEXT: (br_on_cast $label anyref i31ref
;; CHECK-NEXT: (local.get $0)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $br-on-cast (param anyref)
block (result i31ref)
block (result (ref any))
local.get 0
br_on_cast 1 i31ref
end
unreachable
end
drop
)

;; CHECK: (func $br-on-cast-fail (type $8) (param $0 anyref)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block $label (result (ref any))
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result i31ref)
;; CHECK-NEXT: (br_on_cast_fail $label anyref i31ref
;; CHECK-NEXT: (local.get $0)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $br-on-cast-fail (param anyref)
block (result (ref any))
block (result i31ref)
local.get 0
br_on_cast_fail 1 i31ref
end
unreachable
end
drop
)

;; CHECK: (func $struct-new (type $34) (param $0 i32) (param $1 i64) (result (ref $pair))
;; CHECK-NEXT: (struct.new $pair
;; CHECK-NEXT: (local.get $0)
Expand Down