[PATCH 1/9] aarch64: Extend aarch64_field to support constants

[Richard Earnshaw (lists)]

On 08/10/2025 12:49, Alice Carlotti wrote:
> On Wed, Oct 08, 2025 at 11:56:23AM +0100, Richard Earnshaw (lists) wrote:
>> On 08/10/2025 11:34, Alice Carlotti wrote:
>>> On Wed, Oct 08, 2025 at 11:17:21AM +0100, Richard Earnshaw (lists) wrote:
>>>> On 08/10/2025 01:38, Alice Carlotti wrote:
>>>>> Many instructions have constraints on the range of registers they can
>>>>> use.  This means that some bits in the register number are fixed, and
>>>>> therefore aren't mapped to a field in the instruction encoding.
>>>>> Currently we use various adhoc rules to handle these fixed bits, but
>>>>> this doesn't handle all cases and we often have to write new code to
>>>>> support new combinations of permitted registers.
>>>>>
>>>>> This patch allows these constant bits to instead be specified in the
>>>>> same structure used to represent instruction fields.  Uses of the new
>>>>> constant fields will be introduced in subsequent patches.
>>>>
>>>> I'm likely missing something, but it would be nice if there were some explanation of how to use this feature in the comments.
>>>>
>>>> For example, how would I express a even numbered register? an odd-numbered register or a register in the first 16 of a 32-register set?  Can I express things like an odd numbered register that is in the range 0-16 (ie one bit set and one bit clear)?
>>>
>>> There are lots of examples in patches 2, 3, 4, 6 and 7 in this series, that
>>> answer all your questions and more.  Would you consider that to be enough
>>> documentation?  I'll note that we don't have a detailed explanation of how the
>>> existing field definitions work either, but I think it's pretty clear from the
>>> struct elements and existing usage.
>>
>> Not really.  I think this should be properly described in aarch64-opc.h where aarch64_field is defined.
> 
> Ok, do you also want me to add an explanation of how the existing field
> definitions are used at the same time?

I think that would be a good idea.  Anything that is not immediately apparent really should have the interface described explicitly.

> 
>>
>> But this brings up another question, why did you do it this way rather than by adding another field to this type for the
>> additional information?  We don't need these fields to be 32-bit ints, unsigned char would be more than adequate.
> 
> Good question.  This reflects a step towards how I want to pack the entire
> struct into 16 bits in future, but I realise I can just go straight to using a
> struct with bitfields already (and I checked that this is passed in a single
> register for function calls).
> 
> I had planned to use the layout:
> 
>   struct aarch64_field
>   {
>       unsigned char width;
>       unsigned char value:5;
>       unsigned char is_constant:1;
>   };
> 
> but this has the downside of requiring all the existing initialised to be swapped.  So I think it might be best to instead use:
> 
>   struct aarch64_field
>   {
>       unsigned char value;
>       unsigned char width:6;
>       unsigned char is_constant:1;
>   };
> 
> I'll send a new version of this patch later.
> 
I'd use unsigned int if you're using bitfields.  There's no particular benefit to using char and then you can use whatever order seems the most logical.

But I might be inclined to use a macro for the initializers anyway, and that can expand to the fields in whatever order is most appropriate.

struct aarch64_field aarch64_fields[] = 
  AARCH64_FIELD_NIL,
  AARCH64_FIELD_CONST (0, 1),   /* CONST_0.  */
  AARCH64_FIELD_CONST (1, 1),   /* CONST_1.  */
  AARCH64_FIELD_VAL (8, 4),     /* Crm: in the system instructions.  */
...
};

Or whatever macro names make the most sense.  The macros can insert 0's for other fields that don't care.

The alternative would be to use designated initializers, but I think the macros are better (they could still use designated initializers if that would help).

R.


> Alice
> 
>>
>> R.
>>
>>>
>>> Alice
>>>
>>>>
>>>> R.
>>>>
>>>>>
>>>>>
>>>>> diff --git a/opcodes/aarch64-opc.c b/opcodes/aarch64-opc.c
>>>>> index 4b173201e92bc7fd5c6d4e306e1f8fb332b0e041..b0ee3c9d3633075377ca3c14feccc124d0a94ba4 100644
>>>>> --- a/opcodes/aarch64-opc.c
>>>>> +++ b/opcodes/aarch64-opc.c
>>>>> @@ -226,6 +226,8 @@ aarch64_select_operand_for_sizeq_field_coding (const aarch64_opcode *opcode)
>>>>>  const aarch64_field aarch64_fields[] =
>>>>>  {
>>>>>      {  0,  0 },	/* NIL.  */
>>>>> +    { 32,  1 },	/* CONST_0.  */
>>>>> +    { 33,  1 },	/* CONST_1.  */
>>>>>      {  8,  4 },	/* CRm: in the system instructions.  */
>>>>>      { 10,  2 }, /* CRm_dsb_nxs: 2-bit imm. encoded in CRm<3:2>.  */
>>>>>      { 12,  4 },	/* CRn: in the system instructions.  */
>>>>> diff --git a/opcodes/aarch64-opc.h b/opcodes/aarch64-opc.h
>>>>> index 4182c9d5f678beac3a240e08c9842e8620a32bb6..4d2d928c1d8d39b9489e073fc9141c154b38e3a3 100644
>>>>> --- a/opcodes/aarch64-opc.h
>>>>> +++ b/opcodes/aarch64-opc.h
>>>>> @@ -30,6 +30,8 @@
>>>>>  enum aarch64_field_kind
>>>>>  {
>>>>>    FLD_NIL,
>>>>> +  FLD_CONST_0,
>>>>> +  FLD_CONST_1,
>>>>>    FLD_CRm,
>>>>>    FLD_CRm_dsb_nxs,
>>>>>    FLD_CRn,
>>>>> @@ -242,7 +244,10 @@ enum aarch64_field_kind
>>>>>    FLD_ZA5_4,
>>>>>  };
>>>>>  
>>>>> -/* Field description.  */
>>>>> +/* Field description.
>>>>> +   If lsb < 32, then this represents a field in an instruction encoding.
>>>>> +   If lsb >= 32, then this represents a constant value of width <= 5,
>>>>> +   stored as lsb = (value | 32).  */
>>>>>  struct aarch64_field
>>>>>  {
>>>>>    int lsb;
>>>>> @@ -497,8 +502,14 @@ insert_field_2 (const aarch64_field *field, aarch64_insn *code,
>>>>>  		aarch64_insn value, aarch64_insn mask)
>>>>>  {
>>>>>    assert (field->width < 32 && field->width >= 1 && field->lsb >= 0
>>>>> -	  && field->lsb + field->width <= 32);
>>>>> +	  && (field->lsb >= 32 || (field->lsb + field->width <= 32)));
>>>>>    value &= gen_mask (field->width);
>>>>> +  if (field->lsb >= 32)
>>>>> +    {
>>>>> +      /* Value is constant.  */
>>>>> +      assert (value == (field->lsb & 31));
>>>>> +      return;
>>>>> +    }
>>>>>    value <<= field->lsb;
>>>>>    /* In some opcodes, field can be part of the base opcode, e.g. the size
>>>>>       field in FADD.  The following helps avoid corrupt the base opcode.  */
>>>>> @@ -514,6 +525,10 @@ extract_field_2 (const aarch64_field *field, aarch64_insn code,
>>>>>  		 aarch64_insn mask)
>>>>>  {
>>>>>    aarch64_insn value;
>>>>> +  /* Check for constant field.  */
>>>>> +  if (field->lsb >= 32)
>>>>> +    return field->lsb & 31;
>>>>> +
>>>>>    /* Clear any bit that is a part of the base opcode.  */
>>>>>    code &= ~mask;
>>>>>    value = (code >> field->lsb) & gen_mask (field->width);
>>>>
>>