Re: [PATCH v2 1/2] gas, aarch64: Add AdvSIMD lut extension

[Saurabh Jha <saurabh.jha@arm.com>]

On 5/20/2024 2:05 PM, Richard Earnshaw (lists) wrote:
> On 16/05/2024 11:35, Saurabh Jha 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 and a new instruction
>> class. We also modified the existing reglane inserters and extractors
>> to handle the new operands. The lsb and width of these operands are
>> the same as many existing operands 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
>> 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?
>>
>> Regards,
>> Saurabh
> 
> A couple of minor nits:
> 
> +++ b/gas/testsuite/gas/aarch64/advsimd-lut-illegal.s
> @@ -0,0 +1,128 @@
> +// Operand mismatch
> +luti2	v2.16b, { v4.8h }, v8[1]
> +luti2	v2.8h, { v4.16b }, v8[1]
> ...
> 
> In assembly files the convention is that labels start in column 0, while instructions start in column 8 (1 tab stop).
> 
> +++ b/gas/testsuite/gas/aarch64/advsimd-lut.d
> @@ -0,0 +1,32 @@
> +#objdump: -dr
> +#as: -march=armv8-a+lut
> +
> +.*:     file format .*
> +
> +Disassembly of section \.text:
> +
> +0+ <.*>:
> +   0:	4e801000 	luti2	v0.16b, \{v0.16b\}, v0\[0\]
> +   4:	4e80101f 	luti2	v31.16b, \{v0.16b\}, v0\[0\]
> +   8:	4e8013e0 	luti2	v0.16b, \{v31.16b\}, v0\[0\]
> ...
>     ^^
> We normally remove the address component from the dump files (use a regexp), so that inserting (or removing) lines does not cause churn on the testsuite.
> 
> Otherwise this looks OK.
> 
Thanks for the review Richard.

I have addressed both comments and have sent a new patch series. The 
patch in the new series corresponding to this one is 
https://sourceware.org/pipermail/binutils/2024-May/134194.html.