Skip to content

[Parser] Parse ref.func #6097

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
Nov 15, 2023
Merged

[Parser] Parse ref.func #6097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
db3f575
[Parser] Parse ref.func
tlively Nov 9, 2023
7f818b5
Add comment that items must exist
tlively Nov 15, 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
7 changes: 5 additions & 2 deletions src/parser/contexts.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,9 +388,8 @@ struct NullInstrParserCtx {
return Ok{};
}
Result<> makeRefIsNull(Index) { return Ok{}; }

Result<> makeRefFunc(Index, FuncIdxT) { return Ok{}; }
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's the NullInstrParserCtx for?

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 first few phases of parsing only care about finding top-level module elements like globals, functions, etc, or parsing types, so they don't need to do anything interesting with the instructions. The parser contexts for those phases inherit from NullInstrParserCtx to avoid having to repeat this "do nothing" behavior for each of them separately.

Result<> makeRefEq(Index) { return Ok{}; }

Result<> makeRefI31(Index) { return Ok{}; }
Result<> makeI31Get(Index, bool) { return Ok{}; }

Expand Down Expand Up @@ -1259,6 +1258,10 @@ struct ParseDefsCtx : TypeParserCtx<ParseDefsCtx> {
return withLoc(pos, irBuilder.makeRefIsNull());
}

Result<> makeRefFunc(Index pos, Name func) {
return withLoc(pos, irBuilder.makeRefFunc(func));
}

Result<> makeRefEq(Index pos) { return withLoc(pos, irBuilder.makeRefEq()); }

Result<> makeRefI31(Index pos) {
Expand Down
4 changes: 3 additions & 1 deletion src/parser/parsers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1218,7 +1218,9 @@ template<typename Ctx> Result<> makeRefIsNull(Ctx& ctx, Index pos) {
}

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

template<typename Ctx> Result<> makeRefEq(Ctx& ctx, Index pos) {
Expand Down
5 changes: 4 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 @@ -35,6 +35,9 @@ namespace wasm {
//
// To use, call CHECK_ERR(visit(...)) or CHECK_ERR(makeXYZ(...)) on each
// expression in the sequence, then call build().
//
// Unlike `Builder`, `IRBuilder` requires referenced module-level items (e.g.
// globals, tables, functions, etc.) to already exist in the module.
class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
public:
IRBuilder(Module& wasm, Function* func = nullptr)
Expand Down Expand Up @@ -129,7 +132,7 @@ class IRBuilder : public UnifiedExpressionVisitor<IRBuilder, Result<>> {
// [[nodiscard]] Result<> makePop();
[[nodiscard]] Result<> makeRefNull(HeapType type);
[[nodiscard]] Result<> makeRefIsNull();
// [[nodiscard]] Result<> makeRefFunc();
[[nodiscard]] Result<> makeRefFunc(Name func);
[[nodiscard]] Result<> makeRefEq();
// [[nodiscard]] Result<> makeTableGet();
// [[nodiscard]] Result<> makeTableSet();
Expand Down
5 changes: 4 additions & 1 deletion src/wasm/wasm-ir-builder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,10 @@ Result<> IRBuilder::makeRefIsNull() {
return Ok{};
}

// Result<> IRBuilder::makeRefFunc() {}
Result<> IRBuilder::makeRefFunc(Name func) {
push(builder.makeRefFunc(func, wasm.getFunction(func)->type));
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.

Consider adding a comment about wasm-ir-builder having a stricter makeRefFunc() than wasm-builder, one that will throw an exception if the function being made reference to is not already added to the module.

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.

I'll add a general comment about this to the top-level IRBuilder doc comment because this applies to many functions.

return Ok{};
}

Result<> IRBuilder::makeRefEq() {
RefEq curr;
Expand Down
17 changes: 17 additions & 0 deletions test/lit/wat-kitchen-sink.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@

;; CHECK: (data $1 (memory $mem-i64) (i64.const 0) "64-bit")

;; CHECK: (elem declare func $ref-func)

;; CHECK: (export "g1" (global $g1))

;; CHECK: (export "g1.1" (global $g1))
Expand Down Expand Up @@ -2205,6 +2207,21 @@
ref.is_null
)

;; CHECK: (func $ref-func (type $void)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (ref.func $ref-func)
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (ref.func $ref-func)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $ref-func
ref.func $ref-func
drop
ref.func 102
Copy link
Collaborator

@ashleynh ashleynh Nov 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

How did you know $ref-func was at function index 102?

Copy link
Member Author

@tlively tlively Nov 13, 2023

Choose a reason for hiding this comment

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

trial and error ;)

drop
)

;; CHECK: (func $ref-eq (type $28) (param $0 eqref) (param $1 eqref) (result i32)
;; CHECK-NEXT: (ref.eq
;; CHECK-NEXT: (local.get $0)
Expand Down