| Message ID | 20170425083555.13547-1-nikunj@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 25 April 2017 at 09:35, Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> 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 <nikunj@linux.vnet.ibm.com> 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
Peter Maydell <peter.maydell@linaro.org> writes: > On 25 April 2017 at 09:35, Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> 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 <nikunj@linux.vnet.ibm.com> > > 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(&x, 0); ^ /tmp/atomic.c:7:3: warning: implicit declaration of function '__atomic_store_8' is invalid in C99 [-Wimplicit-function-declaration] __atomic_store_8(&x, 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, &y, 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(&x, 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(&x, 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
On 25 April 2017 at 09:58, Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> 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(&x, 0); > ^ > /tmp/atomic.c:7:3: warning: implicit declaration of function '__atomic_store_8' is invalid in C99 [-Wimplicit-function-declaration] > __atomic_store_8(&x, 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, &y, 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(&x, 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(&x, 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
Peter Maydell <peter.maydell@linaro.org> writes: > On 25 April 2017 at 09:58, Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> 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(&x, 0); >> ^ >> /tmp/atomic.c:7:3: warning: implicit declaration of function '__atomic_store_8' is invalid in C99 [-Wimplicit-function-declaration] >> __atomic_store_8(&x, 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, &y, 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(&x, 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(&x, 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
On 25 April 2017 at 10:16, Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote: > Peter Maydell <peter.maydell@linaro.org> writes: > >> On 25 April 2017 at 09:58, Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> 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
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~
diff --git a/configure b/configure index d31a3e8..1e5f7af 100755 --- a/configure +++ b/configure @@ -4598,6 +4598,9 @@ int main(void) EOF if compile_prog "" "" ; then atomic128=yes + elif compile_prog "" "-latomic" ; then + atomic128=yes + lib_atomic="-latomic" fi fi @@ -4628,6 +4631,9 @@ int main(void) EOF if compile_prog "" "" ; then atomic64=yes +elif compile_prog "" "-latomic" ; then + atomic64=yes + lib_atomic="-latomic" fi ######################################## @@ -6065,6 +6071,16 @@ if [ "$TARGET_BASE_ARCH" = "" ]; then TARGET_BASE_ARCH=$TARGET_ARCH fi +if test $atomic64 == "yes" || test $atomic128 == "yes" ; then + libs_softmmu="$lib_atomic $libs_softmmu" +elif test $mttcg == "yes" && test $TARGET_BASE_ARCH == "ppc"; then + echo + echo "Note: Atomic library (-latomic) not available, falling" + echo " back to single threaded mode by default" + echo + mttcg=no +fi + symlink "$source_path/Makefile.target" "$target_dir/Makefile" upper() {
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 <nikunj@linux.vnet.ibm.com> --- Reference: https://lists.gnu.org/archive/html/qemu-ppc/2017-04/msg00277.html --- configure | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)