-
Notifications
You must be signed in to change notification settings - Fork 698
Future-proof type opcodes #823
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.
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
Conversation
titzer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change generally LGTM, but I'm not sure we'll be able to swing this due to time constraints.
BinaryEncoding.md
Outdated
| ### `value_type` | ||
| A single-byte unsigned integer indicating a [value type](Semantics.md#types). These types are encoded as: | ||
| A `varint7` indicating a [value type](Semantics.md#types). These types are encoded as: | ||
| * `-0x01` indicating type `i32` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this is byte value 0x7f?
|
Online and offline comments addressed. Fixed out-of-bounds values. |
BinaryEncoding.md
Outdated
|
|
||
| | Field | Type | Description | | ||
| | ----- | ---- | ----------- | | ||
| | form | `varint32` | `-0x20` (i.e., the byte `0x60`) indicating a function type | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So in this negative constructor opcode space, we have -1, -2, -3, -4, -0x10, -0x20, -0x40. Do you think it makes sense to hoist these out into a new type_constructor subsection so that we can see them all in one list without having to know that these 7 values are all implicitly in the same index space? Then each of the 3 uses could specify which subset are valid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, done.
BinaryEncoding.md
Outdated
|
|
||
| ### `block_type` | ||
| A `varint7` indicating a signature. These types are encoded as: | ||
| * `-0x40` (i.e., the byte `0x40`) indicating a signature with 0 results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to bike-shed here, but do we want to start at -0x40, and then -0x3F for function and -0x3E for anyfunc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, by starting with the lowest absolute value we keep an open-ended gap at the "end" for future extension. The inverse order would become rather weird if we were to grow out of the 6 bit negative range of a varint7, because then we'd have to wrap around and add additional types or categories "before" the basic types.
|
On Thu, Oct 20, 2016 at 4:44 PM, rossberg-chromium <[email protected]
|
|
lgtm; no strong opinion on @titzer's suggestion to pack 'em closer |
|
@titzer, the idea with the gaps is the same as for our instruction opcodes: there are different categories of types (currently, value types, element types, defined types), and it 's nice to leave space to extend each category. |
|
lgtm |
|
Alrighty On Thu, Oct 20, 2016 at 6:27 PM, rossberg-chromium <[email protected]
|
Post MVP we will likely extend Wasm with a few richer types, e.g., function types for blocks, struct types, or function pointers. That introduces a number of places where types will be given either directly or via reference to the type section.
To make Wasm future-proof for efficiently overlaying both these spaces, this PR changes the existing type constructors to have negative opcodes, such that they are distinguishable from indexed type definitions (which are naturally positive).
For example, this will in the future allow
The PR also cleans up the description of type encodings.