Re: [PATCH v2 1/2] aarch64: system register aliasing detection

["Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>]

On 03/10/2023 11:51, Victor Do Nascimento via Binutils wrote:
> This patch adds a mechanism for system register name alias detection
> to register matching mechanisms.
> 
> A new `F_REG_ALIAS' flag is added to the set of register flags and
> used to label which entries in aarch64_sys_regs[] correspond to
> aliases (and thus which CPENC values are non-unique in this array).
> 
> Where this is used is, for example, in `aarch64_print_operand' where,
> in the case of system register decoding, the aarch64_sys_regs[] array
> is iterated through until a match in CPENC value is made and the first
> match accepted.  If insufficient care is given in the ordering of
> system registers in this array, the alias is encountered before the
> "real" register and used incorrectly as the register name in the
> disassembled output.
> 
> With this flag and the new `aarch64_sys_reg_alias_p' test, search
> candidates corresponding to aliases can be conveniently skipped over.
> 
> One concrete example of where this is useful is with the
> `trcextinselr0' system register.  It was initially placed in the
> system register list before `trcextinselr', in contrast to a more
> natural alphabetical order.
> 
> include/ChangeLog:
> 	* opcode/aarch64.h: add `aarch64_sys_reg_alias_p' prototype.
> 
> opcodes/ChangeLog:
> 	* aarch64-opc.c (aarch64_sys_reg_alias_p): New.
> 	(aarch64_print_operand): add aarch64_sys_reg_alias_p check.
> 	(aarch64_sys_regs): Add F_REG_ALIAS flag to "trcextinselr"
> 	entry.
> 	* aarch64-opc.h (F_REG_ALIAS): New.

OK

Reviewed-by: Richard.Earnshaw@arm.com

R.
> ---
>  include/opcode/aarch64.h | 1 +
>  opcodes/aarch64-opc.c    | 9 ++++++++-
>  opcodes/aarch64-opc.h    | 3 +++
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/include/opcode/aarch64.h b/include/opcode/aarch64.h
> index ab42acac8c2..d4ec60c1354 100644
> --- a/include/opcode/aarch64.h
> +++ b/include/opcode/aarch64.h
> @@ -1240,6 +1240,7 @@ typedef struct
>  extern const aarch64_sys_reg aarch64_sys_regs [];
>  extern const aarch64_sys_reg aarch64_pstatefields [];
>  extern bool aarch64_sys_reg_deprecated_p (const uint32_t);
> +extern bool aarch64_sys_reg_alias_p (const uint32_t);
>  extern bool aarch64_pstatefield_supported_p (const aarch64_feature_set,
>  					     const aarch64_sys_reg *);
>  
> diff --git a/opcodes/aarch64-opc.c b/opcodes/aarch64-opc.c
> index df1b5113641..c901d3d587b 100644
> --- a/opcodes/aarch64-opc.c
> +++ b/opcodes/aarch64-opc.c
> @@ -4507,6 +4507,7 @@ aarch64_print_operand (char *buf, size_t size, bfd_vma pc,
>  	     partial match that was found.  */
>  	  if (aarch64_sys_regs[i].value == opnd->sysreg.value
>  	      && ! aarch64_sys_reg_deprecated_p (aarch64_sys_regs[i].flags)
> +	      && ! aarch64_sys_reg_alias_p (aarch64_sys_regs[i].flags)
>  	      && (name == NULL || exact_match))
>  	    {
>  	      name = aarch64_sys_regs[i].name;
> @@ -5320,7 +5321,7 @@ const aarch64_sys_reg aarch64_sys_regs [] =
>    SR_CORE ("trceventctl0r", CPENC (2,1,C0,C8,0),  0),
>    SR_CORE ("trceventctl1r", CPENC (2,1,C0,C9,0),  0),
>    SR_CORE ("trcextinselr0", CPENC (2,1,C0,C8,4),  0),
> -  SR_CORE ("trcextinselr",  CPENC (2,1,C0,C8,4),  0),
> +  SR_CORE ("trcextinselr",  CPENC (2,1,C0,C8,4),  F_REG_ALIAS),
>    SR_CORE ("trcextinselr1", CPENC (2,1,C0,C9,4),  0),
>    SR_CORE ("trcextinselr2", CPENC (2,1,C0,C10,4), 0),
>    SR_CORE ("trcextinselr3", CPENC (2,1,C0,C11,4), 0),
> @@ -5742,6 +5743,12 @@ aarch64_sys_reg_deprecated_p (const uint32_t reg_flags)
>    return (reg_flags & F_DEPRECATED) != 0;
>  }
>  
> +bool
> +aarch64_sys_reg_alias_p (const uint32_t reg_flags)
> +{
> +  return (reg_flags & F_REG_ALIAS) != 0;
> +}
> +
>  /* The CPENC below is fairly misleading, the fields
>     here are not in CPENC form. They are in op2op1 form. The fields are encoded
>     by ins_pstatefield, which just shifts the value by the width of the fields
> diff --git a/opcodes/aarch64-opc.h b/opcodes/aarch64-opc.h
> index 32e4da2bbb6..fe1f882c20e 100644
> --- a/opcodes/aarch64-opc.h
> +++ b/opcodes/aarch64-opc.h
> @@ -292,6 +292,9 @@ verify_constraints (const struct aarch64_inst *, const aarch64_insn, bfd_vma,
>  #undef F_REG_IN_CRM
>  #define F_REG_IN_CRM	(1 << 5)  /* Register extra encoding in CRm.  */
>  
> +#undef F_REG_ALIAS
> +#define F_REG_ALIAS	(1 << 6)  /* Register name aliases another.  */
> +
>  /* PSTATE field name for the MSR instruction this is encoded in "op1:op2:CRm".
>     Part of CRm can be used to encode <pstatefield>. E.g. CRm[3:1] for SME.
>     In order to set/get full PSTATE field name use flag F_REG_IN_CRM and below