On 3/19/22 09:06, Wei Li wrote:
We need a branch to determine when the instruction can touch the
accumulator. But there is a branch provided by movcond.

There is no branch in movcond -- this expands to cmov.

-                /* store value = (old == cmp ? new : old);  */
-                tcg_gen_movcond_tl(TCG_COND_EQ, newv, oldv, cmpv, newv, oldv);
+                tcg_gen_brcond_tl(TCG_COND_EQ, oldv, cmpv, label1);
...
                      /* Perform an unconditional store cycle like physical cpu;
                         must be before changing accumulator to ensure
                         idempotency if the store faults and the instruction
                         is restarted */

Your branch invalidates the comment -- the store becomes conditional, and we no longer get a write fault on read-only pages when the comparison fails. OTOH, we're already getting the incorrect SIGSEGV behaviour, since we get a read fault on an unmapped page instead of a write fault.

The faulting behaviour could be addressed with a write_probe prior to the original load. Alternately, we can use the tcg_gen_atomic_cmpxchg_tl path whenever mod != 3. While an unlocked cmpxchg need not be atomic, it is not required to be non-atomic either, and it would reduce code duplication.


r~

Reply via email to