Re: [PATCH][GCC][ARM] Dot Product NEON intrinsics [Patch (3/8)]

[Kyrill Tkachov]

Hi Christophe,

On 23/11/17 23:26, Christophe Lyon wrote:

On 22 November 2017 at 12:26, Kyrill  Tkachov
<kyrylo.tkac...@foss.arm.com> wrote:

Hi Tamar,

On 06/11/17 16:53, Tamar Christina wrote:

Hi All,

This patch adds the NEON intrinsics for Dot product.

Dot product is available from ARMv8.2-a and onwards.

Regtested on arm-none-eabi, armeb-none-eabi,
aarch64-none-elf and aarch64_be-none-elf with no issues found.

Ok for trunk?

gcc/
2017-11-06  Tamar Christina  <tamar.christ...@arm.com>

         * config/aarch64/arm_neon.h (vdot_u32, vdotq_u32)

This should be config/arm/arm_neon.h

         (vdot_s32, vdotq_s32): New.
         (vdot_lane_u32, vdotq_lane_u32): New.
         (vdot_lane_s32, vdotq_lane_s32): New.


gcc/testsuite/
2017-11-06  Tamar Christina  <tamar.christ...@arm.com>

         * gcc.target/arm/simd/vdot-compile.c: New.
         * gcc.target/arm/simd/vect-dot-qi.h: New.
         * gcc.target/arm/simd/vect-dot-s8.c: New.
         * gcc.target/arm/simd/vect-dot-u8.c: New

--

diff --git a/gcc/config/arm/arm_neon.h b/gcc/config/arm/arm_neon.h
index
0d436e83d0f01f0c86f8d6a25f84466c841c7e11..419080417901f343737741e334cbff818bb1e70a
100644
--- a/gcc/config/arm/arm_neon.h
+++ b/gcc/config/arm/arm_neon.h
@@ -18034,6 +18034,72 @@ vzipq_f16 (float16x8_t __a, float16x8_t __b)
   #endif
  +/* Adv.SIMD Dot Product intrinsics.  */

Please no full stop: "AdvSIMD".

+
+#pragma GCC push_options
+#if __ARM_ARCH >= 8
+#pragma GCC target ("arch=armv8.2-a+dotprod")

<snip>

Not sure if Kyrill actually meant to comment about the three lines
above, but they have a bug:
#if should be before #pragma GCC push_options.

You're right, sorry for missing this :(

Indeed, after this patch was committed (r255064), I've noticed many
regressions, for instance
p64_p128 is now unsupported. This is because the arm_crypto_ok
effective target now fails
with this message:
XXX/arm_neon.h:16911:1: error: inlining failed in call to
always_inline 'vaeseq_u8': target specific option mismatch

Not sure why this wasn't noticed in validations earlier?

Fixed as obvious (r255126).

Thank you for fixing this up.

Kyrill

Christophe

diff --git a/gcc/testsuite/gcc.target/arm/simd/vect-dot-qi.h
b/gcc/testsuite/gcc.target/arm/simd/vect-dot-qi.h
new file mode 100644
index
0000000000000000000000000000000000000000..90b00aff95cfef96d1963be17673dc191cc71169
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/simd/vect-dot-qi.h
@@ -0,0 +1,15 @@
+TYPE char X[N] __attribute__ ((__aligned__(__BIGGEST_ALIGNMENT__)));
+TYPE char Y[N] __attribute__ ((__aligned__(__BIGGEST_ALIGNMENT__)));
+
+__attribute__ ((noinline)) int
+foo1(int len) {
+  int i;
+  TYPE int result = 0;
+  TYPE short prod;
+
+  for (i=0; i<len; i++) {
+    prod = X[i] * Y[i];
+    result += prod;
+  }
+  return result;
+}
\ No newline at end of file

Please add new lines at the end of the new test files.
This applies to a few more new files in this patch.

Ok with these nits fixed.

Thanks,
Kyrill