[acqrel] Added acqrel load and store instructions [v8/v8 : main]
Rezvan Mahdavi Hezaveh (Gerrit)
Rezvan Mahdavi Hezaveh added 4 comments![]( "Open in Gerrit")
void AtomicStoreReleaseMem(FullDecoder* decoder, StoreType type,Two questions here:
1) Is this the right place to check for the bit 6 of the memarg? If no, where is the right place?
2) How we actually check the bit 6? What is another instructions that works with memarg bits to look as an example?
V(acquire_release, "acquire_release memory ordering", false) \How and where wasm feature flags are being used? For example, I cannot see any usage of `v8_flags.shared` in the code base, even though the implementation of shared-everything thread has been started.
V(I32AtomicLoadAcquire, 0xfe50, i_i, "i32.atomic.load_acq", i_l) \I understand the `i_i` (i32 as input and i32 as output) in the signature but what is the `i_l` for?
// Tests should be added heretes/cctest/wasm/ vs test/mjsunit/wasm/?
Looks like the first one tested instructions and the second one is for js interop?
Related details
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Deepti Gandluri (Gerrit)
Deepti Gandluri added 4 comments![]( "Open in Gerrit")
void AtomicStoreReleaseMem(FullDecoder* decoder, StoreType type,Two questions here:
1) Is this the right place to check for the bit 6 of the memarg? If no, where is the right place?
2) How we actually check the bit 6? What is another instructions that works with memarg bits to look as an example?
I'm not sure I fully understand the question, I'm probably missing something - what should bit 6 have a check for? Any loads/Stores or memory operations will have a memarg - it is an immediate parameter to memory instructions. Decoding of memarg though, we should go into tomorrow.
V(acquire_release, "acquire_release memory ordering", false) \How and where wasm feature flags are being used? For example, I cannot see any usage of `v8_flags.shared` in the code base, even though the implementation of shared-everything thread has been started.
Wasm feature flags are treated slightly differently, take a look at https://source.chromium.org/chromium/chromium/src/+/main:v8/src/wasm/wasm-feature-flags.h, expressed in macros makes it hard to find in chrome search.
V(I32AtomicLoadAcquire, 0xfe50, i_i, "i32.atomic.load_acq", i_l) \I understand the `i_i` (i32 as input and i32 as output) in the signature but what is the `i_l` for?
The format for the signature is <output_type>_<input1_type, input2_type,...>. The l here stands for long, for the offset into memory that needs to be loaded. It's possible the documentation may be outdates to not reflect that Wasm has 64-bit pointers now.
// Tests should be added heretes/cctest/wasm/ vs test/mjsunit/wasm/?
Looks like the first one tested instructions and the second one is for js interop?
Yes, also a couple of other distinctions cctest are easier to debug because the wasm cctest framework is lightweight, and if you were writing unit tests for the compiler itself, they're easier to do as cctests. js tests test the whole pipeline - decoding, compilation, instantiation and execution.
Related details
- Rezvan Mahdavi Hezaveh
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |