Behavior of t.atomic.rmw.cmpxchg instructions

[q82419]

Hi,

As the 17. and 18. steps in the spec:

17. If c1 equals c2, then:
    Let c be c2.
18. Else:
    Let c be c3.

And the 21. step will write the c back to the memory.
Where c1 is the origin value on the memory, c2 is the expected value, c3 is the value to replace.

Maybe I can realize this as following simply:

if (memory[addr] != expected)
    memory[addr] = replacement;

But the closest function in C++ std::atomic<T>::compare_exchange_strong has the different behavior.

if (memory[addr] == expected)
   memory[addr] = replacement;

The atomic_rmw instructions are similar to the member functions of std::atomic<T> in C++ STL, such as fetch_add(), fetch_xor, etc.

Is it necessary to have the same behavior between cmpxchg instructions and std::atomic<T>::compare_exchange_strong in C++?

Thanks.

[conrad-watt]

Ah, there are unfortunate mistakes in the draft spec text here. The overview correctly describes the intended behaviour, which is the same as the C++ compare_exchange_strong.

[q82419]

Ah, there are unfortunate mistakes in the draft spec text here. The overview correctly describes the intended behaviour, which is the same as the C++ compare_exchange_strong.

Hi @conrad-watt ,
Thanks for the information!

Another question is that according to the overview:

The atomic compare exchange operators take three operands: an address, an expected value, and a replacement value. If the loaded value is equal to the expected value, the replacement value is stored to the same memory address. If the values are not equal, no value is stored. In either case, the loaded value is returned.

But take the test suite for example:

(invoke "init" (i64.const 0x1111111111111111))
(assert_return (invoke "i32.atomic.rmw8.cmpxchg_u" (i32.const 0) (i32.const 0x11111111) (i32.const 0xcdcdcdcd)) (i32.const 0x11))
(assert_return (invoke "i64.atomic.load" (i32.const 0)) (i64.const 0x1111111111111111))

When invoking the i32.atomic.rmw8.cmpxchg_u in line 326, the address is 0, the expected value is 0x11, and the replacement value is 0xcd. Therefore the loaded value is 0x11.
The loaded value 0x11 is equal to the expected value 0x11, so the replacement value 0xcd is stored to the same memory address.
Then when invoking the i64.atomic.load in line 327, the returned value shoud be i64.const 0x11111111111111cd, not 0x1111111111111111?
The same questions are at line 334, 342, 350, and 358.

Thanks.

[conrad-watt]

Oh dear. You've identified an interesting discrepancy, and it looks like there is real implementation divergence here.

The culprit is the line in the overview which governs how value comparison works. It states that for i32.atomic.rmw8.cmpxchg_u, the "expected" value is wrapped to 8 bits before it is compared. However the reference interpreter and test suite are acting as though the value is not wrapped before comparison (i.e. compare line 325 to line 371).

Running the following test, it appears that Chrome and Firefox implement the overview semantics, while Safari implements the test suite semantics.

(module
  (memory 1 1)
  (func (export "testWrapping") (result i32)
    (i32.store (i32.const 0) (i32.const 0x11))
    (i32.atomic.rmw8.cmpxchg_u (i32.const 0) (i32.const 0x1111) (i32.const 0xcd))
    (drop)
    (i32.load (i32.const 0))))
    
;; chrome returns 205, firefox returns 205, safari returns 17

[conrad-watt]

Well... now we have to work out which is the preferable semantics. Thanks for discovering this @q82419 :)

@dtig who would be the right person to ask regarding V8's code generation? Is the current V8 semantics implemented by wrapping the expected value to the packed width as a separate platform asm instruction, or is there an asm instruction that directly implements sub-32-bit cmpxchg with the wrapping semantics for the expected value?

[conrad-watt]

@Constellation it looks like WebKit's behaviour is the odd one out here, although the fault lies with the reference interpreter for implementing this behaviour in the first place (as seen in the tests added here, which pass in the reference interpreter but should fail according to the overview). In principle, would it be possible to change the behaviour here to align with V8 and SpiderMonkey? (we would also change the reference interpreter and relevant tests).

[q82419]

@conrad-watt
Thanks for your verification very much!

[conrad-watt]

To bump this: the spec, interpreter, and tests in upstream-rebuild, and the spec render at https://webassembly.github.io/threads/ (see #198), should now reflect the following:

• cmpxchg performs the replacement if the expected value matches the read value (see here)
• packed cmpxchg wraps before comparing, enshrining the overview/Chrome/Firefox semantics (see here)

Please let me know if any remaining artefacts fail to reflect the above. I'll flag these issues up again at the in-person meeting, since there really was an unfortunate divergence here.