Saurabh Jha <saurabh....@arm.com> writes:
> On 1/16/2025 8:44 AM, Richard Sandiford wrote:
>> Thanks for the update. Mostly LGTM, but some comments below:
>>
>> <saurabh....@arm.com> writes:
>>> diff --git a/gcc/config/aarch64/aarch64-sve2.md
>>> b/gcc/config/aarch64/aarch64-sve2.md
>>> index f8cfe08f4c0..0a1dc314f94 100644
>>> --- a/gcc/config/aarch64/aarch64-sve2.md
>>> +++ b/gcc/config/aarch64/aarch64-sve2.md
>>> @@ -133,6 +133,7 @@
>>> ;; ---- Optional AES extensions
>>> ;; ---- Optional SHA-3 extensions
>>> ;; ---- Optional SM4 extensions
>>> +;; ---- Table lookup
>>
>> This puts it under:
>>
>> ;; == Cryptographic extensions
>>
>> but it's not a crytographic extension. Probably better to put it under:
>>
>> ;; == General
>>
>> instead.
>>
>>> ;;
>>> =========================================================================
>>> ;; == Moves
>>> @@ -4211,3 +4212,35 @@
>>> "sm4ekey\t%0.s, %1.s, %2.s"
>>> [(set_attr "type" "crypto_sm4")]
>>> )
>>> +
>>> +;;
>>> -------------------------------------------------------------------------
>>> +;; ---- Table lookup
>>> +;;
>>> -------------------------------------------------------------------------
>>> +;; Includes:
>>> +;; - LUTI2
>>> +;; - LUTI4
>>> +;;
>>> -------------------------------------------------------------------------
>>> +
>>> +(define_insn "@aarch64_sve_luti<LUTI_BITS><mode>"
>>> + [(set (match_operand:SVE_FULL_BH 0 "register_operand" "=w")
>>> + (unspec:SVE_FULL_BH
>>> + [(match_operand:SVE_FULL_BH 1 "register_operand" "w")
>>> + (match_operand:VNx16QI 2 "register_operand" "w")
>>
>> This is correct
>>
>>> + (match_operand:DI 3 "const_int_operand")
>>> + (const_int LUTI_BITS)]
>>> + UNSPEC_SVE_LUTI))]
>>> + "TARGET_LUT && TARGET_SVE2_OR_SME2"
>>> + "luti<LUTI_BITS>\t%0.<Vetype>, { %1.<Vetype> }, %2[%3]"
>>> +)
>>> +
>>> +(define_insn "@aarch64_sve_luti<LUTI_BITS><mode>"
>>> + [(set (match_operand:<VSINGLE> 0 "register_operand" "=w")
>>> + (unspec:<VSINGLE>
>>> + [(match_operand:SVE_FULL_Hx2 1 "register_operand" "Uw2")
>>
>> ...but this should use aligned_register_operand instead of
>> register_operand.
> Are you sure we want it to be aligned? That requirement is not there on
> the { <Zn1>.H, <Zn2>.H } operand here:
> https://developer.arm.com/documentation/ddi0602/2024-12/SVE-Instructions/LUTI4--Lookup-table-read-with-4-bit-indices-?lang=en,
>
> as in, nothing like it has to be "Zn times 2....
Ah, right, sorry. I'd even said that last time:
> This operand also isn't required to be aligned: Zn has a 5-bit encoding.
but got it the wrong way round when doing this review. :(
But in that case the constraint should be "w" rather than "Uw2".
"Uw2" is for aligned registers only:
(define_register_constraint "Uw2" "FP_REGS"
"Even floating point and SIMD vector registers."
"regno % 2 == 0")
Richard
