Re: [PATCH 3/6] Arm64: check tied operand specifier in aarch64-gen

[Andrew Carlotti <andrew.carlotti@arm.com>]

On Fri, Feb 23, 2024 at 12:29:00PM +0100, Jan Beulich wrote:
> Make sure that field actually matches the specified operands. Don't
> follow existing F_PSEUDO checking in using assertions, though. Print
> meaingful error messages, thus - while not having a line number
> available - at least providing some indication of where things are
> wrong.

This new check should be helpful.  However, some mnemonics have a lot of
variants, so could you also add the opcode (and maybe the mask) to the new
error messages? For example:

extq (0x05602400,0xfff0fc00): operands 1 and 2 match, but tied=0

> Fix SVE2.1's extq accordingly, but don't extend the testsuite there:
> There are further issues with its operands (SVE_Zm_imm4 doesn't look to
> be correct to use there, as that describes an indexed vector register,
> while here a separate vector register and immediate operand are to be
> specified).
> 
> --- a/opcodes/aarch64-gen.c
> +++ b/opcodes/aarch64-gen.c
> @@ -129,6 +129,7 @@ read_table (const struct aarch64_opcode*
>    const struct aarch64_opcode *ent = table;
>    opcode_node **new_ent;
>    unsigned int index = initialize_index (table);
> +  unsigned int errors = 0;
>  
>    if (!ent->name)
>      return;
> @@ -140,6 +141,8 @@ read_table (const struct aarch64_opcode*
>  
>    do
>      {
> +      bool match = false;
> +
>        /* F_PSEUDO needs to be used together with F_ALIAS to indicate an alias
>  	 opcode is a programmer friendly pseudo instruction available only in
>  	 the assembly code (thus will not show up in the disassembly).  */
> @@ -150,12 +153,45 @@ read_table (const struct aarch64_opcode*
>  	  index++;
>  	  continue;
>  	}
> +
> +      /* Check tied_operand against operands[].  */
> +      for (unsigned int i = 1; i < ARRAY_SIZE (ent->operands); ++i)
> +	{
> +	  if (ent->operands[i] == AARCH64_OPND_NIL)
> +	    break;
> +
> +	  if (ent->operands[i] != ent->operands[0])
> +	    continue;
> +	  match = true;
> +
> +	  if (i != ent->tied_operand)
> +	    {
> +	      fprintf (stderr, "%s: operands 1 and %u match, but tied=%u\n",
> +		       ent->name, i + 1, ent->tied_operand);
> +	      ++errors;
> +	    }
> +	}
> +      if (!match && ent->tied_operand
> +	  /* SME LDR/STR (array vector) tie together inner immediates only.  */
> +	  && ent->iclass != sme_ldr && ent->iclass != sme_str)
> +	{
> +	  fprintf (stderr, "%s: no operands match, but tied=%u\n",
> +		   ent->name, ent->tied_operand);
> +	  ++errors;
> +	}
> +
>        *new_ent = new_opcode_node ();
>        (*new_ent)->opcode = ent->opcode;
>        (*new_ent)->mask = ent->mask;
>        (*new_ent)->index = index++;
>        new_ent = &((*new_ent)->next);
>      } while ((++ent)->name);
> +
> +  if (errors)
> +    {
> +      fprintf (stderr, "%u errors, exiting\n", errors);
> +      xexit (3);
> +    }
>  }
>  
>  static inline void
> --- a/opcodes/aarch64-tbl.h
> +++ b/opcodes/aarch64-tbl.h
> @@ -6375,7 +6375,7 @@ const struct aarch64_opcode aarch64_opco
>    SVE2p1_INSNC("fminqv",0x6417a000, 0xff3fe000, sve2_urqvs, 0, OP3 (Vd, SVE_Pg3, SVE_Zn), OP_SVE_vUS_HSD_HSD, F_OPD_SIZE, C_SCAN_MOVPRFX, 0),
>  
>    SVE2p1_INSN("dupq",0x05202400, 0xffe0fc00, sve_index1, 0, OP2 (SVE_Zd, SVE_Zn_5_INDEX), OP_SVE_VV_BHSD, 0, 0),
> -  SVE2p1_INSN("extq",0x05602400, 0xfff0fc00, sve_misc, 0, OP3 (SVE_Zd, SVE_Zd, SVE_Zm_imm4), OP_SVE_BBB, 0, 0),
> +  SVE2p1_INSN("extq",0x05602400, 0xfff0fc00, sve_misc, 0, OP3 (SVE_Zd, SVE_Zd, SVE_Zm_imm4), OP_SVE_BBB, 0, 1),
>    SVE2p1_INSNC("ld1q",0xc400a000, 0xffe0e000, sve_misc, 0, OP3 (SVE_Zt, SVE_Pg3, SVE_ADDR_ZX), OP_SVE_SZS_QD, 0, C_SCAN_MOVPRFX, 0),
>    SVE2p1_INSNC("ld2q",0xa490e000, 0xfff0e000, sve_misc, 0, OP3 (SME_Zt2, SVE_Pg3, SVE_ADDR_RI_S4x2xVL), OP_SVE_QZU, 0, C_SCAN_MOVPRFX, 0),
>    SVE2p1_INSNC("ld3q",0xa510e000, 0xfff0e000, sve_misc, 0, OP3 (SME_Zt3, SVE_Pg3, SVE_ADDR_RI_S4x2xVL), OP_SVE_QZU, 0, C_SCAN_MOVPRFX, 0),
>