Generate atomic instructions defined in the threads proposal#2386
Conversation
danielvallance
changed the title
danielvallance
changed the title
|
@abrown Would you have a few minutes to look at this? I'm not up-to-speed yet on shared-everything threads. |
|
FYI, Andrew is on leave right now, so it might be a bit until he'll be able to look at this. (Not providing any timelines here because once he is back, he might have a lot to work through before getting to this for all I know.) |
alexcrichton
requested review from
alexcrichton
and removed request for
dicej
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
Thanks for this! Could you also run the validate-valid-module fuzzer locally for a bit to double-check these are correct? That can be done with:
$ FUZZER=validate_valid_module cargo +nightly fuzz run -s none run
If nothing shows up for 10-15m I'd say it's good to go.
| #[inline] | ||
| fn atomic_instruction_valid(module: &Module, _builder: &mut CodeBuilder) -> bool { | ||
| module.config.threads_enabled && module.config.shared_everything_threads_enabled | ||
| } | ||
|
|
There was a problem hiding this comment.
For this it's ok to drop shared_everything_threads_enabled since these instructions only require the wasm threads proposal.
There was a problem hiding this comment.
Ahh ok I see, will do, thanks!
There was a problem hiding this comment.
Hi @alexcrichton , I have run this for 15 minutes and I did not see any issues apart from some warnings at the start which appear on the main branch also:
WARNING: Failed to find function "__sanitizer_acquire_crash_state". Reason dlsym(RTLD_DEFAULT, __sanitizer_acquire_crash_state): symbol not found.
WARNING: Failed to find function "__sanitizer_print_stack_trace". Reason dlsym(RTLD_DEFAULT, __sanitizer_print_stack_trace): symbol not found.
WARNING: Failed to find function "__sanitizer_set_death_callback". Reason dlsym(RTLD_DEFAULT, __sanitizer_set_death_callback): symbol not found.
Just to sanity check that I was running the tool correctly and that I could recognise an error, I also deliberately added a mistake to it and it fell over after a couple of seconds with a very clear and obvious error.
Would you like me to attach the output of the successful run?
There was a problem hiding this comment.
Nah that sounds good, thanks for double-checking! And yeah those warnings are ok to ignore and otherwise having no "THIS DIED" sort of output is expected.
This commit adds suppport for emitting atomic instructions when wasm_smith::Config::threads_enabled and wasm_smith::Config::shared_everything_threads_enabled are true. A link to the atomic instructions in the threads proposal: https://webassembly.github.io/threads/core/syntax/instructions.html#syntax-instr-atomic-memory
danielvallance
force-pushed
the
threads_enabled_instructions
branch
from
ae2f630 to
40bf650
Compare
4 participants
This commit adds suppport for emitting atomic instructions when wasm_smith::Config::threads_enabled and
wasm_smith::Config::shared_everything_threads_enabled are true.
A link to the atomic instructions in the threads proposal: https://webassembly.github.io/threads/core/syntax/instructions.html#syntax-instr-atomic-memory
Addresses #2384