public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Victor Do Nascimento <victor.donascimento@arm.com>
To: <gcc-patches@gcc.gnu.org>
Cc: <richard.sandiford@arm.com>, <Richard.Earnshaw@arm.com>,
	"Victor Do Nascimento" <victor.donascimento@arm.com>
Subject: [PATCH v2 0/4] Libatomic: Cleanup ifunc selector and aliasing
Date: Tue, 11 Jun 2024 13:42:46 +0100	[thread overview]
Message-ID: <20240611124250.1390575-1-victor.donascimento@arm.com> (raw)

Changes in V2:

As explained in patch v2 1/4, it has become clear that the current
approach of querying assembler support for newer architectural
extensions at compile time is undesirable both from a maintainability
as well as a consistency standpoint - Different compiled versions of
Libatomic may have different features depending on the machine on
which they were built.

These issues make for difficult testing as the explosion in number of
`#ifdef' guards makes maintenance error-prone and the dependence on
binutils version means that, as well as deploying changes for testing
in a variety of target configurations, testing must also involve
compiling the library on an increasing number of host configurations,
meaning that the chance of bugs going undetected increases (as was
proved in the pre-commit CI which, due to the use of an older version
of Binutils, picked up on a runtime-error that had hitherto gone
unnoticed).

We therefore do away with the use of all assembly instructions
dependent on Binutils 2.42, choosing to replace them with `.inst's
instead.  This eliminates the latent bug picked up by CI and will
ensure consistent builds of Libatomic across all versions of Binutils.

---

The recent introduction of the optional LSE128 and RCPC3 architectural
extensions to AArch64 has further led to the increased flexibility of
atomic support in the architecture, with many extensions providing
support for distinct atomic operations, each with different potential
applications in mind.

This has led to maintenance difficulties in Libatomic, in particular
regarding the way the ifunc selector is generated via a series of
macro expansions at compile-time.

Until now, irrespective of the atomic operation in question, all atomic
functions for a particular operand size were expected to have the same
number of ifunc alternatives, meaning that a one-size-fits-all
approach could reasonably be taken for the selector.

This meant that if, hypothetically, for a particular architecture and
operand size one particular atomic operation was to have 3 different
implementations associated with different extensions, libatomic would
likewise be required to present three ifunc alternatives for all other
atomic functions.

The consequence in the design choice was the unnecessary use of
function aliasing and the unwieldy code which resulted from this.

This patch series attempts to remediate this issue by making the
preprocessor macros defining the number of ifunc alternatives and
their respective selection functions dependent on the file importing
the ifunc selector-generating framework.

all files are given `LAT_<FILENAME>' macros, defined at the beginning
and undef'd at the end of the file.  It is these macros that are
subsequently used to fine-tune the behaviors of `libatomic_i.h' and
`host-config.h'.

In particular, the definition of the `IFUNC_NCOND(N)' and
`IFUNC_COND_<n>' macros in host-config.h can now be guarded behind
these new file-specific macros, which ultimately control what the
`GEN_SELECTOR(X)' macro in `libatomic_i.h' expands to.  As both of
these headers are imported once per file implementing some atomic
operation, fine-tuned control is now possible.

Regtested with both `--enable-gnu-indirect-function' and
`--disable-gnu-indirect-function' configurations on armv9.4-a target
with LRCPC3 and LSE128 support and without.

Victor Do Nascimento (4):
  Libatomic: AArch64: Convert all lse128 assembly to .insn directives
  Libatomic: Define per-file identifier macros
  Libatomic: Make ifunc selector behavior contingent on importing file
  Libatomic: Clean up AArch64 `atomic_16.S' implementation file

 libatomic/acinclude.m4                       |  18 -
 libatomic/auto-config.h.in                   |   3 -
 libatomic/cas_n.c                            |   2 +
 libatomic/config/linux/aarch64/atomic_16.S   | 511 +++++++++----------
 libatomic/config/linux/aarch64/host-config.h |  35 +-
 libatomic/configure                          |  43 --
 libatomic/configure.ac                       |   3 -
 libatomic/exch_n.c                           |   2 +
 libatomic/fadd_n.c                           |   2 +
 libatomic/fand_n.c                           |   2 +
 libatomic/fence.c                            |   2 +
 libatomic/fenv.c                             |   2 +
 libatomic/fior_n.c                           |   2 +
 libatomic/flag.c                             |   2 +
 libatomic/fnand_n.c                          |   2 +
 libatomic/fop_n.c                            |   2 +
 libatomic/fsub_n.c                           |   2 +
 libatomic/fxor_n.c                           |   2 +
 libatomic/gcas.c                             |   2 +
 libatomic/gexch.c                            |   2 +
 libatomic/glfree.c                           |   2 +
 libatomic/gload.c                            |   2 +
 libatomic/gstore.c                           |   2 +
 libatomic/load_n.c                           |   2 +
 libatomic/store_n.c                          |   2 +
 libatomic/tas_n.c                            |   2 +
 26 files changed, 303 insertions(+), 350 deletions(-)

-- 
2.34.1


             reply	other threads:[~2024-06-11 12:43 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-11 12:42 Victor Do Nascimento [this message]
2024-06-11 12:42 ` [PATCH v2 1/4] Libatomic: AArch64: Convert all lse128 assembly to .insn directives Victor Do Nascimento
2024-06-11 12:42 ` [PATCH v2 2/4] Libatomic: Define per-file identifier macros Victor Do Nascimento
2024-06-11 12:42 ` [PATCH v2 3/4] Libatomic: Make ifunc selector behavior contingent on importing file Victor Do Nascimento
2024-06-11 12:42 ` [PATCH v2 4/4] Libatomic: Clean up AArch64 `atomic_16.S' implementation file Victor Do Nascimento
2024-06-12  8:50 ` [PATCH v2 0/4] Libatomic: Cleanup ifunc selector and aliasing Richard Sandiford

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=20240611124250.1390575-1-victor.donascimento@arm.com \
    --to=victor.donascimento@arm.com \
    --cc=Richard.Earnshaw@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=richard.sandiford@arm.com \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).