-
Notifications
You must be signed in to change notification settings - Fork 55
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
Spec basic instructions #170
Conversation
Unfortunately, it looks like MathJax is messing up the notation pretty badly. I tried a few quick hacks, but to no avail. Leaving that problem for later. |
a4e0fc2
to
4f03457
Compare
@conrad-watt, PTAL. |
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.
Main issue currently is the behaviour of cmpxchg
.
After this PR, I believe the next steps are to add fence
and flesh out the semantics of wait/notify
?
@@ -215,70 +215,70 @@ Instruction Binary Opcode | |||
:math:`\I64.\TRUNC\K{\_sat}\_\F64\K{\_s}` :math:`\hex{FC}~~6` :math:`[\F64] \to [\I64]` :ref:`validation <valid-cvtop>` :ref:`execution <exec-cvtop>`, :ref:`operator <op-trunc_sat_s>` | |||
:math:`\I64.\TRUNC\K{\_sat\_}\F64\K{\_u}` :math:`\hex{FC}~~7` :math:`[\F64] \to [\I64]` :ref:`validation <valid-cvtop>` :ref:`execution <exec-cvtop>`, :ref:`operator <op-trunc_sat_u>` | |||
(reserved) :math:`\hex{FD}` | |||
:math:`\MEMORYATOMICNOTIFY~\memarg` :math:`\hex{FE}~\hex{00}` :math:`[\I32~\I64] \to [\I64]` :ref:`validation <valid-atomic-notify>` |
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.
When I try to build this page, I get
<path>/index-instructions.rst:
WARNING: Malformed table. Text in column margin in table line 210.
It shows up as blank in the built web document.
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.
Oops, forgot to rerun the generator script (this ought to be automatic once we rebase on the main spec repo).
document/core/exec/runtime.rst
Outdated
@@ -174,7 +174,7 @@ Syntactically, a local store is defined as a :ref:`map <notation-map>` from know | |||
\globalinst \\ | |||
\end{array} | |||
|
|||
It is an invariant of the semantics that none of the memory instances :math:`\meminst` in a store is :math:`shared <syntax-shared>`. | |||
It is an invariant of the semantics that none of the memory instances :math:`\meminst` in a store is :ref:`shared <syntax-shared>`. |
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.
maybe clarify "none of the memory instances in a local 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.
Done.
@@ -780,7 +780,7 @@ The following structural rule for global reduction delegates to local reduction | |||
\begin{array}[t]{@{}r@{~}l@{}} |
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.
A few lines up (missed earlier) "Formally, glboal reduction is a relation"
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.
Fixed.
@@ -431,228 +431,212 @@ Memory Instructions | |||
Atomic Memory Instructions | |||
~~~~~~~~~~~~~~~~~~~~~~~~~~ | |||
|
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.
Maybe add an explanatory note that atomic ops don't need to be on shared 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.
Hm, decided not to, since it should be pretty clear from the rules and the spec generally avoids design discussions beyond clarifications.
document/core/valid/instructions.rst
Outdated
}{ | ||
C \vdash t\K{.}\ATOMICRMW\K{.}\ATOMICCMPXCHG~\memarg : [\I32~t~t] \to [t] | ||
C \vdash \MEMORYATOMICNOTIFY~\memarg : [\I32~\I64] \to [\I64] |
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 should have caught this earlier, but I'm almost certain that the type of notify
should be i32 i32 -> i32
. This is what's specified in the overvew. I checked that Binaryen also expects this.
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, the current reduction rules also match that, fixed.
However, the reduction rule for wait seem to have the operand order reversed relative to both validation and overview. Added a todo.
document/core/exec/instructions.rst
Outdated
|
||
b. Let :math:`b^\ast_w` be the byte sequence :math:`\bytes_{\iN}(n)`. | ||
c. Perform the atomic :ref:`action <syntax-act>` :math:`(\ARMW_{\ord}~a.\LDATA[\X{ea}]~b_{\F{r}}^\ast~b_{\F{r}}^\ast)` to read the bytes :math:`b_{\F{r}}^\ast` from data offset :math:`\X{ea}` of the shared :ref:`memory instance <syntax-meminst>` at :ref:`memory address <syntax-memaddr>` :math:`a` and replace them with bytes :math:`b_{\F{w}}^\ast` (which are defined below). |
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 ord
subscript is never defined (should just be atomic/sc).
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.
Also, b_r*
seems to be repeated twice when the second component should be b_w*
(copy-paste nit).
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.
Both fixed.
document/core/exec/instructions.rst
Outdated
|
||
17. Let :math:`c_1` be the integer for which :math:`bytes_{\iN}(m) = b^\ast_r`. | ||
c. Perform the atomic :ref:`action <syntax-act>` :math:`(\ARMW_{\ord}~a.\LDATA[\X{ea}]~b_{\F{r}}^\ast~b_{\F{r}}^\ast)` to read the bytes :math:`b_{\F{r}}^\ast` from data offset :math:`\X{ea}` of the shared :ref:`memory instance <syntax-meminst>` at :ref:`memory address <syntax-memaddr>` :math:`a` and replace them with bytes :math:`b_{\F{w}}^\ast` (which are defined below). |
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.
rmw copy-paste nit replicated here (although as described above this section needs to change).
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.
Fixed.
|
||
:math:`t\K{.}\ATOMICRMW{.}\ATOMICCMPXCHG~\memarg` and :math:`t\K{.}\ATOMICRMW{N}\K{\_u.}\ATOMICCMPXCHG~\memarg` | ||
............................................................................................................... | ||
:math:`t\K{.}\ATOMICRMW({N}\K{\_u})^?\K{.}\ATCMPXCHG~\memarg` |
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.
High-level:
cmpxchg should only perform an rmw action if the comparison succeeds. This means that at the operational level, the instruction non-deterministically either emits an atomic read action where the observed value is != the argument, or an rmw action where the read value is == the argument. The difference is observable unfortunately (due to the total ordering on atomics) so it's not enough to write back the same value.
EDIT: also noticed that the text format doesn't seem to pop the right number of arguments?
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.
Huh, the JS spec also models compareExchange as an rmw even if the comparison fails. I'll double check, but I'm pretty sure that's not correct.
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.
C++11 models it as just a load in the failure case, so I think this is also a JS spec bug.
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.
Fixed the missing pop and added a todo for changing the semantics of the RMW (awaiting resolution?).
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.
currently under discussion tc39/ecma262#2231
document/core/exec/instructions.rst
Outdated
|
||
c. Perform the :ref:`action <syntax-act>` :math:`(\ARD_{\ord}~a.\LDATA[\X{ea}]~b^\ast)` to read the bytes :math:`b^\ast` from data offset :math:`\X{ea}` of the shared :ref:`memory instance <syntax-meminst>` at :ref:`memory address <syntax-memaddr>` :math:`a`. | ||
|
||
11. If :math:`N` and :math:`\sx` are part of the instruction, then: |
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.
can conditionals like this be factored more like the formal semantics?
i.e. have a few preamble lines that state
"1. N = |t| if N is not part of the instruction"
some of the RMWs especially have a lot of case-splitting in the text on this
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.
Done. Also extended numerics section to actually have bytes_iN and friends defined.
document/core/exec/instructions.rst
Outdated
|
||
5. Let :math:`\X{mem}` be the :ref:`memory instance <syntax-meminst>` :math:`S.\SMEMS[a]`. | ||
a. Either: |
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 "try growing memory" text in the non-shared case above was always informal language. Could some of that flavour be added to this case too? i.e.
a. Either:
memory growth succeeds.
... let bla bla bla
b. Or:
memory growth fails.
... bla bla bla
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.
Done.
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 for the thorough review! Yes, I think the next steps will be wait/notify/fence. Do you want to proceed with those?
@@ -215,70 +215,70 @@ Instruction Binary Opcode | |||
:math:`\I64.\TRUNC\K{\_sat}\_\F64\K{\_s}` :math:`\hex{FC}~~6` :math:`[\F64] \to [\I64]` :ref:`validation <valid-cvtop>` :ref:`execution <exec-cvtop>`, :ref:`operator <op-trunc_sat_s>` | |||
:math:`\I64.\TRUNC\K{\_sat\_}\F64\K{\_u}` :math:`\hex{FC}~~7` :math:`[\F64] \to [\I64]` :ref:`validation <valid-cvtop>` :ref:`execution <exec-cvtop>`, :ref:`operator <op-trunc_sat_u>` | |||
(reserved) :math:`\hex{FD}` | |||
:math:`\MEMORYATOMICNOTIFY~\memarg` :math:`\hex{FE}~\hex{00}` :math:`[\I32~\I64] \to [\I64]` :ref:`validation <valid-atomic-notify>` |
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.
Oops, forgot to rerun the generator script (this ought to be automatic once we rebase on the main spec repo).
document/core/exec/instructions.rst
Outdated
@@ -381,7 +381,7 @@ Memory Instructions | |||
~~~~~~~~~~~~~~~~~~~ | |||
|
|||
.. note:: | |||
The alignment :math:`\memarg.\ALIGN` in load and store instructions does not affect the semantics. | |||
The alignment :math:`\memarg.\ALIGN` in load and store instructions does not affect the semantics of accesses to :ref:`unshared <syntax-unshared>` :ref:`memories <syntax-mem>`. |
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, fixed.
document/core/exec/instructions.rst
Outdated
|
||
:math:`t\K{.}\ATOMIC^?.\LOAD~\memarg` and :math:`t\K{.}\ATOMIC^?.\LOAD{N}\K{\_}\sx~\memarg` | ||
........................................................................................... | ||
:math:`t\K{.}\LOAD{N}(\K{\_}\sx)^?~\memarg` |
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.
Fixed.
document/core/exec/instructions.rst
Outdated
|
||
c. Perform the :ref:`action <syntax-act>` :math:`(\ARD_{\ord}~a.\LDATA[\X{ea}]~b^\ast)` to read the bytes :math:`b^\ast` from data offset :math:`\X{ea}` of the shared :ref:`memory instance <syntax-meminst>` at :ref:`memory address <syntax-memaddr>` :math:`a`. | ||
|
||
11. If :math:`N` and :math:`\sx` are part of the instruction, then: |
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.
Done. Also extended numerics section to actually have bytes_iN and friends defined.
document/core/exec/instructions.rst
Outdated
|
||
5. Else: | ||
|
||
a. Perform the :ref:`action <syntax-act>` :math:`(\ARD~a.\LLEN~n)` to read the length :math:`n` of the shared :ref:`memory instance <syntax-meminst>` at :ref:`memory address <syntax-memaddr>` :math:`a`. |
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.
Fixed.
|
||
:math:`t\K{.}\ATOMICRMW{.}\ATOMICCMPXCHG~\memarg` and :math:`t\K{.}\ATOMICRMW{N}\K{\_u.}\ATOMICCMPXCHG~\memarg` | ||
............................................................................................................... | ||
:math:`t\K{.}\ATOMICRMW({N}\K{\_u})^?\K{.}\ATCMPXCHG~\memarg` |
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.
Fixed the missing pop and added a todo for changing the semantics of the RMW (awaiting resolution?).
document/core/exec/runtime.rst
Outdated
@@ -174,7 +174,7 @@ Syntactically, a local store is defined as a :ref:`map <notation-map>` from know | |||
\globalinst \\ | |||
\end{array} | |||
|
|||
It is an invariant of the semantics that none of the memory instances :math:`\meminst` in a store is :math:`shared <syntax-shared>`. | |||
It is an invariant of the semantics that none of the memory instances :math:`\meminst` in a store is :ref:`shared <syntax-shared>`. |
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.
Done.
@@ -780,7 +780,7 @@ The following structural rule for global reduction delegates to local reduction | |||
\begin{array}[t]{@{}r@{~}l@{}} |
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.
Fixed.
@@ -431,228 +431,212 @@ Memory Instructions | |||
Atomic Memory Instructions | |||
~~~~~~~~~~~~~~~~~~~~~~~~~~ | |||
|
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, decided not to, since it should be pretty clear from the rules and the spec generally avoids design discussions beyond clarifications.
document/core/valid/instructions.rst
Outdated
}{ | ||
C \vdash t\K{.}\ATOMICRMW\K{.}\ATOMICCMPXCHG~\memarg : [\I32~t~t] \to [t] | ||
C \vdash \MEMORYATOMICNOTIFY~\memarg : [\I32~\I64] \to [\I64] |
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, the current reduction rules also match that, fixed.
However, the reduction rule for wait seem to have the operand order reversed relative to both validation and overview. Added a todo.
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.
Just a few nits.
I can look at fence/wait/notify in the next couple of weeks, but I really need to submit my thesis so I'm finding it hard to manage my time.
:math:`\I64.\TRUNC\K{\_sat}\_\F64\K{\_s}` :math:`\hex{FC}~~6` :math:`[\F64] \to [\I64]` :ref:`validation <valid-cvtop>` :ref:`execution <exec-cvtop>`, :ref:`operator <op-trunc_sat_s>` | ||
:math:`\I64.\TRUNC\K{\_sat\_}\F64\K{\_u}` :math:`\hex{FC}~~7` :math:`[\F64] \to [\I64]` :ref:`validation <valid-cvtop>` :ref:`execution <exec-cvtop>`, :ref:`operator <op-trunc_sat_u>` | ||
(reserved) :math:`\hex{FD}` | ||
:math:`\MEMORYATOMICNOTIFY~\memarg` :math:`\hex{FE}~\hex{00}` :math:`[\I32~\I64] \to [\I64]` :ref:`validation <valid-memory.atomic.notify>` |
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.
Notify still has the incorrect type here.
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.
Fixed.
document/core/exec/instructions.rst
Outdated
|
||
2. Assert: due to :ref:`validation <valid-loadn>`, :math:`F.\AMODULE.\MIMEMS[0]` exists. | ||
a. Let :math:`\X{st}` be the :ref:`pro forma type <aux-bytes>` :math:`\iN`. |
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.
Is the <aux-bytes>
link association correct? I'd have expected this to link to the definition of storage types in numerics but it seems to jump me to a different section.
Also, it might be clearer to call this a "storage type" rather than a "pro forma 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.
Updated.
13. Else: | ||
11. Else: | ||
|
||
a. Perform the :ref:`action <syntax-act>` :math:`(\ARD~a.\LLEN~n)` to read the length :math:`n` of the shared :ref:`memory instance <syntax-meminst>` at :ref:`memory address <syntax-memaddr>` :math:`a`. |
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.
Need to make sure that the length of b*
matches the width of the load (same in formal semantics).
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, this comment should have been at line 439 just below
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.
Made explicit in the prose. In the formal semantics that's implicit by the side conditions, particularly, its use as an argument to bytes^-1.
document/core/exec/instructions.rst
Outdated
|
||
2. Assert: due to :ref:`validation <valid-storen>`, :math:`F.\AMODULE.\MIMEMS[0]` exists. | ||
a. Let :math:`\X{st}` be the :ref:`pro forma type <aux-bytes>` :math:`\iN`. |
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.
same as above
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.
Fixed.
document/core/exec/instructions.rst
Outdated
|
||
2. Assert: due to :ref:`validation <valid-atomic-rmwn>`, :math:`F.\AMODULE.\MIMEMS[0]` exists. | ||
a. Let :math:`\X{st}` be the :ref:`pro forma type <aux-bytes>` :math:`\iN`. |
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.
same here
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.
Fixed.
document/core/exec/instructions.rst
Outdated
|
||
19. Replace the bytes :math:`\X{mem}.\MIDATA[\X{ea} \slice N/8]` with :math:`b^\ast_w`. | ||
c. Perform the atomic :ref:`action <syntax-act>` :math:`(\ARMW_{\SEQCST}~a.\LDATA[\X{ea}]~b_{\F{r}}^\ast~b_{\F{w}}^\ast)` to read the bytes :math:`b_{\F{r}}^\ast` from data offset :math:`\X{ea}` of the shared :ref:`memory instance <syntax-meminst>` at :ref:`memory address <syntax-memaddr>` :math:`a` and replace them with bytes :math:`b_{\F{w}}^\ast` (which are defined below). |
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.
again, need to ensure that b_r*
has the right length (does length of b_w*
follow as numeric invariant?)
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.
Fixed as above. (Yes, for b*_w this follows from the definition of bytes_st.)
document/core/exec/instructions.rst
Outdated
\wedge & \X{ea} + N/8 \leq n \\ | ||
\wedge & \X{ea} \mod N/8 = 0 \\ | ||
\wedge & c_1 = \extendtu_{\X{st},t}(\bytes^{-1}_{\X{st}}(b_{\F{r}}^\ast)) \\ | ||
\wedge & b_{\F{r}}^\ast = \bytes_{\X{st}}(\wrapt_{t,\X{st}}(\atop_t(c_1, c_2)))) \\ |
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.
think this should be b_w*
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.
Fixed.
|
||
:math:`t\K{.}\ATOMICRMW{.}\ATOMICCMPXCHG~\memarg` and :math:`t\K{.}\ATOMICRMW{N}\K{\_u.}\ATOMICCMPXCHG~\memarg` | ||
............................................................................................................... | ||
:math:`t\K{.}\ATOMICRMW({N}\K{\_u})^?\K{.}\ATCMPXCHG~\memarg` |
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.
currently under discussion tc39/ecma262#2231
document/core/exec/instructions.rst
Outdated
|
||
2. Assert: due to :ref:`validation <valid-atomic-rmwn-cmpxchg>`, :math:`F.\AMODULE.\MIMEMS[0]` exists. | ||
a. Let :math:`\X{st}` be the :ref:`pro forma type <aux-bytes>` :math:`\iN`. |
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.
same as above
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.
Fixed.
document/core/exec/instructions.rst
Outdated
|
||
i. Trap. | ||
|
||
c. Perform the atomic :ref:`action <syntax-act>` :math:`(\ARMW_{\SEQCST}~a.\LDATA[\X{ea}]~b_{\F{r}}^\ast~b_{\F{w}}^\ast)` to read the bytes :math:`b_{\F{r}}^\ast` from data offset :math:`\X{ea}` of the shared :ref:`memory instance <syntax-meminst>` at :ref:`memory address <syntax-memaddr>` :math:`a` and replace them with bytes :math:`b_{\F{w}}^\ast` (which are defined below). |
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.
same as above
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.
Fixed.
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! No stress, I thought your thesis was already due in September.
:math:`\I64.\TRUNC\K{\_sat}\_\F64\K{\_s}` :math:`\hex{FC}~~6` :math:`[\F64] \to [\I64]` :ref:`validation <valid-cvtop>` :ref:`execution <exec-cvtop>`, :ref:`operator <op-trunc_sat_s>` | ||
:math:`\I64.\TRUNC\K{\_sat\_}\F64\K{\_u}` :math:`\hex{FC}~~7` :math:`[\F64] \to [\I64]` :ref:`validation <valid-cvtop>` :ref:`execution <exec-cvtop>`, :ref:`operator <op-trunc_sat_u>` | ||
(reserved) :math:`\hex{FD}` | ||
:math:`\MEMORYATOMICNOTIFY~\memarg` :math:`\hex{FE}~\hex{00}` :math:`[\I32~\I64] \to [\I64]` :ref:`validation <valid-memory.atomic.notify>` |
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.
Fixed.
document/core/exec/instructions.rst
Outdated
|
||
2. Assert: due to :ref:`validation <valid-loadn>`, :math:`F.\AMODULE.\MIMEMS[0]` exists. | ||
a. Let :math:`\X{st}` be the :ref:`pro forma type <aux-bytes>` :math:`\iN`. |
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.
Updated.
13. Else: | ||
11. Else: | ||
|
||
a. Perform the :ref:`action <syntax-act>` :math:`(\ARD~a.\LLEN~n)` to read the length :math:`n` of the shared :ref:`memory instance <syntax-meminst>` at :ref:`memory address <syntax-memaddr>` :math:`a`. |
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.
Made explicit in the prose. In the formal semantics that's implicit by the side conditions, particularly, its use as an argument to bytes^-1.
document/core/exec/instructions.rst
Outdated
|
||
2. Assert: due to :ref:`validation <valid-storen>`, :math:`F.\AMODULE.\MIMEMS[0]` exists. | ||
a. Let :math:`\X{st}` be the :ref:`pro forma type <aux-bytes>` :math:`\iN`. |
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.
Fixed.
document/core/exec/instructions.rst
Outdated
|
||
2. Assert: due to :ref:`validation <valid-atomic-rmwn>`, :math:`F.\AMODULE.\MIMEMS[0]` exists. | ||
a. Let :math:`\X{st}` be the :ref:`pro forma type <aux-bytes>` :math:`\iN`. |
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.
Fixed.
document/core/exec/instructions.rst
Outdated
|
||
19. Replace the bytes :math:`\X{mem}.\MIDATA[\X{ea} \slice N/8]` with :math:`b^\ast_w`. | ||
c. Perform the atomic :ref:`action <syntax-act>` :math:`(\ARMW_{\SEQCST}~a.\LDATA[\X{ea}]~b_{\F{r}}^\ast~b_{\F{w}}^\ast)` to read the bytes :math:`b_{\F{r}}^\ast` from data offset :math:`\X{ea}` of the shared :ref:`memory instance <syntax-meminst>` at :ref:`memory address <syntax-memaddr>` :math:`a` and replace them with bytes :math:`b_{\F{w}}^\ast` (which are defined below). |
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.
Fixed as above. (Yes, for b*_w this follows from the definition of bytes_st.)
document/core/exec/instructions.rst
Outdated
\wedge & \X{ea} + N/8 \leq n \\ | ||
\wedge & \X{ea} \mod N/8 = 0 \\ | ||
\wedge & c_1 = \extendtu_{\X{st},t}(\bytes^{-1}_{\X{st}}(b_{\F{r}}^\ast)) \\ | ||
\wedge & b_{\F{r}}^\ast = \bytes_{\X{st}}(\wrapt_{t,\X{st}}(\atop_t(c_1, c_2)))) \\ |
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.
Fixed.
document/core/exec/instructions.rst
Outdated
|
||
2. Assert: due to :ref:`validation <valid-atomic-rmwn-cmpxchg>`, :math:`F.\AMODULE.\MIMEMS[0]` exists. | ||
a. Let :math:`\X{st}` be the :ref:`pro forma type <aux-bytes>` :math:`\iN`. |
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.
Fixed.
document/core/exec/instructions.rst
Outdated
|
||
i. Trap. | ||
|
||
c. Perform the atomic :ref:`action <syntax-act>` :math:`(\ARMW_{\SEQCST}~a.\LDATA[\X{ea}]~b_{\F{r}}^\ast~b_{\F{w}}^\ast)` to read the bytes :math:`b_{\F{r}}^\ast` from data offset :math:`\X{ea}` of the shared :ref:`memory instance <syntax-meminst>` at :ref:`memory address <syntax-memaddr>` :math:`a` and replace them with bytes :math:`b_{\F{w}}^\ast` (which are defined below). |
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.
Fixed.
yes |
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.
LGTM
This updates the spec for load, store, rmw, memory.size & .grow instructions.