Hi Saurabh,

Not a full review, just something I noticed.

> On 13 Sep 2024, at 11:06, saurabh....@arm.com wrote:
> 
> External email: Use caution opening links or attachments
> 
> 
> The AArch64 FEAT_FAMINMAX extension is optional from Armv9.2-a and
> mandatory from Armv9.5-a. It introduces instructions for computing the
> floating point absolute maximum and minimum of the two vectors
> element-wise.
> 
> This patch introduces SVE2 faminmax intrinsics. The intrinsics of this
> extension are implemented as the following builtin functions:
> * sva[max|min]_[m|x|z]
> * sva[max|min]_[f16|f32|f64]_[m|x|z]
> * sva[max|min]_n_[f16|f32|f64]_[m|x|z]
> 
> gcc/ChangeLog:
> 
>        * config/aarch64/aarch64-sve-builtins-base.cc
>        (svamax): Absolute maximum declaration.
>        (svamin): Absolute minimum declaration.
>        * config/aarch64/aarch64-sve-builtins-base.def
>        (svamax): Absolute maximum declaration.
>        (svamin): Absolute minimum declaration.
>        * config/aarch64/aarch64-sve-builtins-base.h: Declaring function
>        bases for the new intrinsics.
>        * config/aarch64/aarch64.h
>        (TARGET_SVE_FAMINMAX): New flag for SVE2 faminmax.
>        * config/aarch64/iterators.md: New unspecs, iterators, and attrs
>        for the new intrinsics.
> 
> gcc/testsuite/ChangeLog:
> 
>        * gcc.target/aarch64/aminmax.h: New test.
>        * gcc.target/aarch64/sve2/acle/asm/amax_f16.c: New test.
>        * gcc.target/aarch64/sve2/acle/asm/amax_f32.c: New test.
>        * gcc.target/aarch64/sve2/acle/asm/amax_f64.c: New test.
>        * gcc.target/aarch64/sve2/acle/asm/amin_f16.c: New test.
>        * gcc.target/aarch64/sve2/acle/asm/amin_f32.c: New test.
>        * gcc.target/aarch64/sve2/acle/asm/amin_f64.c: New test.
> ---
> .../aarch64/aarch64-sve-builtins-base.cc      |   4 +
> .../aarch64/aarch64-sve-builtins-base.def     |   5 +
> .../aarch64/aarch64-sve-builtins-base.h       |   2 +
> gcc/config/aarch64/aarch64.h                  |   1 +
> gcc/config/aarch64/iterators.md               |  18 +-
> gcc/testsuite/gcc.target/aarch64/aminmax.h    |  13 ++
> .../aarch64/sve2/acle/asm/amax_f16.c          | 155 ++++++++++++++++++
> .../aarch64/sve2/acle/asm/amax_f32.c          | 155 ++++++++++++++++++
> .../aarch64/sve2/acle/asm/amax_f64.c          | 155 ++++++++++++++++++
> .../aarch64/sve2/acle/asm/amin_f16.c          | 155 ++++++++++++++++++
> .../aarch64/sve2/acle/asm/amin_f32.c          | 155 ++++++++++++++++++
> .../aarch64/sve2/acle/asm/amin_f64.c          | 155 ++++++++++++++++++
> 12 files changed, 972 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/aminmax.h
> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/amax_f16.c
> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/amax_f32.c
> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/amax_f64.c
> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/amin_f16.c
> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/amin_f32.c
> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/amin_f64.c
> 
> 

diff --git a/gcc/testsuite/gcc.target/aarch64/aminmax.h 
b/gcc/testsuite/gcc.target/aarch64/aminmax.h
new file mode 100644
index 00000000000..e901da84165
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/aminmax.h
@@ -0,0 +1,13 @@
+#ifdef AMINMAX_IDIOM
+
+#define TEST1(TYPE)
+__attribute__((noipa))                         \
+void fn_##TYPE (TYPE * restrict a,             \
+               TYPE * restrict b,              \
+               TYPE * restrict out) {  \
+  for (int i = 0; i < N; i++) {                        \
+    TYPE diff = b[i] - a[i];           \
+    out[i] = diff > 0 ? diff : -diff;          \
+} }
+
+#endif

This test doesn’t look like it belongs in this patch.
Thanks,
Kyrill


Reply via email to