public inbox for gentoo-dev@lists.gentoo.org
 help / color / mirror / Atom feed
From: Eli Schwartz <eschwartz93@gmail.com>
To: gentoo-dev@lists.gentoo.org
Subject: [gentoo-dev] [PATCH] flag-o-matic.eclass: simplify implementation and work in all cases
Date: Thu, 28 Mar 2024 00:58:11 -0400	[thread overview]
Message-ID: <20240328045811.2289466-1-eschwartz93@gmail.com> (raw)

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<RnNoiseStats> 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=81358
Bug: https://bugs.gentoo.org/820101
Bug: https://bugs.gentoo.org/925672
Signed-off-by: Eli Schwartz <eschwartz93@gmail.com>
---
 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() {
 }
 
 # @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 bytesize
-# is not specified, then check the four most common byte sizes (1, 2, 4, 8).
-# >=16-byte atomics are not included in this default set and must be explicitly
-# passed if required.  This may require you to add a macro definition like
-# -Duint128_t=__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
-	[[ "${#}" == "0" ]] && bytesizes=( "1" "2" "4" "8" ) || bytesizes="${@}"
-
-	for bytesize in ${bytesizes[@]}
-	do
-		# this sample program is informed by the great testing from the buildroot project:
-		# https://github.com/buildroot/buildroot/commit/6856e417da4f3aa77e2a814db2a89429af072f7d
-		read -r -d '' code <<- EOF
-			#include <stdint.h>
-			int main(void)
-			{
-				uint$((${bytesize} * 8))_t a = 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 but 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 continue
-		# 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 build
+	# 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=81358
+	append-libs "-Wl,--push-state,--as-needed,-latomic,--pop-state"
 }
 
 fi
-- 
2.43.2



             reply	other threads:[~2024-03-28  4:58 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-28  4:58 Eli Schwartz [this message]
2024-03-29 18:46 ` [gentoo-dev] [PATCH] flag-o-matic.eclass: simplify implementation and work in all cases Sam James

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240328045811.2289466-1-eschwartz93@gmail.com \
    --to=eschwartz93@gmail.com \
    --cc=gentoo-dev@lists.gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox