Re: [PATCH for-8.1] tcg: Use HAVE_CMPXCHG128 instead of CONFIG_CMPXCHG128

[Richard Henderson <richard.henderson@linaro.org>]

On 7/13/23 22:36, Philippe Mathieu-Daudé wrote:
> Hi Richard,
> 
> On 13/7/23 22:23, Richard Henderson wrote:
>> We adjust CONFIG_ATOMIC128 and CONFIG_CMPXCHG128 with
>> CONFIG_ATOMIC128_OPT in atomic128.h.  It is difficult
>> to tell when those changes have been applied with the
>> ifdef we must use with CONFIG_CMPXCHG128.  So instead
>> use HAVE_CMPXCHG128, which triggers -Werror-undef when
>> the proper header has not been included.
>>
>> Improves tcg_gen_atomic_cmpxchg_i128 for s390x host, which
>> requires CONFIG_ATOMIC128_OPT.  Without this we fall back
>> to EXCP_ATOMIC to single-step 128-bit atomics, which is
>> slow enough to cause some tests to time out.
>>
>> Reported-by: Thomas Huth <thuth@redhat.com>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>
>> Thomas, this issue does not quite match the one you bisected, but
>> other than the cmpxchg, I don't see any see any qemu_{ld,st}_i128
>> being used in BootLinuxS390X.test_s390_ccw_virtio_tcg.
>>
>> As far as I can see, this wasn't broken by the addition of
>> CONFIG_ATOMIC128_OPT, rather that fix didn't go far enough.
>>
>> Anyway, test_s390_ccw_virtio_tcg now passes in 159s on our host.
> 
> IIUC:
> 
> If we have CONFIG_ATOMIC128, we use qatomic_cmpxchg__nocheck;
> else if we have CONFIG_CMPXCHG128 we use __sync_val_compare_and_swap_16;
> in both cases we set HAVE_CMPXCHG128;
> otherwise we can not use atomic128 cmpxchg().
> 
> (I'm trying to figure why we need both CONFIGs).

Or sometimes we use inline asm, because there's no compiler support at all.
Please see host/include/*/host/atomic16-*.h.


r~