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

Message ID 20231003105338.1812768-2-victor.donascimento@arm.com
State Committed
Headers
Series aarch64: standardize system register representation |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Testing passed

Commit Message

Victor Do Nascimento Oct. 3, 2023, 10:51 a.m. UTC
  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.
---
 include/opcode/aarch64.h | 1 +
 opcodes/aarch64-opc.c    | 9 ++++++++-
 opcodes/aarch64-opc.h    | 3 +++
 3 files changed, 12 insertions(+), 1 deletion(-)
  

Comments

Richard Earnshaw (lists) Oct. 3, 2023, 1:40 p.m. UTC | #1
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
  

Patch

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