This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/57][Arm][GAS] Add support for MVE instructions: vpst, vadd, vsub and vabd


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  <andre.simoesdiasvieira@arm.com>

        * 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


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)

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]