Support parsing ordering immediate for i32.atomic.load

[stevenfontanella]

• Copy over spec test generation script from Binaryen, with some changes, e.g. most cases commented except for i32.atomic.load.
  • diff from the Binaryen script
• Add text parsing support for the ordering immediate on i32.atomic.load.

Once the approach looks good, I'll continue with printing support, binary encoding and decoding, and the remaining instructions.

[rmahdav]

Not sure if you intentionally add ordering to i64.atomic.load here or not. Since you are going to update other instructions in follow-up PRs, it is okay to leave it like this probably.

[stevenfontanella]

It's intentional because the ATOMIC_LOAD lexeme got the extra ordering argument which needs to be included here:

relaxed-atomics/interpreter/text/parser.mly

Line 222 in 6a5e3ad

%token<int option -> Memory.offset -> Ast.ordering -> Ast.instr'> ATOMIC_LOAD

. But the complete support still isn't there for i64.atomic.load because i64_atomic_load doesn't take the argument and is hardcoded to use SeqCst instead:

relaxed-atomics/interpreter/syntax/operators.ml

Lines 99 to 100 in 6a5e3ad

	let i64_atomic_load align offset =
	AtomicLoad {ty = I64Type; align; offset; pack = None; ordering = SeqCst}

.

[rmahdav]

Got it. That makes sense.

[rmahdav]

Unrelated to this PR, but I noticed usage of ATOMIC_STORE for some of the load instructions such as i32.atomic.load8_u and the 4 instructions below. Is this a mistake or there is an explanation for that?

[stevenfontanella]

Interesting, that does seem like a typo. Will look at this separately.

[tlively]

If this does cause other atomic loads to be parsed as stores, then I would hope there are tests that fail because of it.

[stevenfontanella]

It was introduced here. ATOMIC_LOAD and ATOMIC_STORE are both tokens with the same signature (at least until this PR): https://github.com/WebAssembly/threads/blob/cc535ada1aa21cfaa3cabf3ac73b89acef78a0a0/interpreter/text/parser.mly#L222, so it turns out they're actually indistinguishable. They don't correspond to text since that's done on the LHS of the token parser: https://github.com/WebAssembly/threads/blob/cc535ada1aa21cfaa3cabf3ac73b89acef78a0a0/interpreter/text/lexer.mll#L267.

So there's no real bug, it's just confusing code. Arguably ATOMIC_LOAD and ATOMIC_STORE could be folded into one token (ATOMIC_MEMORY_OP?) and ditto for similar instructions like LOAD and STORE, but the naming might end up more confusing. I'll send a PR to the threads repo.

[stevenfontanella]

And FWIW I verified this by round-tripping a i32.atomic.load8_u instruction with ./interpreter/wasm test/core/relaxed-atomics/relaxed-atomics.wast -o out.wast which gave back the same instruction.

[tlively]

Approach looks good to me (FWIW)