[PATCH v7 1/4] gas, aarch64: Add AdvSIMD lut extension

Saurabh Jha saurabh.jha@arm.com
Wed May 29 08:39:04 GMT 2024



On 5/28/2024 5:18 PM, Andrew Carlotti wrote:
> On Tue, May 28, 2024 at 03:45:50PM +0100, saurabh.jha@arm.com wrote:
>>
>> Introduces instructions for the Advanced SIMD lut extension for AArch64. They are documented in the following links:
>> * luti2: https://developer.arm.com/documentation/ddi0602/2024-03/SIMD-FP-Instructions/LUTI2--Lookup-table-read-with-2-bit-indices-?lang=en
>> * luti4: https://developer.arm.com/documentation/ddi0602/2024-03/SIMD-FP-Instructions/LUTI4--Lookup-table-read-with-4-bit-indices-?lang=en
>>
>> These instructions needed definition of some new operands. We will first
>> discuss operands for the third operand of the instructions and then
>> discuss a vector register list operand needed for the second operand.
>>
>> The third operands are vectors with bit indices and without type
>> qualifiers. They are called Em_INDEX1_14, Em_INDEX2_13, and Em_INDEX3_12
>> and they have 1 bit, 2 bit, and 3 bit indices respectively. For these
>> new operands, we defined new parsing case branch. The lsb and width of
>> these operands are the same as many existing but the convention is to
>> give different names to fields that serve different purpose so we
>> introduced new fields in aarch64-opc.c and aarch64-opc.h for these new
>> operands.
>>
>> For the second operand of these instructions, we introduced a new
>> operand called LVn_LUT. This represents a vector register list with
>> stride 1. We defined new inserter and extractor for this new operand and
>> it is encoded in FLD_Rn. We are enforcing the number of registers in the
>> reglist using opcode flag rather than operand flag as this is what other
>> SIMD vector register list operands are doing. The disassembly also uses
>> opcode flag to print the correct number of registers.
>> ---
>> Hi,
>>
>> Regression tested for aarch64-none-elf and found no regressions.
>>
>> Ok for binutils-master? I don't have commit access so can someone please commit on my behalf?
> 
> I think the series is fine now (though I can't formally approve it myself),
> although I have a couple of comments for future reference.
> 
>> Regards,
>> Saurabh
>> ---
>>   gas/NEWS                                      |   2 +
>>   gas/config/tc-aarch64.c                       |  67 ++++++
>>   gas/doc/c-aarch64.texi                        |   2 +
>>   gas/testsuite/gas/aarch64/advsimd-lut-bad.d   |   3 +
>>   gas/testsuite/gas/aarch64/advsimd-lut-bad.l   |  25 +++
>>   .../gas/aarch64/advsimd-lut-illegal.d         |   3 +
>>   .../gas/aarch64/advsimd-lut-illegal.l         | 208 ++++++++++++++++++
>>   .../gas/aarch64/advsimd-lut-illegal.s         | 128 +++++++++++
>>   gas/testsuite/gas/aarch64/advsimd-lut.d       |  32 +++
>>   gas/testsuite/gas/aarch64/advsimd-lut.s       |  29 +++
>>   include/opcode/aarch64.h                      |   9 +-
>>   opcodes/aarch64-asm.c                         |  11 +
>>   opcodes/aarch64-asm.h                         |   1 +
>>   opcodes/aarch64-dis.c                         |  15 ++
>>   opcodes/aarch64-dis.h                         |   1 +
>>   opcodes/aarch64-opc.c                         |  23 ++
>>   opcodes/aarch64-opc.h                         |   2 +
>>   opcodes/aarch64-tbl.h                         |  38 +++-
>>   18 files changed, 597 insertions(+), 2 deletions(-)
>>   create mode 100644 gas/testsuite/gas/aarch64/advsimd-lut-bad.d
>>   create mode 100644 gas/testsuite/gas/aarch64/advsimd-lut-bad.l
>>   create mode 100644 gas/testsuite/gas/aarch64/advsimd-lut-illegal.d
>>   create mode 100644 gas/testsuite/gas/aarch64/advsimd-lut-illegal.l
>>   create mode 100644 gas/testsuite/gas/aarch64/advsimd-lut-illegal.s
>>   create mode 100644 gas/testsuite/gas/aarch64/advsimd-lut.d
>>   create mode 100644 gas/testsuite/gas/aarch64/advsimd-lut.s
>>
> 
>> diff --git a/gas/NEWS b/gas/NEWS
>> index 804ef3559a7..b6f40a340d2 100644
>> --- a/gas/NEWS
>> +++ b/gas/NEWS
>> @@ -30,6 +30,8 @@
>>   
>>   * Add support for 'armv9.5-a' for -march in Arm GAS.
>>   
>> +* Add support for the AArch64 Lookup Table Extension (LUT).
>> +
>>   Changes in 2.42:
>>   
>>   * Add support for AMD znver5 processor.
>> diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
>> index 3f838cfd9a0..41547866d2c 100644
>> --- a/gas/config/tc-aarch64.c
>> +++ b/gas/config/tc-aarch64.c
>> @@ -1513,6 +1513,54 @@ parse_vector_reg_list (char **ccp, aarch64_reg_type type,
>>     return error ? PARSE_FAIL : (ret_val << 2) | (nb_regs - 1);
>>   }
>>   
>> +/* Parse a SIMD vector register with a bit index. The SIMD vectors with
>> +   bit indices don't have type qualifiers.
>> +
>> +   Return null if the string pointed to by *CCP is not a valid AdvSIMD
>> +   vector register with a bit index.
>> +
>> +   Otherwise return the register and the bit index information
>> +   in *typeinfo.
>> +
>> +   The validity of the bit index itself is checked separately in encoding.
>> + */
>> +
>> +static const reg_entry *
>> +parse_simd_vector_with_bit_index (char **ccp, struct vector_type_el *typeinfo)
>> +{
>> +  char *str = *ccp;
>> +  const reg_entry *reg = parse_reg (&str);
>> +  struct vector_type_el atype;
>> +
>> +  // Setting it here as this is the convention followed in the
>> +  // rest of the code with indices.
>> +  atype.defined = NTA_HASINDEX;
>> +  // This will be set to correct value in parse_index_expressions.
>> +  atype.index = 0;
>> +  // The rest of the fields are not applicable for this operand.
>> +  atype.type = NT_invtype;
>> +  atype.width = -1;
>> +  atype.element_size = 0;
>> +
>> +  if (reg == NULL)
>> +    return NULL;
>> +
>> +  if (reg->type != REG_TYPE_V)
>> +    return NULL;
>> +
>> +  // Parse the bit index.
>> +  if (!skip_past_char (&str, '['))
>> +    return NULL;
>> +  if (!parse_index_expression (&str, &atype.index))
>> +    return NULL;
>> +  if (!skip_past_char (&str, ']'))
>> +    return NULL;
>> +
>> +  *typeinfo = atype;
>> +  *ccp = str;
>> +  return reg;
>> +}
>> +
>>   /* Directives: register aliases.  */
>>   
>>   static reg_entry *
>> @@ -6790,6 +6838,23 @@ parse_operands (char *str, const aarch64_opcode *opcode)
>>   	  info->reglane.index = vectype.index;
>>   	  break;
>>   
>> +	case AARCH64_OPND_Em_INDEX1_14:
>> +	case AARCH64_OPND_Em_INDEX2_13:
>> +	case AARCH64_OPND_Em_INDEX3_12:
>> +	  // These are SIMD vector operands with bit indices. For example,
>> +	  // 'V27[3]'. These operands don't have type qualifiers before
>> +	  // indices.
>> +	  reg = parse_simd_vector_with_bit_index(&str, &vectype);
>> +
>> +	  if (!reg)
>> +	    goto failure;
>> +	  gas_assert (vectype.defined & NTA_HASINDEX);
>> +
>> +	  info->qualifier = AARCH64_OPND_QLF_NIL;
>> +	  info->reglane.regno = reg->number;
>> +	  info->reglane.index = vectype.index;
>> +	  break;
>> +
>>   	case AARCH64_OPND_SVE_ZnxN:
>>   	case AARCH64_OPND_SVE_ZtxN:
>>   	case AARCH64_OPND_SME_Zdnx2:
>> @@ -6812,6 +6877,7 @@ parse_operands (char *str, const aarch64_opcode *opcode)
>>   	  goto vector_reg_list;
>>   
>>   	case AARCH64_OPND_LVn:
>> +	case AARCH64_OPND_LVn_LUT:
>>   	case AARCH64_OPND_LVt:
>>   	case AARCH64_OPND_LVt_AL:
>>   	case AARCH64_OPND_LEt:
>> @@ -10481,6 +10547,7 @@ static const struct aarch64_option_cpu_value_table aarch64_features[] = {
>>     {"cpa",		AARCH64_FEATURE (CPA), AARCH64_NO_FEATURES},
>>     {"faminmax",		AARCH64_FEATURE (FAMINMAX), AARCH64_FEATURE (SIMD)},
>>     {"fp8",		AARCH64_FEATURE (FP8), AARCH64_FEATURE (SIMD)},
>> +  {"lut",		AARCH64_FEATURE (LUT), AARCH64_FEATURE (SIMD)},
>>     {NULL,		AARCH64_NO_FEATURES, AARCH64_NO_FEATURES},
>>   };
>>   
>> diff --git a/gas/doc/c-aarch64.texi b/gas/doc/c-aarch64.texi
>> index 4da18077f1b..bdfb504a5c5 100644
>> --- a/gas/doc/c-aarch64.texi
>> +++ b/gas/doc/c-aarch64.texi
>> @@ -293,6 +293,8 @@ automatically cause those extensions to be disabled.
>>    @tab Enable the Checked Pointer Arithmetic extension.
>>   @item @code{fp8} @tab
>>    @tab Enable the Floating Point 8 (FP8) extension.
>> +@item @code{lut} @tab
>> + @tab Enable the Lookup Table (LUT) extension.
>>   @end multitable
> 
> We should keep this list alphabeticized, but we can fix this and the other two
> misplaced additions in a follow-up commit.
> 
> It would make more sense to put this hunk and the NEWS updates in the last
> patch of the series (once full support is present), but I think it's ok here as
> long as the whole series is pushed together.
> 
We have the NEWS and texi update in this patch rather than the last 
because the flag and the feature were introduced in this patch.

I am planning a follow up patch to fix texi.


More information about the Binutils mailing list