Re: [Qemu-devel] [PATCH RFC] configure: fix clang failure for libatomic

[Richard Henderson]

On 04/25/2017 10:35 AM, Nikunj A Dadhania wrote:

if compile_prog "" "" ; then
  atomic128=yes
+  elif compile_prog "" "-latomic" ; then
+ atomic128=yes
+ lib_atomic="-latomic"
fi


This is a problem, because I think you'll find that gcc now advertises 
CONFIG_ATOMIC128 for *all* hosts.


This is because by definition, libatomic supplies fallback routines for types 
of arbitrary size.  However, the fallback routines use locking, not actual 
atomic operations.  So the configure test is no longer testing what we intended.



r~

Re: [Qemu-devel] [PATCH RFC] configure: fix clang failure for libatomic

[Peter Maydell]

On 25 April 2017 at 10:16, Nikunj A Dadhania  wrote:
> Peter Maydell  writes:
>
>> On 25 April 2017 at 09:58, Nikunj A Dadhania  
>> wrote:
>>> /tmp/atomic-1660e0.o: In function `main':
>>> /tmp/atomic.c:(.text+0x28): undefined reference to `__atomic_load_8'
>>> /tmp/atomic.c:(.text+0x40): undefined reference to `__atomic_store_8'
>>> /tmp/atomic.c:(.text+0x69): undefined reference to 
>>> `__atomic_compare_exchange_8'
>>> /tmp/atomic.c:(.text+0x7d): undefined reference to `__atomic_exchange_8'
>>> /tmp/atomic.c:(.text+0x91): undefined reference to 
>>> `__atomic_fetch_add_8'
>>> clang-3.9: error: linker command failed with exit code 1 (use -v to see 
>>> invocation)
>>>
>>> With -latomic, the linker succeeds in getting the binary.
>>
>> What host is this on ?
>
> $ uname -a
> Linux abhimanyu 4.9.13-200.fc25.x86_64 #1 SMP Mon Feb 27 16:48:42 UTC 2017 
> x86_64 x86_64 x86_64 GNU/Linux

Hmm, I can repro that, but there's a problem even if we do link against
libatomic. If you disassemble the resulting binaries, gcc produces
what we want:
 f0 48 01 45 e8  lock add %rax,-0x18(%rbp)
and other inline atomic instructions.
clang on the other hand produces
 e8 33 fe ff ff  callq  400610 <__atomic_fetch_add_8@plt>
which is an expensive call to an out of line subroutine.

We need to figure out how to get clang to just emit the inline
operations.

thanks
-- PMM

Re: [Qemu-devel] [PATCH RFC] configure: fix clang failure for libatomic

[Nikunj A Dadhania]

Peter Maydell  writes:

> On 25 April 2017 at 09:58, Nikunj A Dadhania  
> wrote:
>> I was trying out the program in the configure script with clang and I do
>> get errors without libatomic:
>>
>> $ clang /tmp/atomic.c
>> /tmp/atomic.c:6:7: warning: implicit declaration of function 
>> '__atomic_load_8' is invalid in C99 [-Wimplicit-function-declaration]
>>   y = __atomic_load_8(, 0);
>>   ^
>> /tmp/atomic.c:7:3: warning: implicit declaration of function 
>> '__atomic_store_8' is invalid in C99 [-Wimplicit-function-declaration]
>>   __atomic_store_8(, y, 0);
>>   ^
>> /tmp/atomic.c:8:3: warning: implicit declaration of function 
>> '__atomic_compare_exchange_8' is invalid in C99 
>> [-Wimplicit-function-declaration]
>>   __atomic_compare_exchange_8(, , x, 0, 0, 0);
>>   ^
>> /tmp/atomic.c:9:3: warning: implicit declaration of function 
>> '__atomic_exchange_8' is invalid in C99 [-Wimplicit-function-declaration]
>>   __atomic_exchange_8(, y, 0);
>>   ^
>> /tmp/atomic.c:10:3: warning: implicit declaration of function 
>> '__atomic_fetch_add_8' is invalid in C99 [-Wimplicit-function-declaration]
>>   __atomic_fetch_add_8(, y, 0);
>>   ^
>> 5 warnings generated.
>> /tmp/atomic-1660e0.o: In function `main':
>> /tmp/atomic.c:(.text+0x28): undefined reference to `__atomic_load_8'
>> /tmp/atomic.c:(.text+0x40): undefined reference to `__atomic_store_8'
>> /tmp/atomic.c:(.text+0x69): undefined reference to 
>> `__atomic_compare_exchange_8'
>> /tmp/atomic.c:(.text+0x7d): undefined reference to `__atomic_exchange_8'
>> /tmp/atomic.c:(.text+0x91): undefined reference to `__atomic_fetch_add_8'
>> clang-3.9: error: linker command failed with exit code 1 (use -v to see 
>> invocation)
>>
>> With -latomic, the linker succeeds in getting the binary.
>
> What host is this on ?

$ uname -a
Linux abhimanyu 4.9.13-200.fc25.x86_64 #1 SMP Mon Feb 27 16:48:42 UTC 2017 
x86_64 x86_64 x86_64 GNU/Linux
$

Regards
Nikunj

Re: [Qemu-devel] [PATCH RFC] configure: fix clang failure for libatomic

[Peter Maydell]

On 25 April 2017 at 09:58, Nikunj A Dadhania  wrote:
> I was trying out the program in the configure script with clang and I do
> get errors without libatomic:
>
> $ clang /tmp/atomic.c
> /tmp/atomic.c:6:7: warning: implicit declaration of function 
> '__atomic_load_8' is invalid in C99 [-Wimplicit-function-declaration]
>   y = __atomic_load_8(, 0);
>   ^
> /tmp/atomic.c:7:3: warning: implicit declaration of function 
> '__atomic_store_8' is invalid in C99 [-Wimplicit-function-declaration]
>   __atomic_store_8(, y, 0);
>   ^
> /tmp/atomic.c:8:3: warning: implicit declaration of function 
> '__atomic_compare_exchange_8' is invalid in C99 
> [-Wimplicit-function-declaration]
>   __atomic_compare_exchange_8(, , x, 0, 0, 0);
>   ^
> /tmp/atomic.c:9:3: warning: implicit declaration of function 
> '__atomic_exchange_8' is invalid in C99 [-Wimplicit-function-declaration]
>   __atomic_exchange_8(, y, 0);
>   ^
> /tmp/atomic.c:10:3: warning: implicit declaration of function 
> '__atomic_fetch_add_8' is invalid in C99 [-Wimplicit-function-declaration]
>   __atomic_fetch_add_8(, y, 0);
>   ^
> 5 warnings generated.
> /tmp/atomic-1660e0.o: In function `main':
> /tmp/atomic.c:(.text+0x28): undefined reference to `__atomic_load_8'
> /tmp/atomic.c:(.text+0x40): undefined reference to `__atomic_store_8'
> /tmp/atomic.c:(.text+0x69): undefined reference to 
> `__atomic_compare_exchange_8'
> /tmp/atomic.c:(.text+0x7d): undefined reference to `__atomic_exchange_8'
> /tmp/atomic.c:(.text+0x91): undefined reference to `__atomic_fetch_add_8'
> clang-3.9: error: linker command failed with exit code 1 (use -v to see 
> invocation)
>
> With -latomic, the linker succeeds in getting the binary.

What host is this on ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH RFC] configure: fix clang failure for libatomic

[Nikunj A Dadhania]

Peter Maydell  writes:

> On 25 April 2017 at 09:35, Nikunj A Dadhania  
> wrote:
>> Travis builds failure was reported for powernv boot-serial test with
>> qemu built with clang.
>>
>> Debugging revealed that CONFIG_ATOMIC64 wasnt getting set for the clang
>> build because of that atomic operations weren't being used and was
>> resulting in MTTCG failure in the powernv boot-serial test.
>>
>> libatomic is required to successfully test atomic64 and atomic128 for
>> clang. Introduced newer checks for the same. And on failure default to
>> single threaded tcg support in PPC64.
>>
>> Signed-off-by: Nikunj A Dadhania 
>
> Do we really need libatomic?

I was trying out the program in the configure script with clang and I do
get errors without libatomic:

$ clang /tmp/atomic.c 
/tmp/atomic.c:6:7: warning: implicit declaration of function 
'__atomic_load_8' is invalid in C99 [-Wimplicit-function-declaration]
  y = __atomic_load_8(, 0);
  ^
/tmp/atomic.c:7:3: warning: implicit declaration of function 
'__atomic_store_8' is invalid in C99 [-Wimplicit-function-declaration]
  __atomic_store_8(, y, 0);
  ^
/tmp/atomic.c:8:3: warning: implicit declaration of function 
'__atomic_compare_exchange_8' is invalid in C99 
[-Wimplicit-function-declaration]
  __atomic_compare_exchange_8(, , x, 0, 0, 0);
  ^
/tmp/atomic.c:9:3: warning: implicit declaration of function 
'__atomic_exchange_8' is invalid in C99 [-Wimplicit-function-declaration]
  __atomic_exchange_8(, y, 0);
  ^
/tmp/atomic.c:10:3: warning: implicit declaration of function 
'__atomic_fetch_add_8' is invalid in C99 [-Wimplicit-function-declaration]
  __atomic_fetch_add_8(, y, 0);
  ^
5 warnings generated.
/tmp/atomic-1660e0.o: In function `main':
/tmp/atomic.c:(.text+0x28): undefined reference to `__atomic_load_8'
/tmp/atomic.c:(.text+0x40): undefined reference to `__atomic_store_8'
/tmp/atomic.c:(.text+0x69): undefined reference to 
`__atomic_compare_exchange_8'
/tmp/atomic.c:(.text+0x7d): undefined reference to `__atomic_exchange_8'
/tmp/atomic.c:(.text+0x91): undefined reference to `__atomic_fetch_add_8'
clang-3.9: error: linker command failed with exit code 1 (use -v to see 
invocation)

With -latomic, the linker succeeds in getting the binary.

> I thought the intention here was that atomic operations were only done
> on types of width of the pointer or less, which should all be doable
> by the compiler inline, and thus don't need the external library.

You are right, even without -latomic in libs_softmmu, tests are passing.
But then __atomic_load_8() isn't used in qemu, if it was using them,
build would fail.

> In the past "doesn't work without libatomic" usually meant
>"accidentally tried to do an atomic operation on a type that is too
>wide".

Regards
Nikunj

Re: [Qemu-devel] [PATCH RFC] configure: fix clang failure for libatomic

[Peter Maydell]

On 25 April 2017 at 09:35, Nikunj A Dadhania  wrote:
> Travis builds failure was reported for powernv boot-serial test with
> qemu built with clang.
>
> Debugging revealed that CONFIG_ATOMIC64 wasnt getting set for the clang
> build because of that atomic operations weren't being used and was
> resulting in MTTCG failure in the powernv boot-serial test.
>
> libatomic is required to successfully test atomic64 and atomic128 for
> clang. Introduced newer checks for the same. And on failure default to
> single threaded tcg support in PPC64.
>
> Signed-off-by: Nikunj A Dadhania 

Do we really need libatomic? I thought the intention here was
that atomic operations were only done on types of width of the
pointer or less, which should all be doable by the compiler inline,
and thus don't need the external library. In the past "doesn't
work without libatomic" usually meant "accidentally tried to
do an atomic operation on a type that is too wide".

thanks
-- PMM