I have re-written this to use RTL builtins - regression tested and bootstrapped
on aarch64-none-linux-gnu with no issues:
aarch64: Use RTL builtins for integer mls intrinsics
Rewrite integer mls Neon intrinsics to use RTL builtins rather than
inline assembly code, allowing for better scheduling and
optimization.
gcc/Changelog:
2021-01-11 Jonathan Wright <jonathan.wri...@arm.com>
* config/aarch64/aarch64-simd-builtins.def: Add mls builtin
generator macro.
* config/aarch64/arm_neon.h (vmls_s8): Use RTL builtin rather
than asm.
(vmls_s16): Likewise.
(vmls_s32): Likewise.
(vmls_u8): Likewise.
(vmls_u16): Likewise.
(vmls_u32): Likewise.
(vmlsq_s8): Likewise.
(vmlsq_s16): Likewise.
(vmlsq_s32): Likewise.
(vmlsq_u8): Likewise.
(vmlsq_u16): Likewise.
(vmlsq_u32): Likewise.
________________________________
From: Richard Sandiford <richard.sandif...@arm.com>
Sent: 19 January 2021 17:43
To: Jonathan Wright <jonathan.wri...@arm.com>
Cc: gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>; Richard Earnshaw
<richard.earns...@arm.com>; Kyrylo Tkachov <kyrylo.tkac...@arm.com>
Subject: Re: [PATCH] aarch64: Use GCC vector extensions for integer mls
intrinsics
Jonathan Wright <jonathan.wri...@arm.com> writes:
> Hi,
>
> As subject, this patch rewrites integer mls Neon intrinsics to use
> a - b * c rather than inline assembly code, allowing for better
> scheduling and optimization.
>
> Regression tested and bootstrapped on aarch64-none-linux-gnu - no
> issues.
>
> If ok, please commit to master (I don't have commit rights.)
Thanks for doing this. The patch looks good from a functional
point of view. I guess my only concern is that things like:
a = vmla_u8 (vmulq_u8 (b, c), d, e);
would become:
a = b * c + d * e;
and I don't think anything guarantees that the user's original
choice of instructon selection will be preserved. We might end
up with the equivalent of:
a = vmla_u8 (vmulq_u8 (d, e), b, c);
giving different latencies.
If we added built-in functions instead, we could lower them to
IFN_FMA and IFN_FNMA, which support integers as well as floats,
and which stand a better chance of preserving the original grouping.
There again, the unfused floating-point MLAs already decompose
into separate multiplies and adds (although they can't of course
use IFN_FMA).
Any thoughts on doing it that way instead?
I'm not saying the patch shouldn't go in though, just thought it
was worth asking.
Thanks,
Richard
>
> Thanks,
> Jonathan
>
> ---
>
> gcc/Changelog:
>
> 2021-01-14 Jonathan Wright <jonathan.wri...@arm.com>
>
> * config/aarch64/arm_neon.h (vmls_s8): Use C rather than asm.
> (vmls_s16): Likewise.
> (vmls_s32): Likewise.
> (vmls_u8): Likewise.
> (vmls_u16): Likewise.
> (vmls_u32): Likewise.
> (vmlsq_s8): Likewise.
> (vmlsq_s16): Likewise.
> (vmlsq_s32): Likewise.
> (vmlsq_u8): Likewise.
> (vmlsq_u16): Likewise.
> (vmlsq_u32): Likewise.
>
> diff --git a/gcc/config/aarch64/arm_neon.h b/gcc/config/aarch64/arm_neon.h
> index
> 608e582d25820062a409310e7f3fc872660f8041..ad04eab1e753aa86f20a8f6cc2717368b1840ef7
> 100644
> --- a/gcc/config/aarch64/arm_neon.h
> +++ b/gcc/config/aarch64/arm_neon.h
> @@ -7968,72 +7968,45 @@ __extension__ extern __inline int8x8_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> vmls_s8 (int8x8_t __a, int8x8_t __b, int8x8_t __c)
> {
> - int8x8_t __result;
> - __asm__ ("mls %0.8b,%2.8b,%3.8b"
> - : "=w"(__result)
> - : "0"(__a), "w"(__b), "w"(__c)
> - : /* No clobbers */);
> - return __result;
> + uint8x8_t __result = (uint8x8_t) __a - (uint8x8_t) __b * (uint8x8_t) __c;
> + return (int8x8_t) __result;
> }
>
> __extension__ extern __inline int16x4_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> vmls_s16 (int16x4_t __a, int16x4_t __b, int16x4_t __c)
> {
> - int16x4_t __result;
> - __asm__ ("mls %0.4h,%2.4h,%3.4h"
> - : "=w"(__result)
> - : "0"(__a), "w"(__b), "w"(__c)
> - : /* No clobbers */);
> - return __result;
> + uint16x4_t __result = (uint16x4_t) __a - (uint16x4_t) __b * (uint16x4_t)
> __c;
> + return (int16x4_t) __result;
> }
>
> __extension__ extern __inline int32x2_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> vmls_s32 (int32x2_t __a, int32x2_t __b, int32x2_t __c)
> {
> - int32x2_t __result;
> - __asm__ ("mls %0.2s,%2.2s,%3.2s"
> - : "=w"(__result)
> - : "0"(__a), "w"(__b), "w"(__c)
> - : /* No clobbers */);
> - return __result;
> + uint32x2_t __result = (uint32x2_t) __a - (uint32x2_t) __b * (uint32x2_t)
> __c;
> + return (int32x2_t) __result;
> }
>
> __extension__ extern __inline uint8x8_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> vmls_u8 (uint8x8_t __a, uint8x8_t __b, uint8x8_t __c)
> {
> - uint8x8_t __result;
> - __asm__ ("mls %0.8b,%2.8b,%3.8b"
> - : "=w"(__result)
> - : "0"(__a), "w"(__b), "w"(__c)
> - : /* No clobbers */);
> - return __result;
> + return __a - __b * __c;
> }
>
> __extension__ extern __inline uint16x4_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> vmls_u16 (uint16x4_t __a, uint16x4_t __b, uint16x4_t __c)
> {
> - uint16x4_t __result;
> - __asm__ ("mls %0.4h,%2.4h,%3.4h"
> - : "=w"(__result)
> - : "0"(__a), "w"(__b), "w"(__c)
> - : /* No clobbers */);
> - return __result;
> + return __a - __b * __c;
> }
>
> __extension__ extern __inline uint32x2_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> vmls_u32 (uint32x2_t __a, uint32x2_t __b, uint32x2_t __c)
> {
> - uint32x2_t __result;
> - __asm__ ("mls %0.2s,%2.2s,%3.2s"
> - : "=w"(__result)
> - : "0"(__a), "w"(__b), "w"(__c)
> - : /* No clobbers */);
> - return __result;
> + return __a - __b * __c;
> }
>
> #define vmlsl_high_lane_s16(a, b, c, d) \
> @@ -8565,72 +8538,45 @@ __extension__ extern __inline int8x16_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> vmlsq_s8 (int8x16_t __a, int8x16_t __b, int8x16_t __c)
> {
> - int8x16_t __result;
> - __asm__ ("mls %0.16b,%2.16b,%3.16b"
> - : "=w"(__result)
> - : "0"(__a), "w"(__b), "w"(__c)
> - : /* No clobbers */);
> - return __result;
> + uint8x16_t __result = (uint8x16_t) __a - (uint8x16_t) __b * (uint8x16_t)
> __c;
> + return (int8x16_t) __result;
> }
>
> __extension__ extern __inline int16x8_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> vmlsq_s16 (int16x8_t __a, int16x8_t __b, int16x8_t __c)
> {
> - int16x8_t __result;
> - __asm__ ("mls %0.8h,%2.8h,%3.8h"
> - : "=w"(__result)
> - : "0"(__a), "w"(__b), "w"(__c)
> - : /* No clobbers */);
> - return __result;
> + uint16x8_t __result = (uint16x8_t) __a - (uint16x8_t) __b * (uint16x8_t)
> __c;
> + return (int16x8_t) __result;
> }
>
> __extension__ extern __inline int32x4_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> vmlsq_s32 (int32x4_t __a, int32x4_t __b, int32x4_t __c)
> {
> - int32x4_t __result;
> - __asm__ ("mls %0.4s,%2.4s,%3.4s"
> - : "=w"(__result)
> - : "0"(__a), "w"(__b), "w"(__c)
> - : /* No clobbers */);
> - return __result;
> + uint32x4_t __result = (uint32x4_t) __a - (uint32x4_t) __b * (uint32x4_t)
> __c;
> + return (int32x4_t) __result;
> }
>
> __extension__ extern __inline uint8x16_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> vmlsq_u8 (uint8x16_t __a, uint8x16_t __b, uint8x16_t __c)
> {
> - uint8x16_t __result;
> - __asm__ ("mls %0.16b,%2.16b,%3.16b"
> - : "=w"(__result)
> - : "0"(__a), "w"(__b), "w"(__c)
> - : /* No clobbers */);
> - return __result;
> + return __a - __b * __c;
> }
>
> __extension__ extern __inline uint16x8_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> vmlsq_u16 (uint16x8_t __a, uint16x8_t __b, uint16x8_t __c)
> {
> - uint16x8_t __result;
> - __asm__ ("mls %0.8h,%2.8h,%3.8h"
> - : "=w"(__result)
> - : "0"(__a), "w"(__b), "w"(__c)
> - : /* No clobbers */);
> - return __result;
> + return __a - __b * __c;
> }
>
> __extension__ extern __inline uint32x4_t
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> vmlsq_u32 (uint32x4_t __a, uint32x4_t __b, uint32x4_t __c)
> {
> - uint32x4_t __result;
> - __asm__ ("mls %0.4s,%2.4s,%3.4s"
> - : "=w"(__result)
> - : "0"(__a), "w"(__b), "w"(__c)
> - : /* No clobbers */);
> - return __result;
> + return __a - __b * __c;
> }
>
> __extension__ extern __inline int16x8_t
diff --git a/gcc/config/aarch64/aarch64-simd-builtins.def b/gcc/config/aarch64/aarch64-simd-builtins.def
index ef83d5eee55a3d3e952a5078abaf0c03b4c3b01c..93a087987bb7f039b2f85a6e1d2e05eb95fa0058 100644
--- a/gcc/config/aarch64/aarch64-simd-builtins.def
+++ b/gcc/config/aarch64/aarch64-simd-builtins.def
@@ -183,6 +183,9 @@
/* Implemented by aarch64_mla_n<mode>. */
BUILTIN_VDQHS (TERNOP, mla_n, 0, NONE)
+ /* Implemented by aarch64_mls<mode>. */
+ BUILTIN_VDQ_BHSI (TERNOP, mls, 0, NONE)
+
/* Implemented by aarch64_<su>mlsl<mode>. */
BUILTIN_VD_BHSI (TERNOP, smlsl, 0, NONE)
BUILTIN_VD_BHSI (TERNOPU, umlsl, 0, NONE)
diff --git a/gcc/config/aarch64/arm_neon.h b/gcc/config/aarch64/arm_neon.h
index ef865625a3da545470549745afea03878a0bdbbc..45b3c125babae2e3d32d6cd3b36ce09c502c04d8 100644
--- a/gcc/config/aarch64/arm_neon.h
+++ b/gcc/config/aarch64/arm_neon.h
@@ -7888,72 +7888,48 @@ __extension__ extern __inline int8x8_t
__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
vmls_s8 (int8x8_t __a, int8x8_t __b, int8x8_t __c)
{
- int8x8_t __result;
- __asm__ ("mls %0.8b,%2.8b,%3.8b"
- : "=w"(__result)
- : "0"(__a), "w"(__b), "w"(__c)
- : /* No clobbers */);
- return __result;
+ return __builtin_aarch64_mlsv8qi (__a, __b, __c);
}
__extension__ extern __inline int16x4_t
__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
vmls_s16 (int16x4_t __a, int16x4_t __b, int16x4_t __c)
{
- int16x4_t __result;
- __asm__ ("mls %0.4h,%2.4h,%3.4h"
- : "=w"(__result)
- : "0"(__a), "w"(__b), "w"(__c)
- : /* No clobbers */);
- return __result;
+ return __builtin_aarch64_mlsv4hi (__a, __b, __c);
}
__extension__ extern __inline int32x2_t
__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
vmls_s32 (int32x2_t __a, int32x2_t __b, int32x2_t __c)
{
- int32x2_t __result;
- __asm__ ("mls %0.2s,%2.2s,%3.2s"
- : "=w"(__result)
- : "0"(__a), "w"(__b), "w"(__c)
- : /* No clobbers */);
- return __result;
+ return __builtin_aarch64_mlsv2si (__a, __b, __c);
}
__extension__ extern __inline uint8x8_t
__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
vmls_u8 (uint8x8_t __a, uint8x8_t __b, uint8x8_t __c)
{
- uint8x8_t __result;
- __asm__ ("mls %0.8b,%2.8b,%3.8b"
- : "=w"(__result)
- : "0"(__a), "w"(__b), "w"(__c)
- : /* No clobbers */);
- return __result;
+ return (uint8x8_t) __builtin_aarch64_mlsv8qi ((int8x8_t) __a,
+ (int8x8_t) __b,
+ (int8x8_t) __c);
}
__extension__ extern __inline uint16x4_t
__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
vmls_u16 (uint16x4_t __a, uint16x4_t __b, uint16x4_t __c)
{
- uint16x4_t __result;
- __asm__ ("mls %0.4h,%2.4h,%3.4h"
- : "=w"(__result)
- : "0"(__a), "w"(__b), "w"(__c)
- : /* No clobbers */);
- return __result;
+ return (uint16x4_t) __builtin_aarch64_mlsv4hi ((int16x4_t) __a,
+ (int16x4_t) __b,
+ (int16x4_t) __c);
}
__extension__ extern __inline uint32x2_t
__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
vmls_u32 (uint32x2_t __a, uint32x2_t __b, uint32x2_t __c)
{
- uint32x2_t __result;
- __asm__ ("mls %0.2s,%2.2s,%3.2s"
- : "=w"(__result)
- : "0"(__a), "w"(__b), "w"(__c)
- : /* No clobbers */);
- return __result;
+ return (uint32x2_t) __builtin_aarch64_mlsv2si ((int32x2_t) __a,
+ (int32x2_t) __b,
+ (int32x2_t) __c);
}
#define vmlsl_high_lane_s16(a, b, c, d) \
@@ -8425,72 +8401,48 @@ __extension__ extern __inline int8x16_t
__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
vmlsq_s8 (int8x16_t __a, int8x16_t __b, int8x16_t __c)
{
- int8x16_t __result;
- __asm__ ("mls %0.16b,%2.16b,%3.16b"
- : "=w"(__result)
- : "0"(__a), "w"(__b), "w"(__c)
- : /* No clobbers */);
- return __result;
+ return __builtin_aarch64_mlsv16qi (__a, __b, __c);
}
__extension__ extern __inline int16x8_t
__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
vmlsq_s16 (int16x8_t __a, int16x8_t __b, int16x8_t __c)
{
- int16x8_t __result;
- __asm__ ("mls %0.8h,%2.8h,%3.8h"
- : "=w"(__result)
- : "0"(__a), "w"(__b), "w"(__c)
- : /* No clobbers */);
- return __result;
+ return __builtin_aarch64_mlsv8hi (__a, __b, __c);
}
__extension__ extern __inline int32x4_t
__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
vmlsq_s32 (int32x4_t __a, int32x4_t __b, int32x4_t __c)
{
- int32x4_t __result;
- __asm__ ("mls %0.4s,%2.4s,%3.4s"
- : "=w"(__result)
- : "0"(__a), "w"(__b), "w"(__c)
- : /* No clobbers */);
- return __result;
+ return __builtin_aarch64_mlsv4si (__a, __b, __c);
}
__extension__ extern __inline uint8x16_t
__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
vmlsq_u8 (uint8x16_t __a, uint8x16_t __b, uint8x16_t __c)
{
- uint8x16_t __result;
- __asm__ ("mls %0.16b,%2.16b,%3.16b"
- : "=w"(__result)
- : "0"(__a), "w"(__b), "w"(__c)
- : /* No clobbers */);
- return __result;
+ return (uint8x16_t) __builtin_aarch64_mlsv16qi ((int8x16_t) __a,
+ (int8x16_t) __b,
+ (int8x16_t) __c);
}
__extension__ extern __inline uint16x8_t
__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
vmlsq_u16 (uint16x8_t __a, uint16x8_t __b, uint16x8_t __c)
{
- uint16x8_t __result;
- __asm__ ("mls %0.8h,%2.8h,%3.8h"
- : "=w"(__result)
- : "0"(__a), "w"(__b), "w"(__c)
- : /* No clobbers */);
- return __result;
+ return (uint16x8_t) __builtin_aarch64_mlsv8hi ((int16x8_t) __a,
+ (int16x8_t) __b,
+ (int16x8_t) __c);
}
__extension__ extern __inline uint32x4_t
__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
vmlsq_u32 (uint32x4_t __a, uint32x4_t __b, uint32x4_t __c)
{
- uint32x4_t __result;
- __asm__ ("mls %0.4s,%2.4s,%3.4s"
- : "=w"(__result)
- : "0"(__a), "w"(__b), "w"(__c)
- : /* No clobbers */);
- return __result;
+ return (uint32x4_t) __builtin_aarch64_mlsv4si ((int32x4_t) __a,
+ (int32x4_t) __b,
+ (int32x4_t) __c);
}
__extension__ extern __inline int16x8_t
