Hi Kugan,
On 15 November 2017 at 12:23, James Greenhalgh <james.greenha...@arm.com> wrote:
> On Wed, Nov 15, 2017 at 09:58:28AM +0000, Kyrill Tkachov wrote:
>> Hi Kugan,
>>
>> On 07/11/17 04:10, Kugan Vivekanandarajah wrote:
>> > Hi,
>> >
>> > Attached patch implements the vld1_*_x2 intrinsics as defined by the
>> > neon document.
>> >
>> > Bootstrap for the latest patch is ongoing on aarch64-linux-gnu. Is
>> > this OK for trunk if no regressions?
>> >
>>
>> This looks mostly ok to me (though I cannot approve) modulo a couple of
>> minor type issues below.
>
> Thanks for the review Kyrill!
>
> I'm happy to trust Kyrill's knowledge of the back-end here, so the patch
> is OK with the changes Kyrill requested.
>
> Thanks for the patch!
>
> James
>
>> > gcc/ChangeLog:
>> >
>> > 2017-11-06 Kugan Vivekanandarajah <kug...@linaro.org>
>> >
>> > * config/aarch64/aarch64-simd.md (aarch64_ld1x2<VQ:mode>): New.
>> > (aarch64_ld1x2<VDC:mode>): Likewise.
>> > (aarch64_simd_ld1<mode>_x2): Likewise.
>> > (aarch64_simd_ld1<mode>_x2): Likewise.
>> > * config/aarch64/arm_neon.h (vld1_u8_x2): New.
>> > (vld1_s8_x2): Likewise.
>> > (vld1_u16_x2): Likewise.
>> > (vld1_s16_x2): Likewise.
>> > (vld1_u32_x2): Likewise.
>> > (vld1_s32_x2): Likewise.
>> > (vld1_u64_x2): Likewise.
>> > (vld1_s64_x2): Likewise.
>> > (vld1_f16_x2): Likewise.
>> > (vld1_f32_x2): Likewise.
>> > (vld1_f64_x2): Likewise.
>> > (vld1_p8_x2): Likewise.
>> > (vld1_p16_x2): Likewise.
>> > (vld1_p64_x2): Likewise.
>> > (vld1q_u8_x2): Likewise.
>> > (vld1q_s8_x2): Likewise.
>> > (vld1q_u16_x2): Likewise.
>> > (vld1q_s16_x2): Likewise.
>> > (vld1q_u32_x2): Likewise.
>> > (vld1q_s32_x2): Likewise.
>> > (vld1q_u64_x2): Likewise.
>> > (vld1q_s64_x2): Likewise.
>> > (vld1q_f16_x2): Likewise.
>> > (vld1q_f32_x2): Likewise.
>> > (vld1q_f64_x2): Likewise.
>> > (vld1q_p8_x2): Likewise.
>> > (vld1q_p16_x2): Likewise.
>> > (vld1q_p64_x2): Likewise.
>> >
>> > gcc/testsuite/ChangeLog:
>> >
>> > 2017-11-06 Kugan Vivekanandarajah <kug...@linaro.org>
>> >
>> > * gcc.target/aarch64/advsimd-intrinsics/vld1x2.c: New test.
>>
Sorry for not seeing this before you committed this patch, but the new
test fails to compile on arm targets.
Can you add the proper guard, as there is in other tests in the same dir?
Other question: why do you force -O3? The harness iterates on O0, O1, ....
Thanks,
Christophe
>> +__extension__ extern __inline int8x8x2_t
>> +__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
>> +vld1_s8_x2 (const uint8_t *__a)
>>
>> This should be "const int8_t *"
>>
>> +{
>> + int8x8x2_t ret;
>> + __builtin_aarch64_simd_oi __o;
>> + __o = __builtin_aarch64_ld1x2v8qi ((const __builtin_aarch64_simd_qi *)
>> __a);
>> + ret.val[0] = (int8x8_t) __builtin_aarch64_get_dregoiv8qi (__o, 0);
>> + ret.val[1] = (int8x8_t) __builtin_aarch64_get_dregoiv8qi (__o, 1);
>> + return ret;
>> +}
>>
>> ...
>>
>> +__extension__ extern __inline int32x2x2_t
>> +__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
>> +vld1_s32_x2 (const uint32_t *__a)
>>
>> Likewise, this should be "const int32_t *"
>>
>> +{
>> + int32x2x2_t ret;
>> + __builtin_aarch64_simd_oi __o;
>> + __o = __builtin_aarch64_ld1x2v2si ((const __builtin_aarch64_simd_si *)
>> __a);
>> + ret.val[0] = (int32x2_t) __builtin_aarch64_get_dregoiv2si (__o, 0);
>> + ret.val[1] = (int32x2_t) __builtin_aarch64_get_dregoiv2si (__o, 1);
>> + return ret;
>> +}
>> +
>>
>>
