From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 118323 invoked by alias); 13 May 2019 13:42:05 -0000 Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org Received: (qmail 116592 invoked by uid 89); 13 May 2019 13:41:40 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 spammy=appearance X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 13 May 2019 13:41:37 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 5541E80D; Mon, 13 May 2019 06:41:36 -0700 (PDT) Received: from [10.2.207.62] (e107157-lin.cambridge.arm.com [10.2.207.62]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 837453F71E; Mon, 13 May 2019 06:41:35 -0700 (PDT) Subject: Re: [PATCH 2/57][Arm][GAS] Add support for MVE instructions: vpst, vadd, vsub and vabd To: Nick Clifton , binutils@sourceware.org Cc: Richard Earnshaw References: <19569550-4d2e-0bb3-592a-d91050d490f6@arm.com> <70aab06d-34c1-476d-e760-59ac05f645df@arm.com> <632b1db8-acee-6152-1720-a6a19ff6bb81@redhat.com> From: "Andre Vieira (lists)" Message-ID: <953a3022-7a11-bfc8-4515-25a4778315a3@arm.com> Date: Mon, 13 May 2019 13:42:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <632b1db8-acee-6152-1720-a6a19ff6bb81@redhat.com> Content-Type: multipart/mixed; boundary="------------750F84A3BD242DE56166771F" X-IsSubscribed: yes X-SW-Source: 2019-05/txt/msg00177.txt.bz2 This is a multi-part message in MIME format. --------------750F84A3BD242DE56166771F Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2418 Hi, After Nick's comments I decided to clean up the definition and uses of check_simd_pred_availability. I hope it makes the function clearer now. This is to be applied on top of the MVE series (was easier and cleaner than rebasing everything). Is this OK? Cheers, Andre gas/ChangeLog 2019-05-13 Andre Vieira * config/tc-arm.c (check_simd_pred_availability): Refactor. (do_neon_dyadic_i_su): Refactor use of check_simd_pred_availability. (do_neon_dyadic_i64_su): Likewise. (do_neon_shl): Likewise. (do_neon_qshl): Likewise. (do_neon_rshl): Likewise. (do_neon_logic): Likewise. (do_neon_dyadic_if_su): Likewise. (do_neon_addsub_if_i): Likewise. (do_neon_mac_maybe_scalar): Likewise. (do_neon_fmac): Likewise. (do_neon_mul): Likewise. (do_neon_qdmulh): Likewise. (do_neon_qrdmlah): Likewise. (do_neon_abs_neg): Likewise. (do_neon_sli): Likewise. (do_neon_sri): Likewise. (do_neon_qshlu_imm): Likewise. (do_neon_cvt_1): Likewise. (do_neon_cvttb_1): Likewise. (do_neon_mvn): Likewise. (do_neon_rev): Likewise. (do_neon_dup): Likewise. (do_neon_mov): Likewise. (do_neon_rshift_round_imm): Likewise. (do_neon_sat_abs_neg): Likewise. (do_neon_cls): Likewise. (do_neon_clz): Likewise. (do_vmaxnm): Likewise. (do_vrint_1): Likewise. (do_vcmla): Likewise. (do_vcadd): Likewise. On 02/05/2019 11:56, Nick Clifton wrote: > Hi Andre, > >> This patch adds most of the framework used by the rest of the GAS patches for MVE. > > I noticed that this function: > >> +static int >> +check_simd_pred_availability (int fp, unsigned check) > > returns an integer value, but it is only ever used in boolean > tests. IMHO it should either have a bfd_boolean return type, > or else an enum with the return values having textual names to > indicate their meaning. > > I also saw that in do_neon_logic() there is a test against > the function returning FAIL: > > if (rs == NS_QQQ > && check_simd_pred_availability (0, NEON_CHECK_ARCH | NEON_CHECK_CC) > == FAIL) > > But FAIL is not one of the values explicitly returned by > check_simd_pred_availability().... > > Cheers > Nick > > --------------750F84A3BD242DE56166771F Content-Type: text/x-patch; name="refactor.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="refactor.patch" Content-length: 13398 diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c index 20db5d9b278ccda70ac266f7df9f18e87cc430ea..6ba5e735cb78ec0e6f9d8ea1d93dc4a993a1a9a3 100644 --- a/gas/config/tc-arm.c +++ b/gas/config/tc-arm.c @@ -16562,7 +16562,13 @@ if (!thumb_mode && (check & NEON_CHECK_CC)) return SUCCESS; } -static int + +/* Return TRUE if the SIMD instruction is available for the current + cpu_variant. FP is set to TRUE if this is a SIMD floating-point + instruction. CHECK contains th. CHECK contains the set of bits to pass to + vfp_or_neon_is_neon for the NEON specific checks. */ + +static bfd_boolean check_simd_pred_availability (int fp, unsigned check) { if (inst.cond > COND_ALWAYS) @@ -16570,7 +16576,7 @@ if (inst.cond > COND_ALWAYS) if (!ARM_CPU_HAS_FEATURE (cpu_variant, mve_ext)) { inst.error = BAD_FPU; - return 1; + return FALSE; } inst.pred_insn_type = INSIDE_VPT_INSN; } @@ -16579,18 +16585,18 @@ else if (inst.cond < COND_ALWAYS) if (ARM_CPU_HAS_FEATURE (cpu_variant, mve_ext)) inst.pred_insn_type = MVE_OUTSIDE_PRED_INSN; else if (vfp_or_neon_is_neon (check) == FAIL) - return 2; + return FALSE; } else { if (!ARM_CPU_HAS_FEATURE (cpu_variant, fp ? mve_fp_ext : mve_ext) && vfp_or_neon_is_neon (check) == FAIL) - return 3; + return FALSE; if (ARM_CPU_HAS_FEATURE (cpu_variant, mve_ext)) inst.pred_insn_type = MVE_OUTSIDE_PRED_INSN; } -return 0; +return TRUE; } /* Neon instruction encoders, in approximate order of appearance. */ @@ -16598,7 +16604,7 @@ return 0; static void do_neon_dyadic_i_su (void) { - if (check_simd_pred_availability (0, NEON_CHECK_ARCH | NEON_CHECK_CC)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_ARCH | NEON_CHECK_CC)) return; enum neon_shape rs; @@ -16620,7 +16626,7 @@ do_neon_dyadic_i_su (void) static void do_neon_dyadic_i64_su (void) { - if (check_simd_pred_availability (0, NEON_CHECK_CC | NEON_CHECK_ARCH)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_CC | NEON_CHECK_ARCH)) return; enum neon_shape rs; struct neon_type_el et; @@ -16662,7 +16668,7 @@ neon_imm_shift (int write_ubit, int uval, int isquad, struct neon_type_el et, static void do_neon_shl (void) { - if (check_simd_pred_availability (0, NEON_CHECK_ARCH | NEON_CHECK_CC)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_ARCH | NEON_CHECK_CC)) return; if (!inst.operands[2].isreg) @@ -16742,7 +16748,7 @@ do_neon_shl (void) static void do_neon_qshl (void) { - if (check_simd_pred_availability (0, NEON_CHECK_ARCH | NEON_CHECK_CC)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_ARCH | NEON_CHECK_CC)) return; if (!inst.operands[2].isreg) @@ -16816,7 +16822,7 @@ do_neon_qshl (void) static void do_neon_rshl (void) { - if (check_simd_pred_availability (0, NEON_CHECK_ARCH | NEON_CHECK_CC)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_ARCH | NEON_CHECK_CC)) return; enum neon_shape rs; @@ -16930,8 +16936,8 @@ do_neon_logic (void) { enum neon_shape rs = neon_select_shape (NS_DDD, NS_QQQ, NS_NULL); if (rs == NS_QQQ - && check_simd_pred_availability (0, NEON_CHECK_ARCH | NEON_CHECK_CC) - == FAIL) + && !check_simd_pred_availability (FALSE, + NEON_CHECK_ARCH | NEON_CHECK_CC)) return; else if (rs != NS_QQQ && !ARM_CPU_HAS_FEATURE (cpu_variant, fpu_neon_ext_v1)) @@ -16953,8 +16959,8 @@ do_neon_logic (void) /* Because neon_select_shape makes the second operand a copy of the first if the second operand is not present. */ if (rs == NS_QQI - && check_simd_pred_availability (0, NEON_CHECK_ARCH | NEON_CHECK_CC) - == FAIL) + && !check_simd_pred_availability (FALSE, + NEON_CHECK_ARCH | NEON_CHECK_CC)) return; else if (rs != NS_QQI && !ARM_CPU_HAS_FEATURE (cpu_variant, fpu_neon_ext_v1)) @@ -17397,8 +17403,8 @@ do_neon_dyadic_if_su (void) && et.type == NT_float && !ARM_CPU_HAS_FEATURE (cpu_variant,fpu_neon_ext_v1), BAD_FPU); - if (check_simd_pred_availability (et.type == NT_float, - NEON_CHECK_ARCH | NEON_CHECK_CC)) + if (!check_simd_pred_availability (et.type == NT_float, + NEON_CHECK_ARCH | NEON_CHECK_CC)) return; neon_dyadic_misc (NT_unsigned, N_SUF_32, 0); @@ -17422,8 +17428,8 @@ do_neon_addsub_if_i (void) they are predicated or not. */ if ((rs == NS_QQQ || rs == NS_QQR) && et.size != 64) { - if (check_simd_pred_availability (et.type == NT_float, - NEON_CHECK_ARCH | NEON_CHECK_CC)) + if (!check_simd_pred_availability (et.type == NT_float, + NEON_CHECK_ARCH | NEON_CHECK_CC)) return; } else @@ -17584,7 +17590,7 @@ do_neon_mac_maybe_scalar (void) if (try_vfp_nsyn (3, do_vfp_nsyn_mla_mls) == SUCCESS) return; - if (check_simd_pred_availability (0, NEON_CHECK_CC | NEON_CHECK_ARCH)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_CC | NEON_CHECK_ARCH)) return; if (inst.operands[2].isscalar) @@ -17621,7 +17627,7 @@ do_neon_fmac (void) && try_vfp_nsyn (3, do_vfp_nsyn_fma_fms) == SUCCESS) return; - if (check_simd_pred_availability (1, NEON_CHECK_CC | NEON_CHECK_ARCH)) + if (!check_simd_pred_availability (TRUE, NEON_CHECK_CC | NEON_CHECK_ARCH)) return; if (ARM_CPU_HAS_FEATURE (cpu_variant, mve_fp_ext)) @@ -17675,7 +17681,7 @@ do_neon_mul (void) if (try_vfp_nsyn (3, do_vfp_nsyn_mul) == SUCCESS) return; - if (check_simd_pred_availability (0, NEON_CHECK_CC | NEON_CHECK_ARCH)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_CC | NEON_CHECK_ARCH)) return; if (inst.operands[2].isscalar) @@ -17708,7 +17714,7 @@ do_neon_mul (void) static void do_neon_qdmulh (void) { - if (check_simd_pred_availability (0, NEON_CHECK_ARCH | NEON_CHECK_CC)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_ARCH | NEON_CHECK_CC)) return; if (inst.operands[2].isscalar) @@ -18145,7 +18151,7 @@ do_mve_vmaxv (void) static void do_neon_qrdmlah (void) { - if (check_simd_pred_availability (0, NEON_CHECK_ARCH | NEON_CHECK_CC)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_ARCH | NEON_CHECK_CC)) return; if (!ARM_CPU_HAS_FEATURE (cpu_variant, mve_ext)) { @@ -18225,8 +18231,8 @@ do_neon_abs_neg (void) rs = neon_select_shape (NS_DD, NS_QQ, NS_NULL); et = neon_check_type (2, rs, N_EQK, N_S_32 | N_F_16_32 | N_KEY); - if (check_simd_pred_availability (et.type == NT_float, - NEON_CHECK_ARCH | NEON_CHECK_CC)) + if (!check_simd_pred_availability (et.type == NT_float, + NEON_CHECK_ARCH | NEON_CHECK_CC)) return; inst.instruction |= LOW4 (inst.operands[0].reg) << 12; @@ -18243,7 +18249,7 @@ do_neon_abs_neg (void) static void do_neon_sli (void) { - if (check_simd_pred_availability (0, NEON_CHECK_ARCH | NEON_CHECK_CC)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_ARCH | NEON_CHECK_CC)) return; enum neon_shape rs; @@ -18269,7 +18275,7 @@ do_neon_sli (void) static void do_neon_sri (void) { - if (check_simd_pred_availability (0, NEON_CHECK_ARCH | NEON_CHECK_CC)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_ARCH | NEON_CHECK_CC)) return; enum neon_shape rs; @@ -18294,7 +18300,7 @@ do_neon_sri (void) static void do_neon_qshlu_imm (void) { - if (check_simd_pred_availability (0, NEON_CHECK_ARCH | NEON_CHECK_CC)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_ARCH | NEON_CHECK_CC)) return; enum neon_shape rs; @@ -18767,7 +18773,8 @@ do_neon_cvt_1 (enum neon_cvt_mode mode) || flavour == neon_cvt_flavour_s32_f32 || flavour == neon_cvt_flavour_u32_f32)) { - if (check_simd_pred_availability (1, NEON_CHECK_CC | NEON_CHECK_ARCH)) + if (!check_simd_pred_availability (TRUE, + NEON_CHECK_CC | NEON_CHECK_ARCH)) return; } else if (mode == neon_cvt_mode_n) @@ -18854,8 +18861,8 @@ do_neon_cvt_1 (enum neon_cvt_mode mode) || flavour == neon_cvt_flavour_s32_f32 || flavour == neon_cvt_flavour_u32_f32)) { - if (check_simd_pred_availability (1, - NEON_CHECK_CC | NEON_CHECK_ARCH8)) + if (!check_simd_pred_availability (TRUE, + NEON_CHECK_CC | NEON_CHECK_ARCH8)) return; } else if (mode == neon_cvt_mode_z @@ -18868,8 +18875,8 @@ do_neon_cvt_1 (enum neon_cvt_mode mode) || flavour == neon_cvt_flavour_s32_f32 || flavour == neon_cvt_flavour_u32_f32)) { - if (check_simd_pred_availability (1, - NEON_CHECK_CC | NEON_CHECK_ARCH)) + if (!check_simd_pred_availability (TRUE, + NEON_CHECK_CC | NEON_CHECK_ARCH)) return; } /* fall through. */ @@ -18878,8 +18885,8 @@ do_neon_cvt_1 (enum neon_cvt_mode mode) { NEON_ENCODE (FLOAT, inst); - if (check_simd_pred_availability (1, - NEON_CHECK_CC | NEON_CHECK_ARCH8)) + if (!check_simd_pred_availability (TRUE, + NEON_CHECK_CC | NEON_CHECK_ARCH8)) return; inst.instruction |= LOW4 (inst.operands[0].reg) << 12; @@ -19039,7 +19046,7 @@ do_neon_cvttb_1 (bfd_boolean t) else if (rs == NS_QQ || rs == NS_QQI) { int single_to_half = 0; - if (check_simd_pred_availability (1, NEON_CHECK_ARCH)) + if (!check_simd_pred_availability (TRUE, NEON_CHECK_ARCH)) return; enum neon_cvt_flavour flavour = get_neon_cvt_flavour (rs); @@ -19179,7 +19186,7 @@ neon_move_immediate (void) static void do_neon_mvn (void) { - if (check_simd_pred_availability (0, NEON_CHECK_CC | NEON_CHECK_ARCH)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_CC | NEON_CHECK_ARCH)) return; if (inst.operands[1].isreg) @@ -19523,7 +19530,7 @@ do_neon_ext (void) static void do_neon_rev (void) { - if (check_simd_pred_availability (0, NEON_CHECK_ARCH | NEON_CHECK_CC)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_ARCH | NEON_CHECK_CC)) return; enum neon_shape rs; @@ -19588,7 +19595,7 @@ do_neon_dup (void) N_8 | N_16 | N_32 | N_KEY, N_EQK); if (rs == NS_QR) { - if (check_simd_pred_availability (0, NEON_CHECK_ARCH)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_ARCH)) return; } else @@ -19754,7 +19761,8 @@ do_neon_mov (void) case NS_QQ: /* case 0/1. */ { - if (check_simd_pred_availability (0, NEON_CHECK_CC | NEON_CHECK_ARCH)) + if (!check_simd_pred_availability (FALSE, + NEON_CHECK_CC | NEON_CHECK_ARCH)) return; /* The architecture manual I have doesn't explicitly state which value the U bit should have for register->register moves, but @@ -19784,7 +19792,8 @@ do_neon_mov (void) /* fall through. */ case NS_QI: /* case 2/3. */ - if (check_simd_pred_availability (0, NEON_CHECK_CC | NEON_CHECK_ARCH)) + if (!check_simd_pred_availability (FALSE, + NEON_CHECK_CC | NEON_CHECK_ARCH)) return; inst.instruction = 0x0800010; neon_move_immediate (); @@ -20089,7 +20098,7 @@ do_mve_movl (void) static void do_neon_rshift_round_imm (void) { - if (check_simd_pred_availability (0, NEON_CHECK_ARCH | NEON_CHECK_CC)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_ARCH | NEON_CHECK_CC)) return; enum neon_shape rs; @@ -20186,7 +20195,7 @@ do_neon_zip_uzp (void) static void do_neon_sat_abs_neg (void) { - if (check_simd_pred_availability (0, NEON_CHECK_CC | NEON_CHECK_ARCH)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_CC | NEON_CHECK_ARCH)) return; enum neon_shape rs; @@ -20222,7 +20231,7 @@ do_neon_recip_est (void) static void do_neon_cls (void) { - if (check_simd_pred_availability (0, NEON_CHECK_ARCH | NEON_CHECK_CC)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_ARCH | NEON_CHECK_CC)) return; enum neon_shape rs; @@ -20239,7 +20248,7 @@ do_neon_cls (void) static void do_neon_clz (void) { - if (check_simd_pred_availability (0, NEON_CHECK_ARCH | NEON_CHECK_CC)) + if (!check_simd_pred_availability (FALSE, NEON_CHECK_ARCH | NEON_CHECK_CC)) return; enum neon_shape rs; @@ -20792,7 +20801,7 @@ do_vmaxnm (void) if (try_vfp_nsyn (3, do_vfp_nsyn_fpv8) == SUCCESS) return; - if (check_simd_pred_availability (1, NEON_CHECK_CC | NEON_CHECK_ARCH8)) + if (!check_simd_pred_availability (TRUE, NEON_CHECK_CC | NEON_CHECK_ARCH8)) return; neon_dyadic_misc (NT_untyped, N_F_16_32, 0); @@ -20856,7 +20865,8 @@ do_vrint_1 (enum neon_cvt_mode mode) if (et.type == NT_invtype) return; - if (check_simd_pred_availability (1, NEON_CHECK_CC | NEON_CHECK_ARCH8)) + if (!check_simd_pred_availability (TRUE, + NEON_CHECK_CC | NEON_CHECK_ARCH8)) return; NEON_ENCODE (FLOAT, inst); @@ -20959,7 +20969,8 @@ do_vcmla (void) _("immediate out of range")); rot /= 90; - if (check_simd_pred_availability (1, NEON_CHECK_ARCH8 | NEON_CHECK_CC)) + if (!check_simd_pred_availability (TRUE, + NEON_CHECK_ARCH8 | NEON_CHECK_CC)) return; if (inst.operands[2].isscalar) @@ -21036,8 +21047,8 @@ do_vcadd (void) if (et.type == NT_invtype) return; - if (check_simd_pred_availability (et.type == NT_float, NEON_CHECK_ARCH8 - | NEON_CHECK_CC)) + if (!check_simd_pred_availability (et.type == NT_float, + NEON_CHECK_ARCH8 | NEON_CHECK_CC)) return; if (et.type == NT_float) --------------750F84A3BD242DE56166771F--