-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
WASM text format improved integration #32917
base: main
Are you sure you want to change the base?
Conversation
@@ -6,12 +6,18 @@ page-type: landing-page | |||
|
|||
{{WebAssemblySidebar}} | |||
|
|||
This page lists all the documented WebAssembly text-format instructions. | |||
|
|||
{{ListSubPages("","3")}} |
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.
As there is no sidebar there is no other way to get a complete listing of what is documented. I'd suggest maybe reverting to the old way list or a macro if we can get the sidebar.
@@ -1,14 +1,14 @@ | |||
--- | |||
title: Copy | |||
slug: WebAssembly/Reference/Memory/Copy | |||
title: memory.copy |
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 my original thinking here was that the actual instruction is memory.copy
- not copy
- so having the full instruction is useful.
However in other cases, such as store
the instruction depends on the type of data you want to store - so you would have i32.store
or f64.store
(note, we documtened store
but actually there are effectively variants..
So the questions are:
- Do we prefix none of them - i.e.
copy
andstore
have no prefix in the name- In this case we might have to move the variables under global and local so we can document them with an inferred type.
- Do we prefix all of them with the affected type, such as
memory
- even though this is not how they are called. Somemory.copy
,memory.store
- Do we prefix them but with some "proxy" for the type. So we'd use
memory.copy
butnumber.store
?
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.
I wouldn't do 2.
Between the others I'm not sure, both seem good to me.
@@ -1,14 +1,14 @@ | |||
--- | |||
title: Copy | |||
slug: WebAssembly/Reference/Memory/Copy | |||
title: memory.copy |
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.
At the moment title is just a name of the function, but should we have better naming to give context here - such as we do in API refer. For example instead of
title: memory.copy | |
title: memory.copy: WASM text instruction |
And if not that, what would be good?
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.
Yes, great idea!
page-type: webassembly-instruction | ||
--- | ||
|
||
{{WebAssemblySidebar}} | ||
|
||
The **`store`** instructions, are used to store a number in memory. | ||
The **`store`** [memory](/en-US/docs/WebAssembly/Reference/Memory) instruction is used to store a number in memory. |
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 I was also thinking of this pattern with a link to the parent type of instruction in the body - again, mostly for better cross linking. Thoughts?
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 call!
|
||
For the integer numbers, you can also store a wide typed number as a narrower number in memory, e.g. store a 32-bit number in an 8-bit slot (**`i32.store8`**). If the number doesn't fit in the narrower number type it will wrap. | ||
There are variants of the instruction, such as `i32.store8`, `i32.store16`, and so on (see below), for storing a wide typed integer number as a narrower number in memory. |
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.
The variants are all listed below in the syntax section, for all cases that take a number type. Should we list the supported number types in the description as done here for ALL items?
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.
Why would you list all types here but not in other pages? E.g. add
page-type: webassembly-instruction | ||
--- | ||
|
||
{{WebAssemblySidebar}} | ||
|
||
The **`abs`** instructions, short for _absolute_, are used to get the absolute value of a number. That is, it returns x if x is positive, and the negation of x if x is negative. | ||
The **`abs`** [numeric](/en-US/docs/WebAssembly/Reference/Numeric) WebAssembly text-format instruction, short for _absolute_, is used to get the absolute value of an `f32` or `f64` number on the stack. That is, it returns x if x is positive, and the negation of x if x is negative. |
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.
Similarly
- should it be text-format instructions, or Web Assembly text-format instructions, or just instructions?
- Should we simplify the "short for" to a bracketed expansion.
My preference is that we use a pattern like this:
The **`abs`** [numeric](/en-US/docs/WebAssembly/Reference/Numeric) WebAssembly text-format instruction, short for _absolute_, is used to get the absolute value of an `f32` or `f64` number on the stack. That is, it returns x if x is positive, and the negation of x if x is negative. | |
The **`abs`** (absolute) [numeric instruction](/en-US/docs/WebAssembly/Reference/Numeric), is used to get the absolute value of an `f32` or `f64` number on the stack. That is, it returns x if x is positive, and the negation of x if x is negative. |
@wbamberg Can you offer advice on adding the WebAssembly instruction reference to the WebAssemblySidebar. The simplest approach would be to just append a link to above doc (and perhaps the ones below):
But what I'd like to do is expand out the sub tree and include that . Is something like this using ListSubpagesForSidebar likely to work (note, I'd call this several times to get the full listing, for each class of instruction):
Or would be be better listing all of them with no sorting, and somehow matching on the page type? If so, can you point me to the sidebar you would copy? |
Something like this: https://github.com/mdn/yari/blob/1ffb0261196752308a48b9f87d32101427d82430/kumascript/macros/HTMLSidebar.ejs#L363-L375 should work, if I understand your use case. It would probably be good to have a section for each instruction type, using
|
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.
Thanks @hamishwillee for showing some love for these pages! I think the changes you made will make them much more readable!
page-type: webassembly-instruction | ||
--- | ||
|
||
{{WebAssemblySidebar}} | ||
|
||
The **`load`** instructions, are used to load a number from memory onto the stack. | ||
The **`load`** [memory](/en-US/docs/WebAssembly/Reference/Memory) instruction is used to load a number from memory onto the stack. |
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.
Why the change to singular? Since there are multiple load instructions (see table below)
@@ -1,14 +1,14 @@ | |||
--- | |||
title: Copy | |||
slug: WebAssembly/Reference/Memory/Copy | |||
title: memory.copy |
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.
Yes, great idea!
@@ -1,14 +1,14 @@ | |||
--- | |||
title: Copy | |||
slug: WebAssembly/Reference/Memory/Copy | |||
title: memory.copy |
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.
I wouldn't do 2.
Between the others I'm not sure, both seem good to me.
page-type: webassembly-instruction | ||
--- | ||
|
||
{{WebAssemblySidebar}} | ||
|
||
The **`size`** instruction, returns the amount of pages the memory instance currently has, each page is sized 64KiB. | ||
The **`memory.size`** instruction, returns the size of the memory instance, in 64KiB pages. |
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.
I believe that there are active discussions about other memory sizes, might be worth keeping the "currently"
page-type: webassembly-instruction | ||
--- | ||
|
||
{{WebAssemblySidebar}} | ||
|
||
The **`store`** instructions, are used to store a number in memory. | ||
The **`store`** [memory](/en-US/docs/WebAssembly/Reference/Memory) instruction is used to store a number in memory. |
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 call!
|
||
For the integer numbers, you can also store a wide typed number as a narrower number in memory, e.g. store a 32-bit number in an 8-bit slot (**`i32.store8`**). If the number doesn't fit in the narrower number type it will wrap. | ||
There are variants of the instruction, such as `i32.store8`, `i32.store16`, and so on (see below), for storing a wide typed integer number as a narrower number in memory. |
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.
Why would you list all types here but not in other pages? E.g. add
This pull request has merge conflicts that must be resolved before it can be merged. |
The WASM text format is partially documented in WebAssembly instruction reference but you wouldn't know that if you didn't already know about it. The content incomplete, but IMO it is still very useful and we are foolish not to include it.
This attempts to work on some better cross linking, and serve for discussion on how we might expand this out in future, based on our learnings from other parts of MDN.
Specifically:
add
not "Addition".This is a precursor to work I am doing for #32777 because without understanding the text format it is hard to use or understand when you might use the new multi memory features.
ON HOLD. I have done some of this in #32919. Plan to come back to this. It's in the queue.