On Tue, Sep 13, 2016 at 01:54:15PM +0100, Tamar Christina wrote:
> Hi all,
> 
> This patch adds the following NEON intrinsics to the ARM Aarch64 GCC
> (and fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72758):
> 

<snip>

> Added new tests for these and ran regression tests on aarch64-none-linux-gnu.
> 
> Ok for trunk?

Hi Tamar,

Thanks for the patch! If I'm not mistaken, this also fixes PR target/58693.

The convention when fixing a bugzilla is to place a line in your ChangeLog
which looks like:

        PR component/xxxxx

This then gets picked up by a little robot which scrapes the commits, and
updates bugzilla with a link to the commit that fixes the bug. See
gcc/ChangeLog for examples.

This patch looks OK to me functionally, but I'd appreciate a second set
of eyes on the testsuite changes. I've CC'ed Christophe Lyon who wrote
the testsuite. Do remember that these testcases are shared with the ARM
port, so it is appropriate to test an ARM build against your changes. In
this case, I see your patch introduces some errors when testing the ARM
target. Presumably the check against __ARM_FEATURE_CRYPTO is not strong
enough to ensure that the poly64_t types are available.

    FAIL: gcc.target/aarch64/advsimd-intrinsics/vdup-vmov.c   -O0  (test for 
excess errors)
    Excess errors:
    .../gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vdup-vmov.c:171:5: 
warning: implicit declaration of function 'vmov_n_p64'; did you mean 
'vmov_n_u64'? [-Wimplicit-function-declaration]
    .../gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vdup-vmov.c:171:5: 
warning: implicit declaration of function 'vmovq_n_p64'; did you mean 
'vmovq_n_u64'? [-Wimplicit-function-declaration]
    .../gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vdup-vmov.c:170:30: 
error: incompatible types when assigning to type 'poly64x2_t {aka __vector(2) 
long long unsigned int}' from type 'int'

    FAIL: gcc.target/aarch64/advsimd-intrinsics/vget_lane.c   -O0  (test for 
excess errors)
    FAIL: gcc.target/aarch64/advsimd-intrinsics/vldX.c   -O0  (test for excess 
errors)
    FAIL: gcc.target/aarch64/advsimd-intrinsics/vldX_lane.c   -O0  (test for 
excess errors)
    FAIL: gcc.target/aarch64/advsimd-intrinsics/vreinterpret_p128.c   -O0  
(test for excess errors)
    FAIL: gcc.target/aarch64/advsimd-intrinsics/vreinterpret_p64.c   -O0  (test 
for excess errors)
    FAIL: gcc.target/aarch64/advsimd-intrinsics/vstX_lane.c   -O0  (test for 
excess errors)

Could you take a look at this?

Thanks,
James

Reply via email to