NetBSD Problem Report #54428

From www@netbsd.org  Thu Aug  1 18:10:33 2019
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 6AA717A178
	for <gnats-bugs@gnats.NetBSD.org>; Thu,  1 Aug 2019 18:10:33 +0000 (UTC)
Message-Id: <20190801181032.8C15A7A1B4@mollari.NetBSD.org>
Date: Thu,  1 Aug 2019 18:10:32 +0000 (UTC)
From: n54@gmx.com
Reply-To: n54@gmx.com
To: gnats-bugs@NetBSD.org
Subject: GCC broken with std::atomic<__int128_t> with -mcx16
X-Send-Pr-Version: www-1.0

>Number:         54428
>Category:       toolchain
>Synopsis:       GCC broken with std::atomic<__int128_t> with -mcx16
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    toolchain-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Aug 01 18:15:00 +0000 2019
>Last-Modified:  Sun Aug 04 14:25:00 +0000 2019
>Originator:     Kamil Rytarowski
>Release:        NetBSD/amd64 8.99.42
>Organization:
TNF
>Environment:
NetBSD rugged 8.99.42 NetBSD 8.99.42 (GENERIC) #2: Sat Jun  1 22:52:15 CEST 2019  root@chieftec:/public/netbsd-root/sys/arch/amd64/compile/GENERIC amd64
>Description:
NetBSD deliberately does not ship with libatomic rejecting its idea due to corner cases when it does not work reliably.

https://clang.llvm.org/docs/Toolchain.html#atomics-library

This design decision breaks std::atomic<__int128_t> on NetBSD/amd64 with CPU supporting LOCK CMPXCHG16B, because GCC translates __atomic calls to libatomic library calls always.

This does not happen with Clang as it builds a functional code.

$ man gcc

       -mcx16
           This option enables GCC to generate "CMPXCHG16B" instructions in
           64-bit code to implement compare-and-exchange operations on 16-byte
           aligned 128-bit objects.  This is useful for atomic updates of data
           structures exceeding one machine word in size.  The compiler uses
           this instruction to implement __sync Builtins.  However, for
           __atomic Builtins operating on 128-bit integers, a library call is
           always used.


$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/lto-wrapper
Target: x86_64--netbsd
Configured with: /usr/src/tools/gcc/../../external/gpl3/gcc/dist/configure --target=x86_64--netbsd --enable-long-long --enable-threads --with-bugurl=http://www.NetBSD.org/Misc/send-pr.html --with-pkgversion='NetBSD nb3 20190319' --with-system-zlib --without-isl --enable-__cxa_atexit --enable-libstdcxx-time=rt --enable-libstdcxx-threads --with-diagnostics-color=auto-if-env --with-tune=nocona --with-default-libstdcxx-abi=new --with-mpc-lib=/var/obj/mknative/amd64-x86_64/usr/src/external/lgpl3/mpc/lib/libmpc --with-mpfr-lib=/var/obj/mknative/amd64-x86_64/usr/src/external/lgpl3/mpfr/lib/libmpfr --with-gmp-lib=/var/obj/mknative/amd64-x86_64/usr/src/external/lgpl3/gmp/lib/libgmp --with-mpc-include=/usr/src/external/lgpl3/mpc/dist/src --with-mpfr-include=/usr/src/external/lgpl3/mpfr/dist/src --with-gmp-include=/usr/src/external/lgpl3/gmp/lib/libgmp/arch/x86_64 --enable-tls --disable-multilib --disable-libstdcxx-pch --build=x86_64--netbsd --host=x86_64--netbsd --with-sysroot=/var/obj/mknativ
 e/amd64-x86_64/usr/src/destdir.amd64
Thread model: posix
gcc version 7.4.0 (nb3 20190319)

$ clang -v
clang version 7.0.1 (tags/RELEASE_701/final)
Target: x86_64-unknown-netbsd8.99
Thread model: posix
InstalledDir: /usr/pkg/bin

$ objdump -d ./a.out
[...]
0000000000400a50 <_ZNKSt6atomicInE4loadESt12memory_order>: // std::atomic<__int128>::load(std::memory_order) const
[...]
  400ac3:       f0 48 0f c7 0e          lock cmpxchg16b (%rsi)
[...]
>How-To-Repeat:
$ cat test2.cpp
#include <atomic>

#include <cstdio>

int
main(int argc, char **argv)
{
#if defined(__GCC_HAVE_SYNC_COMPARE_AND_SWAP_16)
	puts("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 defined");
#else
	puts("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 NOT defined");
#endif

	std::atomic<__int128_t> val;

	return val + 1;
}

$ g++ -mcx16 test2.cpp
ld: /var/tmp//cc6DZLsM.o: in function `std::atomic<__int128>::load(std::memory_order) const':
test2.cpp:(.text._ZNKSt6atomicInE4loadESt12memory_order[_ZNKSt6atomicInE4loadESt12memory_order]+0x2d): undefined reference to `__atomic_load_16'
>Fix:
1. Revisit the libatomic statement and ship it to those who are aware about its corner cases.

2. Patch GCC to support 128bit std::atomic without libatomic and reuse lock cmpxchg16b on x86_64.

>Audit-Trail:
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: toolchain/54428: GCC broken with std::atomic<__int128_t> with
 -mcx16
Date: Fri, 2 Aug 2019 07:15:59 +0200

 On Thu, Aug 01, 2019 at 06:15:01PM +0000, n54@gmx.com wrote:
 > 1. Revisit the libatomic statement and ship it to those who are aware about its corner cases.
 > 
 > 2. Patch GCC to support 128bit std::atomic without libatomic and reuse lock cmpxchg16b on x86_64.

 3. do not support std::atomic<__int128_t> at all on amd64
 (patch headers to generate an explicit error, or add lib stubs that make
 it error out at link time)?


 Martin

From: Kamil Rytarowski <n54@gmx.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: toolchain/54428: GCC broken with std::atomic<__int128_t> with
 -mcx16
Date: Fri, 2 Aug 2019 15:04:01 +0200

 This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
 --kYow0RIiIuEAf33lZfCBFJxI9PiAFGlWe
 Content-Type: multipart/mixed; boundary="kCwQLOnrlkrPtI3r81aunx4ZgooyDXYKK";
  protected-headers="v1"
 From: Kamil Rytarowski <n54@gmx.com>
 To: gnats-bugs@netbsd.org
 Message-ID: <27806673-394e-99cb-702a-fc66487dcc6a@gmx.com>
 Subject: Re: toolchain/54428: GCC broken with std::atomic<__int128_t> with
  -mcx16
 References: <pr-toolchain-54428@gnats.netbsd.org>
  <20190801181032.8C15A7A1B4@mollari.NetBSD.org>
  <20190802052001.66A417A1BB@mollari.NetBSD.org>
 In-Reply-To: <20190802052001.66A417A1BB@mollari.NetBSD.org>

 --kCwQLOnrlkrPtI3r81aunx4ZgooyDXYKK
 Content-Type: text/plain; charset=utf-8
 Content-Language: en-US
 Content-Transfer-Encoding: quoted-printable

 On 02.08.2019 07:20, Martin Husemann wrote:
 > The following reply was made to PR toolchain/54428; it has been noted b=
 y GNATS.
 >=20
 > From: Martin Husemann <martin@duskware.de>
 > To: gnats-bugs@netbsd.org
 > Cc:=20
 > Subject: Re: toolchain/54428: GCC broken with std::atomic<__int128_t> w=
 ith
 >  -mcx16
 > Date: Fri, 2 Aug 2019 07:15:59 +0200
 >=20
 >  On Thu, Aug 01, 2019 at 06:15:01PM +0000, n54@gmx.com wrote:
 >  > 1. Revisit the libatomic statement and ship it to those who are awar=
 e about its corner cases.
 >  >=20
 >  > 2. Patch GCC to support 128bit std::atomic without libatomic and reu=
 se lock cmpxchg16b on x86_64.
 > =20
 >  3. do not support std::atomic<__int128_t> at all on amd64
 >  (patch headers to generate an explicit error, or add lib stubs that ma=
 ke
 >  it error out at link time)?
 > =20

 It already errors with GCC.

 Why to reject it? Relatively recent amd64 does support this operation
 natively in hardware.

 > =20
 >  Martin
 > =20
 >=20



 --kCwQLOnrlkrPtI3r81aunx4ZgooyDXYKK--

 --kYow0RIiIuEAf33lZfCBFJxI9PiAFGlWe
 Content-Type: application/pgp-signature; name="signature.asc"
 Content-Description: OpenPGP digital signature
 Content-Disposition: attachment; filename="signature.asc"

 -----BEGIN PGP SIGNATURE-----

 iQIzBAEBCAAdFiEELaxVpweEzw+lMDwuS7MI6bAudmwFAl1ENMsACgkQS7MI6bAu
 dmxDDw/+NXFlEUrqM0rBcAINPH6dwlnpofNWrmxKb/avp1cT9yIzzLbe6M1UXOQc
 I/Z6xfi7RF6vc/M/xgjOJADMlwwgA8XxROoWdbO8+CVdWHgEuBqc+hUM6IFDvUqb
 QMyX+Q91vD8N5eLfNly4V4qfI+RsX43EIH0/frlgURqyaXxXKKxgtp3tJqhGAgIA
 UkxUKqbp4dsjqFwkFMRjIaDhQb1llT5I2Kx+Oqv97Hk5QyZ/u/UTdjeBKU9Wa0sH
 c61Nfx3FAtLSrsp/q3nkIOeRTgxakVCHe/T5TAKE1ovyNRE+GOKMW+L/xZ80VFfY
 SerrkypBWm9QHb+PlcYJtt4/86LGm6r9KDNNt9Ebp9SbVArm7JTNGTxYHqDOxezC
 oxHwyBE6g6bV4iTdRBazmPY8/Jz0yf+eQuYuSf8R10OkrYpYwtVIP4qkpL1yTOMz
 m78ByT6RQ9GYd7sS6I//jOL5wgOb+4tqHLS/44MT4wBTMV6LLJ79h29+O3iaE2Yx
 EMKTxpX+Kp169sFkop4MTDJ6RGEGpSQypPhLCTfXgtBk47TkOcVV71bXejxsKV6e
 9ObxH+das9+9y9rVi6zUGDSkj7MoeJ+uOVY/OqWs0IgKnrGiwTuMe+CB1rQwW/bJ
 9RuK7DKpaj2Gtn0YcS9OVCZfqQtedzAwI8bqEpmH4yo8b/P06VM=
 =UYwW
 -----END PGP SIGNATURE-----

 --kYow0RIiIuEAf33lZfCBFJxI9PiAFGlWe--

From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: toolchain/54428: GCC broken with std::atomic<__int128_t> with
 -mcx16
Date: Sun, 4 Aug 2019 08:02:32 +0200

 On Fri, Aug 02, 2019 at 01:10:02PM +0000, Kamil Rytarowski wrote:
 >  Why to reject it? Relatively recent amd64 does support this operation
 >  natively in hardware.

 Yeah, and once we switch our default compiler target cpu for amd64 to
 something new enough we can have a __HAVE_ATOMIC_.... for it and enable
 it on amd64. Or even enable it now depending on a define provided by the
 compiler when targetting a new enough cpu.

 But don't silently runtime-break default builds on devices still officially
 supported.

 Martin

From: coypu@sdf.org
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: toolchain/54428: GCC broken with std::atomic<__int128_t> with
 -mcx16
Date: Sun, 4 Aug 2019 14:22:44 +0000

 let me re-iterate:
 128bit atomics are a minimum requirement to boot Windows in the past two
 versions. ARM>v7 and arm64 also have it.

 We are *very* unusual for not having it. And I have also run into code
 in the wild expecting it.

Home	PR Database Search