Skip to content

[spec] Use typeuse syntax for call_indirect#599

Merged
rossberg merged 1 commit intomasterfrom
call-sig
Nov 9, 2017
Merged

[spec] Use typeuse syntax for call_indirect#599
rossberg merged 1 commit intomasterfrom
call-sig

Conversation

@rossberg
Copy link
Member

@rossberg rossberg commented Nov 7, 2017

This is a late but small change/extension to the text format that I floated with some people.

Instead of a simple type index, the call signature for call_indirect is specified using the existing typeuse syntax used e.g. in function declarations:

call_indirect (type $t)
call_indirect (param i32 i32)
call_indirect (param i32 i64) (result i32)

That makes it consistent with how types are/will be referenced elsewhere (besides function declarations, e.g. in block signatures in the multi-value proposal). Furthermore, as the example shows, it implies that you can write the call signature inline, like for functions and blocks.

The expansion rules are like for function declarations. However, inline types for call_indirect must not include parameter names.

@rossberg rossberg requested review from binji and lukewagner November 7, 2017 16:59
Copy link
Member

@lukewagner lukewagner left a comment

Choose a reason for hiding this comment

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

Much more convenient to write/read the signature inline, so I like this change.

@rossberg
Copy link
Member Author

rossberg commented Nov 9, 2017

Merging based on above approval and off-line feedback.

@rossberg rossberg merged commit c247b6a into master Nov 9, 2017
@rossberg rossberg deleted the call-sig branch November 9, 2017 14:13
rossberg added a commit that referenced this pull request Nov 9, 2017
Copy link
Member

@binji binji left a comment

Choose a reason for hiding this comment

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

lgtm too.

rossberg added a commit that referenced this pull request Nov 10, 2017
Implement #599 in interpreter and adapt & add tests.
dschuff added a commit to WebAssembly/binaryen that referenced this pull request Nov 11, 2017
Function type gets its own element rather than being a part of the call_indirect
(see WebAssembly/spec#599)
dschuff added a commit to WebAssembly/binaryen that referenced this pull request Nov 13, 2017
Function type gets its own element rather than being a part of the call_indirect
(see WebAssembly/spec#599)
@cretz
Copy link

cretz commented Mar 2, 2018

I have a request for clarification here as I'm just getting around to implementing this. On

(call_indirect (param i64) (i64.const 0) (i32.const 0))
I am not seeing an existing function type this is referencing. According to the spec, "If no such index exists, then a new type definition of the form [...] is inserted at the end of the module."

So, just to confirm, the module-level function type def list can be altered while parsing instructions? I can ask this in a new issue or in another place if this issue isn't where it should happen.

@binji
Copy link
Member

binji commented Mar 2, 2018

So, just to confirm, the module-level function type def list can be altered while parsing instructions?

Yes, the behavior is the same as defining a function without an explicit function type.

@cretz
Copy link

cretz commented Mar 2, 2018

Thanks @binji, and just one more confirmation since order is important, if the type for the func that contains this call_indirect insn of an unforeseen type has also never been seen before, the outer func's type is added to the def list before the call_indirect ones within, right? (sorry, I hope that was clear)

@binji
Copy link
Member

binji commented Mar 2, 2018

Not sure if I quite understand, but here's an example:

(module
  (type (func (param f64)))
  (import "spectest" "table" (table 10 anyfunc))
  (func (param i32)
    (call_indirect (param f32) (f32.const 1) (get_local 0))))

The type section should have:

type 0: [f64] -> []
type 1: [i32] -> []
type 2: [f32] -> []

@rossberg
Copy link
Member Author

rossberg commented Mar 2, 2018

Yes, note the sentence at the end of the section, which states that "abbreviations are expanded in the order they appear".

@cretz
Copy link

cretz commented Mar 2, 2018

Perfect, thanks! It's just taking me a bit of extra work because not only did I never have the type def list passed in as contextual state to my insn parser, but I hadn't needed any module-level mutable state in there until now (granted I could just return a bunch of immutable things from every recursive insn-parse call and shore up, but same annoyance applies). I also didn't add the function type to the module-level list until after I had parsed the whole function w/ insns hence the most recent question. Thanks again.

spoonincode pushed a commit to EOSIO/binaryen that referenced this pull request May 4, 2018
)

Function type gets its own element rather than being a part of the call_indirect
(see WebAssembly/spec#599)

EOSIO note: Only cherry-pick the .cpp changes for this. Trying to pull up the tests is not feasible. We don't use the tests anyways.
raoxiaojia pushed a commit to WasmCert/spec that referenced this pull request Apr 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants