Skip to content
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

[interpreter] implement atomic.wait and atomic.notify #194

Merged
merged 6 commits into from
May 30, 2023

Conversation

conrad-watt
Copy link
Collaborator

also split out relevant tests into a separate file

@conrad-watt conrad-watt changed the title implement atomic.wait and atomic.notify [interpreter] implement atomic.wait and atomic.notify Oct 19, 2022
@rossberg
Copy link
Member

rossberg commented Oct 24, 2022

I'm somewhat confused, where is the notion of signal coming from? Is that an encoding of the reaction rules from the paper?

I would have expected a more direct implementation with administrative instructions wait' and notify', as on paper. And then perhaps auxiliary functions like the following to implement context matching:

match_wait' : code -> (memory_instance * address * f64 * (code -> code)) option
match_notify' : code -> (memory_instance * address * i64 * (code -> code)) option

(where the returned functions are "plug" function generated on the way to replace the found instruction with new code).

Also, the step function as written seems to initiate all reactions eagerly in a meta-level loop, whereas each wait/notify match is a separate step in the paper rules.

@conrad-watt
Copy link
Collaborator Author

conrad-watt commented Oct 24, 2022

We discussed before that we wanted to shift away from the paper formulation to an axiomatic specification for wait/notify - i.e. emitting actions without blocking that then get resolved consistently in the memory model. These "signals" are intended to try to reflect the actions relevant for wait/notify in that formulation. I guess I could rename the datatype here to "action" to make the correspondance clearer.

@rossberg
Copy link
Member

Ah okay, that probably was a long time ago, so I don't recall. :) What would the reduction rules look like? If these signals correspond to actions, why do they become part of the thread representation in the interpreter?

@conrad-watt
Copy link
Collaborator Author

I think you're right - they shouldn't be part of the thread representation. I've just changed things up to introduce an explicit suspend administrative instruction, and tweaked the step function to now return a pair of thread * action, removing the action from the thread representation itself.

The reduction rules will look something like (roughly)

wait -> i32 1
with act = load... (not matching)

wait -> suspend
with act = load... (matching) + wait ...
suspend -> i32 0
with act = woken ...

suspend -> i32 2
with <some timeout handling>
notify -> i32 n
with act = wakes n...

and then in the axiomatic memory model, all the wait, woken, and wakes actions have to satisfy a queue specification.

interpreter/exec/eval.ml Outdated Show resolved Hide resolved
interpreter/exec/eval.ml Outdated Show resolved Hide resolved
interpreter/exec/eval.ml Outdated Show resolved Hide resolved
interpreter/exec/eval.ml Outdated Show resolved Hide resolved
interpreter/runtime/memory.ml Outdated Show resolved Hide resolved
interpreter/exec/eval.ml Outdated Show resolved Hide resolved
interpreter/exec/eval.ml Outdated Show resolved Hide resolved
interpreter/exec/eval.ml Outdated Show resolved Hide resolved
interpreter/exec/eval.ml Outdated Show resolved Hide resolved
interpreter/exec/eval.ml Outdated Show resolved Hide resolved
conrad-watt and others added 2 commits April 25, 2023 07:04
Co-authored-by: Andreas Rossberg <rossberg@dfinity.org>
@conrad-watt
Copy link
Collaborator Author

@rossberg apologies that I let this lie for so long, but PTAL

@conrad-watt conrad-watt mentioned this pull request May 16, 2023
@conrad-watt
Copy link
Collaborator Author

@rossberg friendly ping

let ts1', count1 = wake ts1 m addr count in
let ts2', count2 = wake ts2 m addr (Int32.sub count count1) in
ts1' @ [{t' with code = plug_value t'.code (I32 (Int32.add count1 count2))}] @ ts2'
| _ -> ts1 @ [t'] @ ts2
Copy link
Member

Choose a reason for hiding this comment

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

If WaitAction is not distinguished here, why is it actually needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fine with removing it - I added it to more closely match the paper but since Notify_actions are being dealt with operationally here it doesn't have any direct purpose.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, we may want to think about getting the interpreter closer to the spec somehow wrt the actions it emits, and possibly how it uses them. But let's land this for now.

type suspend_signal =
No_signal
(* memory, cell index, target timestamp *)
| Wait_signal of memory_inst * Memory.address * float
Copy link
Member

Choose a reason for hiding this comment

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

Good question, it should correspond to what we using on paper. Is that a real number?

interpreter/exec/eval.ml Outdated Show resolved Hide resolved
@conrad-watt
Copy link
Collaborator Author

@rossberg ptal

@conrad-watt conrad-watt merged commit 09f2831 into main May 30, 2023
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.

None yet

2 participants