diff mbox series

aarch64: Use GCC vector extensions for integer mls intrinsics

Message ID DBBPR08MB4758E64E1E6F0DD0A8F1F9B3EBA40@DBBPR08MB4758.eurprd08.prod.outlook.com
State New
Headers show
Series aarch64: Use GCC vector extensions for integer mls intrinsics | expand

Commit Message

Jonathan Wright Jan. 18, 2021, 3:35 p.m. UTC 
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,
Jonathan

---

gcc/Changelog:

2021-01-14  Jonathan Wright  <jonathan.wright@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.

Comments

Richard Sandiford Jan. 19, 2021, 5:43 p.m. UTC | #1 
Jonathan Wright <Jonathan.Wright@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.wright@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
Jonathan Wright Jan. 27, 2021, 12:56 p.m. UTC | #2 
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.wright@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.

​
Kyrylo Tkachov Jan. 27, 2021, 2:51 p.m. UTC | #3 
From: Jonathan Wright <Jonathan.Wright@arm.com> 
Sent: 27 January 2021 12:57
To: Richard Sandiford <Richard.Sandiford@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw <Richard.Earnshaw@arm.com>
Subject: Re: [PATCH] aarch64: Use GCC vector extensions for integer mls intrinsics

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  <mailto:jonathan.wright@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.
​

Ok.
Thanks,
Kyrill
diff mbox series

Patch

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