From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 2EEAE3858C42 for ; Thu, 25 Jan 2024 17:17:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2EEAE3858C42 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 2EEAE3858C42 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=217.140.110.172 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706203067; cv=none; b=mmyyDhqxsl//B703/F1axwTyXjVrpdTqZ80hp86fs0c0oXy5TMEADm2IvuUigm9N7c0GRO/Iq8FfHfnAPiqT4Ws+iZAEd2n9SCx3h/GSxSaO5crt+y+rq5CNaPGksKzuDxUJKEXFvKqv1h7He2FUUwSdtOsggSXm0vVsCQYJ0F0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706203067; c=relaxed/simple; bh=kDAkOWDpFGoUn/oW9S1wQxLT6mWJgJWFsYuLOsSkSno=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=iTUi0CwKTIHgynQdY7nfWOxreuPmYBdxshNdnvbqNgYHEKqU0MCJ3/6NoD4X0DBNT7t2hugab3hYEmVfRL4nhAAb0d+qyLWgZbWpIr4iAPlDY1yUsb+GKKQOBZK9iVkVZ8u7jBjls1mdm37meAS3mo84eaa+6aKylHyRuQfaPfc= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 55D6E1FB; Thu, 25 Jan 2024 09:18:29 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 351DD3F73F; Thu, 25 Jan 2024 09:17:44 -0800 (PST) From: Richard Sandiford To: Victor Do Nascimento Mail-Followup-To: Victor Do Nascimento ,, , , richard.sandiford@arm.com Cc: , , Subject: Re: [PATCH v4 1/4] libatomic: atomic_16.S: Improve ENTRY, END and ALIAS macro interface References: <20240124171853.3112540-1-victor.donascimento@arm.com> <20240124171853.3112540-2-victor.donascimento@arm.com> Date: Thu, 25 Jan 2024 17:17:42 +0000 In-Reply-To: <20240124171853.3112540-2-victor.donascimento@arm.com> (Victor Do Nascimento's message of "Wed, 24 Jan 2024 17:17:30 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-21.2 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Victor Do Nascimento writes: > The introduction of further architectural-feature dependent ifuncs > for AArch64 makes hard-coding ifunc `_i' suffixes to functions > cumbersome to work with. It is awkward to remember which ifunc maps > onto which arch feature and makes the code harder to maintain when new > ifuncs are added and their suffixes possibly altered. > > This patch uses pre-processor `#define' statements to map each suffix to > a descriptive feature name macro, for example: > > #define LSE(NAME) NAME##_i1 > > Where we wish to generate ifunc names with the pre-processor's token > concatenation feature, we add a level of indirection to previous macro > calls. If before we would have had`MACRO(_i)', we now have > `MACRO_FEAT(name, feature)'. Where we wish to refer to base > functionality (i.e., functions where ifunc suffixes are absent), the > original `MACRO()' may be used to bypass suffixing. > > Consequently, for base functionality, where the ifunc suffix is > absent, the macro interface remains the same. For example, the entry > and endpoints of `libat_store_16' remain defined by: > > ENTRY (libat_store_16) > > and > > END (libat_store_16) > > For the LSE2 implementation of the same 16-byte atomic store, we now > have: > > ENTRY_FEAT (libat_store_16, LSE2) > > and > > END_FEAT (libat_store_16, LSE2) > > For the aliasing of function names, we define the following new > implementation of the ALIAS macro: > > ALIAS (FN_BASE_NAME, FROM_SUFFIX, TO_SUFFIX) > > Defining the `CORE(NAME)' macro to be the identity operator, it > returns the base function name unaltered and allows us to alias > target-specific ifuncs to the corresponding base implementation. > For example, we'd alias the LSE2 `libat_exchange_16' to it base > implementation with: > > ALIAS (libat_exchange_16, LSE2, CORE) > > libatomic/ChangeLog: > * config/linux/aarch64/atomic_16.S (CORE): New macro. > (LSE2): Likewise. > (ENTRY_FEAT): Likewise. > (ENTRY_FEAT1): Likewise. > (END_FEAT): Likewise. > (END_FEAT1): Likewise. > (ALIAS): Modify macro to take in `arch' arguments. > (ALIAS1): New. > --- > libatomic/config/linux/aarch64/atomic_16.S | 79 +++++++++++++--------- > 1 file changed, 47 insertions(+), 32 deletions(-) > > diff --git a/libatomic/config/linux/aarch64/atomic_16.S b/libatomic/config/linux/aarch64/atomic_16.S > index ad14f8f2e6e..16a42925903 100644 > --- a/libatomic/config/linux/aarch64/atomic_16.S > +++ b/libatomic/config/linux/aarch64/atomic_16.S > @@ -40,22 +40,38 @@ > > .arch armv8-a+lse > > -#define ENTRY(name) \ > - .global name; \ > - .hidden name; \ > - .type name,%function; \ > +#define LSE2(NAME) NAME##_i1 > +#define CORE(NAME) NAME > + > +#define ENTRY(NAME) ENTRY_FEAT1 (NAME) > + > +#define ENTRY_FEAT(NAME, FEAT) \ > + ENTRY_FEAT1 (FEAT (NAME)) > + > +#define ENTRY_FEAT1(NAME) \ > + .global NAME; \ > + .hidden NAME; \ > + .type NAME,%function; \ I don't think ENTRY_FEAT1 is necessary now. It should be possible to keep ENTRY as it was and use: #define ENTRY_FEAT(NAME, FEAT) \ ENTRY (FEAT (NAME)) Similarly for END/END_FEAT. OK with those changes, thanks. Richard > .p2align 4; \ > -name: \ > - .cfi_startproc; \ > +NAME: \ > + .cfi_startproc; \ > hint 34 // bti c > > -#define END(name) \ > +#define END(NAME) END_FEAT1 (NAME) > + > +#define END_FEAT(NAME, FEAT) \ > + END_FEAT1 (FEAT (NAME)) > + > +#define END_FEAT1(NAME) \ > .cfi_endproc; \ > - .size name, .-name; > + .size NAME, .-NAME; > + > +#define ALIAS(NAME, FROM, TO) \ > + ALIAS1 (FROM (NAME),TO (NAME)) > > -#define ALIAS(alias,name) \ > - .global alias; \ > - .set alias, name; > +#define ALIAS1(ALIAS, NAME) \ > + .global ALIAS; \ > + .set ALIAS, NAME; > > #define res0 x0 > #define res1 x1 > @@ -108,7 +124,7 @@ ENTRY (libat_load_16) > END (libat_load_16) > > > -ENTRY (libat_load_16_i1) > +ENTRY_FEAT (libat_load_16, LSE2) > cbnz w1, 1f > > /* RELAXED. */ > @@ -128,7 +144,7 @@ ENTRY (libat_load_16_i1) > ldp res0, res1, [x0] > dmb ishld > ret > -END (libat_load_16_i1) > +END_FEAT (libat_load_16, LSE2) > > > ENTRY (libat_store_16) > @@ -148,7 +164,7 @@ ENTRY (libat_store_16) > END (libat_store_16) > > > -ENTRY (libat_store_16_i1) > +ENTRY_FEAT (libat_store_16, LSE2) > cbnz w4, 1f > > /* RELAXED. */ > @@ -160,7 +176,7 @@ ENTRY (libat_store_16_i1) > stlxp w4, in0, in1, [x0] > cbnz w4, 1b > ret > -END (libat_store_16_i1) > +END_FEAT (libat_store_16, LSE2) > > > ENTRY (libat_exchange_16) > @@ -237,7 +253,7 @@ ENTRY (libat_compare_exchange_16) > END (libat_compare_exchange_16) > > > -ENTRY (libat_compare_exchange_16_i1) > +ENTRY_FEAT (libat_compare_exchange_16, LSE2) > ldp exp0, exp1, [x1] > mov tmp0, exp0 > mov tmp1, exp1 > @@ -270,7 +286,7 @@ ENTRY (libat_compare_exchange_16_i1) > /* ACQ_REL/SEQ_CST. */ > 4: caspal exp0, exp1, in0, in1, [x0] > b 0b > -END (libat_compare_exchange_16_i1) > +END_FEAT (libat_compare_exchange_16, LSE2) > > > ENTRY (libat_fetch_add_16) > @@ -556,21 +572,20 @@ END (libat_test_and_set_16) > > /* Alias entry points which are the same in baseline and LSE2. */ > > -ALIAS (libat_exchange_16_i1, libat_exchange_16) > -ALIAS (libat_fetch_add_16_i1, libat_fetch_add_16) > -ALIAS (libat_add_fetch_16_i1, libat_add_fetch_16) > -ALIAS (libat_fetch_sub_16_i1, libat_fetch_sub_16) > -ALIAS (libat_sub_fetch_16_i1, libat_sub_fetch_16) > -ALIAS (libat_fetch_or_16_i1, libat_fetch_or_16) > -ALIAS (libat_or_fetch_16_i1, libat_or_fetch_16) > -ALIAS (libat_fetch_and_16_i1, libat_fetch_and_16) > -ALIAS (libat_and_fetch_16_i1, libat_and_fetch_16) > -ALIAS (libat_fetch_xor_16_i1, libat_fetch_xor_16) > -ALIAS (libat_xor_fetch_16_i1, libat_xor_fetch_16) > -ALIAS (libat_fetch_nand_16_i1, libat_fetch_nand_16) > -ALIAS (libat_nand_fetch_16_i1, libat_nand_fetch_16) > -ALIAS (libat_test_and_set_16_i1, libat_test_and_set_16) > - > +ALIAS (libat_exchange_16, LSE2, CORE) > +ALIAS (libat_fetch_add_16, LSE2, CORE) > +ALIAS (libat_add_fetch_16, LSE2, CORE) > +ALIAS (libat_fetch_sub_16, LSE2, CORE) > +ALIAS (libat_sub_fetch_16, LSE2, CORE) > +ALIAS (libat_fetch_or_16, LSE2, CORE) > +ALIAS (libat_or_fetch_16, LSE2, CORE) > +ALIAS (libat_fetch_and_16, LSE2, CORE) > +ALIAS (libat_and_fetch_16, LSE2, CORE) > +ALIAS (libat_fetch_xor_16, LSE2, CORE) > +ALIAS (libat_xor_fetch_16, LSE2, CORE) > +ALIAS (libat_fetch_nand_16, LSE2, CORE) > +ALIAS (libat_nand_fetch_16, LSE2, CORE) > +ALIAS (libat_test_and_set_16, LSE2, CORE) > > /* GNU_PROPERTY_AARCH64_* macros from elf.h for use in asm code. */ > #define FEATURE_1_AND 0xc0000000