[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