Hi,
On Thu, 20 Feb 2025 at 15:18, Hannes Braun <han...@hannesbraun.net> wrote:
>
> vld1q_s8_x3, vld1q_s16_x3, vld1q_s8_x4 and vld1q_s16_x4 were expecting
> pointers to unsigned integers. These parameters should be pointers to
> signed integers.
>
> gcc/ChangeLog:
>
> * config/arm/arm_neon.h (vld1q_s8_x3): Use int8_t instead of
> uint16_t.
> (vld1q_s16_x3): Use int16_t instead of uint16_t.
> (vld1q_s8_x4): Likewise.
> (vld1q_s16_x4): Likewise.
Thanks for catching this and providing a fix.
Too bad the testcases are not compiled with enough warnings to detect this.
Minor comments (I can't approve the patch)
IIUC, that's for PR 118942? So the ChangeLog entry should mention PR
target/118942 (just before the * config/arm/arm_neon.h line), and the
commit title should end with [PR 118942], but that can be fixed at
commit time.
The contrib/gcc-changelog/git_check_commit.py script can help you
check that your commit message will be accepted by the commit hooks.
Looking at the original patch from Ezra, I think we should also have
enabled the relevant tests in gcc.target/aarch64/advsimd-intrinsics
(e.g. vld1x3.c) ?
Thanks,
Christophe
>
> Signed-off-by: Hannes Braun <han...@hannesbraun.net>
> ---
> gcc/config/arm/arm_neon.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/config/arm/arm_neon.h b/gcc/config/arm/arm_neon.h
> index 5b1c55c8d9f..578ada88fa6 100644
> --- a/gcc/config/arm/arm_neon.h
> +++ b/gcc/config/arm/arm_neon.h
> @@ -10854,7 +10854,7 @@ vld1q_s64_x2 (const int64_t * __a)
>
> __extension__ extern __inline int8x16x3_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> -vld1q_s8_x3 (const uint8_t * __a)
> +vld1q_s8_x3 (const int8_t * __a)
> {
> union { int8x16x3_t __i; __builtin_neon_ci __o; } __rv;
> __rv.__o = __builtin_neon_vld1q_x3v16qi ((const __builtin_neon_qi *) __a);
> @@ -10863,7 +10863,7 @@ vld1q_s8_x3 (const uint8_t * __a)
>
> __extension__ extern __inline int16x8x3_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> -vld1q_s16_x3 (const uint16_t * __a)
> +vld1q_s16_x3 (const int16_t * __a)
> {
> union { int16x8x3_t __i; __builtin_neon_ci __o; } __rv;
> __rv.__o = __builtin_neon_vld1q_x3v8hi ((const __builtin_neon_hi *) __a);
> @@ -10890,7 +10890,7 @@ vld1q_s64_x3 (const int64_t * __a)
>
> __extension__ extern __inline int8x16x4_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> -vld1q_s8_x4 (const uint8_t * __a)
> +vld1q_s8_x4 (const int8_t * __a)
> {
> union { int8x16x4_t __i; __builtin_neon_xi __o; } __rv;
> __rv.__o = __builtin_neon_vld1q_x4v16qi ((const __builtin_neon_qi *) __a);
> @@ -10899,7 +10899,7 @@ vld1q_s8_x4 (const uint8_t * __a)
>
> __extension__ extern __inline int16x8x4_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> -vld1q_s16_x4 (const uint16_t * __a)
> +vld1q_s16_x4 (const int16_t * __a)
> {
> union { int16x8x4_t __i; __builtin_neon_xi __o; } __rv;
> __rv.__o = __builtin_neon_vld1q_x4v8hi ((const __builtin_neon_hi *) __a);
> --
> 2.48.1
>
