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 CD3953858D1E for ; Tue, 7 Nov 2023 23:14:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CD3953858D1E 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 CD3953858D1E 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=1699398875; cv=none; b=p5+JCFJ7VVvlzdsoQTmNPOTqA3J5Q+4/k+vIIKxcMo7lt+LgWUqzH2ltmUYKhBvM+ar7mTlcGkppE17ObXPJWiPBqLMzlX2VPKFLz2XC8e95L6VA84WY5uuEjBdH4JHPUvbUQqRKToq/WOshK8SC8qcSh8SW0goqBDnjmbwihGM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699398875; c=relaxed/simple; bh=WIxVmPJFa4wwUgCdYpA9Ko8+vjNSQcc8OcNJvjV3XIg=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=b1GxIs9XhmA0agh7usXMYRXiVAW3hvvSoKXGOHHOgM2FheRtNB7kMNyeoMGCJBMk1f16flDGuH3G2peimhU2YQLCAIMFRmrwdOvoTa4cyV+0KzPHaSYkv73Jfn5aBMeNylxJjEO3d3NL3yJhtj7YnzRZWpJ0RasD8aPJKqzeaQI= 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 1A49B1476; Tue, 7 Nov 2023 15:15:18 -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 D31123F64C; Tue, 7 Nov 2023 15:14:32 -0800 (PST) From: Richard Sandiford To: Victor Do Nascimento Mail-Followup-To: Victor Do Nascimento ,, , , richard.sandiford@arm.com Cc: , , Subject: Re: [PATCH 4/5] aarch64: Implement 128-bit extension to ACLE sysreg r/w builtins References: <20231107103211.2837188-1-victor.donascimento@arm.com> <20231107103211.2837188-5-victor.donascimento@arm.com> Date: Tue, 07 Nov 2023 23:14:31 +0000 In-Reply-To: <20231107103211.2837188-5-victor.donascimento@arm.com> (Victor Do Nascimento's message of "Tue, 7 Nov 2023 10:30:13 +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=-23.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: > Implement the ACLE builtins for 128-bit system register manipulation: > > * __uint128_t __arm_rsr128(const char *special_register); > * void __arm_wsr128(const char *special_register, __uint128_t value); > > gcc/ChangeLog: > > * config/aarch64/aarch64-builtins.cc (AARCH64_RSR128): New > `enum aarch64_builtins' value. > (AARCH64_WSR128): Likewise. > (aarch64_init_rwsr_builtins): Init `__builtin_aarch64_rsr128' > and `__builtin_aarch64_wsr128' builtins. > (aarch64_expand_rwsr_builtin): Extend function to handle > `__builtin_aarch64_{rsr|wsr}128'. > * config/aarch64/aarch64-protos.h (aarch64_retrieve_sysreg): > Update function signature. > * config/aarch64/aarch64.cc (F_REG_128): New. > (aarch64_retrieve_sysreg): Add 128-bit register mode check. > * config/aarch64/aarch64.md (UNSPEC_SYSREG_RTI): New. > (UNSPEC_SYSREG_WTI): Likewise. > (aarch64_read_sysregti): Likewise. > (aarch64_write_sysregti): Likewise. > --- > gcc/config/aarch64/aarch64-builtins.cc | 50 +++++++++++++++++++++----- > gcc/config/aarch64/aarch64-protos.h | 2 +- > gcc/config/aarch64/aarch64.cc | 6 +++- > gcc/config/aarch64/aarch64.md | 18 ++++++++++ > gcc/config/aarch64/arm_acle.h | 11 ++++++ > 5 files changed, 77 insertions(+), 10 deletions(-) > > diff --git a/gcc/config/aarch64/aarch64-builtins.cc b/gcc/config/aarch64/aarch64-builtins.cc > index c5f20f68bca..40d3788b5e0 100644 > --- a/gcc/config/aarch64/aarch64-builtins.cc > +++ b/gcc/config/aarch64/aarch64-builtins.cc > @@ -815,11 +815,13 @@ enum aarch64_builtins > AARCH64_RSR64, > AARCH64_RSRF, > AARCH64_RSRF64, > + AARCH64_RSR128, > AARCH64_WSR, > AARCH64_WSRP, > AARCH64_WSR64, > AARCH64_WSRF, > AARCH64_WSRF64, > + AARCH64_WSR128, > AARCH64_BUILTIN_MAX > }; > > @@ -1842,6 +1844,10 @@ aarch64_init_rwsr_builtins (void) > = build_function_type_list (double_type_node, const_char_ptr_type, NULL); > AARCH64_INIT_RWSR_BUILTINS_DECL (RSRF64, rsrf64, fntype); > > + fntype > + = build_function_type_list (uint128_type_node, const_char_ptr_type, NULL); > + AARCH64_INIT_RWSR_BUILTINS_DECL (RSR128, rsr128, fntype); > + > fntype > = build_function_type_list (void_type_node, const_char_ptr_type, > uint32_type_node, NULL); > @@ -1867,6 +1873,12 @@ aarch64_init_rwsr_builtins (void) > = build_function_type_list (void_type_node, const_char_ptr_type, > double_type_node, NULL); > AARCH64_INIT_RWSR_BUILTINS_DECL (WSRF64, wsrf64, fntype); > + > + fntype > + = build_function_type_list (void_type_node, const_char_ptr_type, > + uint128_type_node, NULL); > + AARCH64_INIT_RWSR_BUILTINS_DECL (WSR128, wsr128, fntype); > + > } > > /* Initialize the memory tagging extension (MTE) builtins. */ > @@ -2710,6 +2722,7 @@ aarch64_expand_rwsr_builtin (tree exp, rtx target, int fcode) > tree arg0, arg1; > rtx const_str, input_val, subreg; > enum machine_mode mode; > + enum insn_code icode; > class expand_operand ops[2]; > > arg0 = CALL_EXPR_ARG (exp, 0); > @@ -2718,7 +2731,18 @@ aarch64_expand_rwsr_builtin (tree exp, rtx target, int fcode) > || fcode == AARCH64_WSRP > || fcode == AARCH64_WSR64 > || fcode == AARCH64_WSRF > - || fcode == AARCH64_WSRF64); > + || fcode == AARCH64_WSRF64 > + || fcode == AARCH64_WSR128); > + > + bool op128 = (fcode == AARCH64_RSR128 || fcode == AARCH64_WSR128); > + enum machine_mode sysreg_mode = op128 ? TImode : DImode; > + > + if (op128 && !TARGET_D128) > + { > + error_at (EXPR_LOCATION (exp), "128-bit system register suppport requires " > + "the +d128 Armv9.4-A extension"); Elsewhere we've put feature names in quotes, since they're code or code-adjacent. Probably also best to drop Armv9.4-A part, since the requirement is tied only to +d128. So: error_at (EXPR_LOCATION (exp), "128-bit system register suppport requires the %" " extension"); (formatted that way to avoid a long line). > + return const0_rtx; > + } > > /* Argument 0 (system register name) must be a string literal. */ > gcc_assert (TREE_CODE (arg0) == ADDR_EXPR > @@ -2741,7 +2765,7 @@ aarch64_expand_rwsr_builtin (tree exp, rtx target, int fcode) > sysreg_name[pos] = TOLOWER (sysreg_name[pos]); > > const char* name_output = aarch64_retrieve_sysreg ((const char *) sysreg_name, > - write_op); > + write_op, op128); > if (name_output == NULL) > { > error_at (EXPR_LOCATION (exp), "invalid system register name provided"); > @@ -2760,13 +2784,17 @@ aarch64_expand_rwsr_builtin (tree exp, rtx target, int fcode) > mode = TYPE_MODE (TREE_TYPE (arg1)); > input_val = copy_to_mode_reg (mode, expand_normal (arg1)); > > + icode = (op128 ? CODE_FOR_aarch64_write_sysregti > + : CODE_FOR_aarch64_write_sysregdi); > + > switch (fcode) > { > case AARCH64_WSR: > case AARCH64_WSRP: > case AARCH64_WSR64: > case AARCH64_WSRF64: > - subreg = lowpart_subreg (DImode, input_val, mode); > + case AARCH64_WSR128: > + subreg = lowpart_subreg (sysreg_mode, input_val, mode); > break; > case AARCH64_WSRF: > subreg = gen_lowpart_SUBREG (SImode, input_val); > @@ -2775,8 +2803,8 @@ aarch64_expand_rwsr_builtin (tree exp, rtx target, int fcode) > } > > create_fixed_operand (&ops[0], const_str); > - create_input_operand (&ops[1], subreg, DImode); > - expand_insn (CODE_FOR_aarch64_write_sysregdi, 2, ops); > + create_input_operand (&ops[1], subreg, sysreg_mode); > + expand_insn (icode, 2, ops); > > return target; > } > @@ -2784,10 +2812,13 @@ aarch64_expand_rwsr_builtin (tree exp, rtx target, int fcode) > /* Read operations are implied by !write_op. */ > gcc_assert (call_expr_nargs (exp) == 1); > > + icode = (op128 ? CODE_FOR_aarch64_read_sysregti > + : CODE_FOR_aarch64_read_sysregdi); > + > /* Emit the initial read_sysregdi rtx. */ > - create_output_operand (&ops[0], target, DImode); > + create_output_operand (&ops[0], target, sysreg_mode); > create_fixed_operand (&ops[1], const_str); > - expand_insn (CODE_FOR_aarch64_read_sysregdi, 2, ops); > + expand_insn (icode, 2, ops); > target = ops[0].value; > > /* Do any necessary post-processing on the result. */ > @@ -2797,7 +2828,8 @@ aarch64_expand_rwsr_builtin (tree exp, rtx target, int fcode) > case AARCH64_RSRP: > case AARCH64_RSR64: > case AARCH64_RSRF64: > - return lowpart_subreg (TYPE_MODE (TREE_TYPE (exp)), target, DImode); > + case AARCH64_RSR128: > + return lowpart_subreg (TYPE_MODE (TREE_TYPE (exp)), target, sysreg_mode); > case AARCH64_RSRF: > subreg = gen_lowpart_SUBREG (SImode, target); > return gen_lowpart_SUBREG (SFmode, subreg); > @@ -3048,11 +3080,13 @@ aarch64_general_expand_builtin (unsigned int fcode, tree exp, rtx target, > case AARCH64_RSR64: > case AARCH64_RSRF: > case AARCH64_RSRF64: > + case AARCH64_RSR128: > case AARCH64_WSR: > case AARCH64_WSRP: > case AARCH64_WSR64: > case AARCH64_WSRF: > case AARCH64_WSRF64: > + case AARCH64_WSR128: > return aarch64_expand_rwsr_builtin (exp, target, fcode); > } > > diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h > index dbd486cfea4..6a306134c2d 100644 > --- a/gcc/config/aarch64/aarch64-protos.h > +++ b/gcc/config/aarch64/aarch64-protos.h > @@ -831,7 +831,7 @@ bool aarch64_sve_ptrue_svpattern_p (rtx, struct simd_immediate_info *); > bool aarch64_simd_valid_immediate (rtx, struct simd_immediate_info *, > enum simd_immediate_check w = AARCH64_CHECK_MOV); > bool aarch64_valid_sysreg_name_p (const char *); > -const char *aarch64_retrieve_sysreg (const char *, bool); > +const char *aarch64_retrieve_sysreg (const char *, bool, bool); > rtx aarch64_check_zero_based_sve_index_immediate (rtx); > bool aarch64_sve_index_immediate_p (rtx); > bool aarch64_sve_arith_immediate_p (machine_mode, rtx, bool); > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > index c0d75f167be..ff7e75e1f19 100644 > --- a/gcc/config/aarch64/aarch64.cc > +++ b/gcc/config/aarch64/aarch64.cc > @@ -2846,6 +2846,8 @@ typedef struct { > #define F_ARCHEXT (1 << 4) > /* Flag indicating register name is alias for another system register. */ > #define F_REG_ALIAS (1 << 5) > +/* Flag indicatinig registers which may be implemented with 128-bits. */ > +#define F_REG_128 (1 << 6) > > /* Database of system registers, their encodings and architectural > requirements. */ > @@ -28245,7 +28247,7 @@ aarch64_valid_sysreg_name_p (const char *regname) > name, otherwise NULL. WRITE_P is true iff the register is being > written to. */ > const char * > -aarch64_retrieve_sysreg (const char *regname, bool write_p) > +aarch64_retrieve_sysreg (const char *regname, bool write_p, bool is128op) > { > const sysreg_t *sysreg = aarch64_lookup_sysreg_map (regname); > if (sysreg == NULL) The comment should describe the new parameter. Looks good otherwise. Thanks, Richard > @@ -28255,6 +28257,8 @@ aarch64_retrieve_sysreg (const char *regname, bool write_p) > else > return NULL; > } > + if (is128op && !(sysreg->properties & F_REG_128)) > + return NULL; > if ((write_p && (sysreg->properties & F_REG_READ)) > || (!write_p && (sysreg->properties & F_REG_WRITE))) > return NULL; > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md > index aee8f8ad65a..3be813efcd4 100644 > --- a/gcc/config/aarch64/aarch64.md > +++ b/gcc/config/aarch64/aarch64.md > @@ -282,7 +282,9 @@ > UNSPEC_RDFFR > UNSPEC_WRFFR > UNSPEC_SYSREG_RDI > + UNSPEC_SYSREG_RTI > UNSPEC_SYSREG_WDI > + UNSPEC_SYSREG_WTI > ;; Represents an SVE-style lane index, in which the indexing applies > ;; within the containing 128-bit block. > UNSPEC_SVE_LANE_SELECT > @@ -486,6 +488,14 @@ > "mrs\t%x0, %1" > ) > > +(define_insn "aarch64_read_sysregti" > + [(set (match_operand:TI 0 "register_operand" "=r") > + (unspec_volatile:TI [(match_operand 1 "aarch64_sysreg_string" "")] > + UNSPEC_SYSREG_RTI))] > + "TARGET_D128" > + "mrrs\t%x0, %H0, %x1" > +) > + > (define_insn "aarch64_write_sysregdi" > [(unspec_volatile:DI [(match_operand 0 "aarch64_sysreg_string" "") > (match_operand:DI 1 "register_operand" "rZ")] > @@ -494,6 +504,14 @@ > "msr\t%0, %x1" > ) > > +(define_insn "aarch64_write_sysregti" > + [(unspec_volatile:TI [(match_operand 0 "aarch64_sysreg_string" "") > + (match_operand:TI 1 "register_operand" "r")] > + UNSPEC_SYSREG_WTI)] > + "TARGET_D128" > + "msrr\t%x0, %x1, %H1" > +) > + > (define_insn "indirect_jump" > [(set (pc) (match_operand:DI 0 "register_operand" "r"))] > "" > diff --git a/gcc/config/aarch64/arm_acle.h b/gcc/config/aarch64/arm_acle.h > index 71ada878299..80282b361a4 100644 > --- a/gcc/config/aarch64/arm_acle.h > +++ b/gcc/config/aarch64/arm_acle.h > @@ -344,6 +344,17 @@ __rndrrs (uint64_t *__res) > #define __arm_wsrf64(__regname, __value) \ > __builtin_aarch64_wsrf64 (__regname, __value) > > +#pragma GCC push_options > +#pragma GCC target ("+nothing+d128") > + > +#define __arm_rsr128(__regname) \ > + __builtin_aarch64_rsr128 (__regname) > + > +#define __arm_wsr128(__regname, __value) \ > + __builtin_aarch64_wsr128 (__regname, __value) > + > +#pragma GCC pop_options > + > #ifdef __cplusplus > } > #endif