From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by finch.gentoo.org (Postfix) with ESMTPS id E981A158041 for ; Fri, 29 Mar 2024 18:47:13 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 5D2E6E2AA9; Fri, 29 Mar 2024 18:47:04 +0000 (UTC) Received: from smtp.gentoo.org (mail.gentoo.org [IPv6:2001:470:ea4a:1:5054:ff:fec7:86e4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 23802E2AA6 for ; Fri, 29 Mar 2024 18:47:04 +0000 (UTC) From: Sam James To: Eli Schwartz Cc: gentoo-dev@lists.gentoo.org Subject: Re: [gentoo-dev] [PATCH] flag-o-matic.eclass: simplify implementation and work in all cases In-Reply-To: <20240328045811.2289466-1-eschwartz93@gmail.com> (Eli Schwartz's message of "Thu, 28 Mar 2024 00:58:11 -0400") Organization: Gentoo References: <20240328045811.2289466-1-eschwartz93@gmail.com> User-Agent: mu4e 1.12.2; emacs 30.0.50 Date: Fri, 29 Mar 2024 18:46:59 +0000 Message-ID: <87ttkoana4.fsf@gentoo.org> Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-dev@lists.gentoo.org Reply-to: gentoo-dev@lists.gentoo.org X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" X-Archives-Salt: 111999ab-7138-41a2-abe5-ed58c4a77185 X-Archives-Hash: 2ef0deed0b57a9c130fd3a0814531d50 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Eli Schwartz writes: > It curently uses some magic test to decide whether handcrafted code > works with or without -latomic. But it can claim that -latomic is not > needed for that case, while it is still needed for other cases. > >> okay so append-atomic-flags does not work for me in this case >> noise-suppression-for-voice is doing `struct RnNoiseStats { uint32_t a, = b, c, d; }; std::atomic m_stats;` >> not just a single large integer > > It is simplest to always add -latomic when an ebuild gets that deep > feeling that yeah, it would like some atomics please. The downsides to > listing a linker library are exactly: > > - it might be unavailable > - it might be unneeded > > And the former case is trivial to solve -- this function already does so > -- while the latter case has a sanctioned approach that is already used > for other intrinsic compiler libraries, but not for atomic "because the > build system would have a hard time if we had to build atomic early on" > which isn't a very good reason to break ebuilds which aren't building > sys-devel/gcc. > > As a side benefit, we now handle -latomic such that a package which > requires it, but only for parts of the installed package, does not > overlink to libatomic in *all* binaries/libraries, even if the default > LDFLAGS are overridden and the global -Wl,--as-needed disappears. > > Bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D81358 > Bug: https://bugs.gentoo.org/820101 > Bug: https://bugs.gentoo.org/925672 > Signed-off-by: Eli Schwartz LGTM. Thanks. > --- > eclass/flag-o-matic.eclass | 80 +++++++++----------------------------- > 1 file changed, 19 insertions(+), 61 deletions(-) > > diff --git a/eclass/flag-o-matic.eclass b/eclass/flag-o-matic.eclass > index 5ce7601fdde2..0e5271c7824f 100644 > --- a/eclass/flag-o-matic.eclass > +++ b/eclass/flag-o-matic.eclass > @@ -1015,69 +1015,27 @@ test-compile() { > } >=20=20 > # @FUNCTION: append-atomic-flags > -# @USAGE: [bytes] > # @DESCRIPTION: > -# Attempts to detect if appending -latomic is required to use > -# a specific-sized atomic intrinsic, and if so, appends it. If the byte= size > -# is not specified, then check the four most common byte sizes (1, 2, 4,= 8). > -# >=3D16-byte atomics are not included in this default set and must be e= xplicitly > -# passed if required. This may require you to add a macro definition li= ke > -# -Duint128_t=3D__uint128_t to your CFLAGS. > +# Attempts to detect if appending -latomic works, and does so. > append-atomic-flags() { > - # this implementation is as described in bug #820101 > - local code > - > - # first, ensure we can compile a trivial program > - # this is because we can't distinguish if test-compile > - # fails because -latomic is actually needed or if we have a > - # broken toolchain (like due to bad FLAGS) > - read -r -d '' code <<- EOF > - int main(void) > - { > - return 0; > - } > - EOF > - > - # if toolchain is broken, just return silently. it's better to > - # let other pieces of the build fail later down the line than to > - # make people think that something to do with atomic support is the > - # cause of their problems. > - test-compile "c+ld" "${code}" || return > - > - local bytesizes > - [[ "${#}" =3D=3D "0" ]] && bytesizes=3D( "1" "2" "4" "8" ) || bytesizes= =3D"${@}" > - > - for bytesize in ${bytesizes[@]} > - do > - # this sample program is informed by the great testing from the buildr= oot project: > - # https://github.com/buildroot/buildroot/commit/6856e417da4f3aa77e2a81= 4db2a89429af072f7d > - read -r -d '' code <<- EOF > - #include > - int main(void) > - { > - uint$((${bytesize} * 8))_t a =3D 0; > - __atomic_add_fetch(&a, 3, __ATOMIC_RELAXED); > - __atomic_compare_exchange_n(&a, &a, 2, 1, __ATOMIC_RELAXED, __ATOMIC= _RELAXED); > - return 0; > - } > - EOF > - > - # do nothing if test program links fine > - test-compile "c+ld" "${code}" && continue > - > - # ensure that the toolchain supports -latomic > - test-flags-CCLD "-latomic" &>/dev/null || die "-latomic is required bu= t not supported by $(tc-getCC)" > - > - append-libs "-latomic" > - > - # verify that this did indeed fix the problem > - test-compile "c+ld" "${code}" || \ > - die "libatomic does not include an implementation of ${bytesize}-byte= atomics for this toolchain" > - > - # if any of the required bytesizes require -latomic, no need to contin= ue > - # checking the others > - return > - done > + # Make sure that the flag is actually valid. If it isn't, then maybe the > + # library both doesn't exist and is redundant, or maybe the toolchain is > + # broken, but let the build succeed or fail on its own. > + test-flags-CCLD "-latomic" &>/dev/null || return > + > + # We unconditionally append this flag. In the case that it's needed, the > + # flag is, well, needed. In the case that it's not needed, it causes no > + # harm, because we ensure that this specific library is definitely > + # certainly linked with as-needed. > + # > + # Really, this should be implemented directly in the compiler, including > + # the use of push/pop for as-needed. It's exactly what the gcc spec file > + # does for e.g. -lgcc_s, but gcc is concerned about doing so due to bui= ld > + # system internals and as a result all users have to deal with this mess > + # instead. > + # > + # See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D81358 > + append-libs "-Wl,--push-state,--as-needed,-latomic,--pop-state" > } >=20=20 > fi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iOUEARYKAI0WIQQlpruI3Zt2TGtVQcJzhAn1IN+RkAUCZgcMpF8UgAAAAAAuAChp c3N1ZXItZnByQG5vdGF0aW9ucy5vcGVucGdwLmZpZnRoaG9yc2VtYW4ubmV0MjVB NkJCODhERDlCNzY0QzZCNTU0MUMyNzM4NDA5RjUyMERGOTE5MA8cc2FtQGdlbnRv by5vcmcACgkQc4QJ9SDfkZAivQD/axb4t3We8sYywogtM8+Hem5QQWRuNJVO8753 prw/J8QBAOQ744jJxYBW+bZgnYlc82TkLb77IVWBh3aVIsxfasIL =cfMF -----END PGP SIGNATURE----- --=-=-=--