[PATCH] opcodes/aarch64: rename fields[]

Alice Carlotti alice.carlotti@arm.com
Wed Aug 6 15:46:58 GMT 2025


On Fri, Aug 01, 2025 at 09:21:47AM +0200, Jan Beulich wrote:
> To be a fair global name space citizen, give it an aarch64_ prefix. In
> two cases, drop a variable that's used only once.
> ---
> Pre-approved by Richard.

LGTM as well.

> 
> The name is still pretty unspecific, but I wasn't sure if e.g.
> aarch64_insn_fields[] would have been fully appropriate. Plus the way
> it's done the name fits with the underlying type (aarch64_field).

Consistency makes sense here, although I'll note that aarch64_insn_fields would
also be accurate (it represents segments of the 32 bit instruction encoding).

We're actually working to remove this array entirely soon - it just adds an
unhelpful layer of indirection to the code.  But that's no reason not to fix
the name space issue now.

Alice

> 
> --- a/opcodes/aarch64-asm.c
> +++ b/opcodes/aarch64-asm.c
> @@ -40,7 +40,6 @@ static inline void
>  insert_fields (aarch64_insn *code, aarch64_insn value, aarch64_insn mask, ...)
>  {
>    uint32_t num;
> -  const aarch64_field *field;
>    enum aarch64_field_kind kind;
>    va_list va;
>  
> @@ -50,9 +49,8 @@ insert_fields (aarch64_insn *code, aarch
>    while (num--)
>      {
>        kind = va_arg (va, enum aarch64_field_kind);
> -      field = &fields[kind];
>        insert_field (kind, code, value, mask);
> -      value >>= field->width;
> +      value >>= aarch64_fields[kind].width;
>      }
>    va_end (va);
>  }
> @@ -72,7 +70,7 @@ insert_all_fields_after (const aarch64_o
>        {
>  	kind = self->fields[i];
>  	insert_field (kind, code, value, 0);
> -	value >>= fields[kind].width;
> +	value >>= aarch64_fields[kind].width;
>        }
>  }
>  
> --- a/opcodes/aarch64-dis.c
> +++ b/opcodes/aarch64-dis.c
> @@ -149,7 +149,6 @@ aarch64_insn
>  extract_fields (aarch64_insn code, aarch64_insn mask, ...)
>  {
>    uint32_t num;
> -  const aarch64_field *field;
>    enum aarch64_field_kind kind;
>    va_list va;
>  
> @@ -160,8 +159,7 @@ extract_fields (aarch64_insn code, aarch
>    while (num--)
>      {
>        kind = va_arg (va, enum aarch64_field_kind);
> -      field = &fields[kind];
> -      value <<= field->width;
> +      value <<= aarch64_fields[kind].width;
>        value |= extract_field (kind, code, mask);
>      }
>    va_end (va);
> @@ -184,7 +182,7 @@ extract_all_fields_after (const aarch64_
>         i < ARRAY_SIZE (self->fields) && self->fields[i] != FLD_NIL; ++i)
>      {
>        kind = self->fields[i];
> -      value <<= fields[kind].width;
> +      value <<= aarch64_fields[kind].width;
>        value |= extract_field (kind, code, 0);
>      }
>    return value;
> @@ -1240,7 +1238,8 @@ aarch64_ext_addr_simm (const aarch64_ope
>    info->addr.base_regno = extract_field (FLD_Rn, code, 0);
>    /* simm (imm9 or imm7)  */
>    imm = extract_field (self->fields[0], code, 0);
> -  info->addr.offset.imm = sign_extend (imm, fields[self->fields[0]].width - 1);
> +  info->addr.offset.imm
> +    = sign_extend (imm, aarch64_fields[self->fields[0]].width - 1);
>    if (self->fields[0] == FLD_imm7
>        || info->qualifier == AARCH64_OPND_QLF_imm_tag)
>      /* scaled immediate in ld/st pair instructions.  */
> --- a/opcodes/aarch64-opc.c
> +++ b/opcodes/aarch64-opc.c
> @@ -223,7 +223,7 @@ aarch64_select_operand_for_sizeq_field_c
>  
>  /* Instruction bit-fields.
>  +   Keep synced with 'enum aarch64_field_kind'.  */
> -const aarch64_field fields[] =
> +const aarch64_field aarch64_fields[] =
>  {
>      {  0,  0 },	/* NIL.  */
>      {  8,  4 },	/* CRm: in the system instructions.  */
> --- a/opcodes/aarch64-opc.h
> +++ b/opcodes/aarch64-opc.h
> @@ -248,7 +248,7 @@ struct aarch64_field
>  
>  typedef struct aarch64_field aarch64_field;
>  
> -extern const aarch64_field fields[];
> +extern const aarch64_field aarch64_fields[];
>  
>  /* Operand description.  */
>  
> @@ -424,7 +424,7 @@ static inline unsigned
>  get_operand_field_width (const aarch64_operand *operand, unsigned n)
>  {
>    assert (operand->fields[n] != FLD_NIL);
> -  return fields[operand->fields[n]].width;
> +  return aarch64_fields[operand->fields[n]].width;
>  }
>  
>  /* Return the total width of the operand *OPERAND.  */
> @@ -434,7 +434,7 @@ get_operand_fields_width (const aarch64_
>    int i = 0;
>    unsigned width = 0;
>    while (operand->fields[i] != FLD_NIL)
> -    width += fields[operand->fields[i++]].width;
> +    width += aarch64_fields[operand->fields[i++]].width;
>    assert (width > 0 && width < 32);
>    return width;
>  }
> @@ -479,7 +479,7 @@ gen_mask (int width)
>  static inline int
>  gen_sub_field (enum aarch64_field_kind kind, int lsb_rel, int width, aarch64_field *ret)
>  {
> -  const aarch64_field *field = &fields[kind];
> +  const aarch64_field *field = &aarch64_fields[kind];
>    if (lsb_rel < 0 || width <= 0 || lsb_rel + width > field->width)
>      return 0;
>    ret->lsb = field->lsb + lsb_rel;
> @@ -525,7 +525,7 @@ static inline void
>  insert_field (enum aarch64_field_kind kind, aarch64_insn *code,
>  	      aarch64_insn value, aarch64_insn mask)
>  {
> -  insert_field_2 (&fields[kind], code, value, mask);
> +  insert_field_2 (&aarch64_fields[kind], code, value, mask);
>  }
>  
>  /* Extract field KIND of CODE and return the value.  MASK can be zero or the
> @@ -535,7 +535,7 @@ static inline aarch64_insn
>  extract_field (enum aarch64_field_kind kind, aarch64_insn code,
>  	       aarch64_insn mask)
>  {
> -  return extract_field_2 (&fields[kind], code, mask);
> +  return extract_field_2 (&aarch64_fields[kind], code, mask);
>  }
>  
>  extern aarch64_insn


More information about the Binutils mailing list