diff mbox

[PING,3/3,Arm] Add support for IEEE-conformant versions of scalar fmin* and fmax*

Message ID VI1PR08MB047967D00172FE66FB36DDA084E50@VI1PR08MB0479.eurprd08.prod.outlook.com
State New
Headers show

Commit Message

David Sherwood Dec. 22, 2015, 5:07 p.m. UTC 
Hi Kyrill,

Thanks for the reply, I think this latest patch addresses your
comments. I have added a test for the scalar forms of fmax/fmin.

Is this ok now?

Tested:

arm-none-eabi: no regressions

Cheers,
Dave.

ChangeLog:

2015-12-22  David Sherwood  <david.sherwood@arm.com>

    gcc/
        * config/arm/iterators.md (VMAXMINFNM): New int iterator.
        (fmaxmin): New int attribute.
        (fmaxmin_op): Likewise.
        * config/arm/unspecs.md (UNSPEC_VMAXNM): New unspec.
        (UNSPEC_VMINNM): Likewise.
        * config/arm/neon.md (fmaxmin): New pattern.
        * config/arm/vfp.md (fmaxmin): Likewise.
    gcc/testsuite
        * gcc.target/arm/fmaxmin.x: New file used by tests below.
        * gcc.target/arm/fmaxmin.c: New test.
        * gcc.target/arm/vect-fmaxmin.c: Likewise.

Comments

Kyrill Tkachov Dec. 23, 2015, 9:25 a.m. UTC | #1 
Hi David,

On 22/12/15 17:07, David Sherwood wrote:
> Hi Kyrill,
>
> Thanks for the reply, I think this latest patch addresses your
> comments. I have added a test for the scalar forms of fmax/fmin.
>
> Is this ok now?
>
> Tested:
>
> arm-none-eabi: no regressions

I've also bootstrapped this patch on arm-none-linux-gnueabihf

> Cheers,
> Dave.
>
> ChangeLog:
>
> 2015-12-22  David Sherwood  <david.sherwood@arm.com>
>
>      gcc/
>          * config/arm/iterators.md (VMAXMINFNM): New int iterator.
>          (fmaxmin): New int attribute.
>          (fmaxmin_op): Likewise.
>          * config/arm/unspecs.md (UNSPEC_VMAXNM): New unspec.
>          (UNSPEC_VMINNM): Likewise.
>          * config/arm/neon.md (fmaxmin): New pattern.
>          * config/arm/vfp.md (fmaxmin): Likewise.
>      gcc/testsuite
>          * gcc.target/arm/fmaxmin.x: New file used by tests below.
>          * gcc.target/arm/fmaxmin.c: New test.
>          * gcc.target/arm/vect-fmaxmin.c: Likewise.
>

Yes, this is ok now.
Thanks,
Kyrill

>
> ________________________________________
> From: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>
> Sent: Wednesday, December 16, 2015 9:31 AM
> To: David Sherwood; GCC Patches
> Subject: Re: [PING][Patch 3/3][Arm] Add support for IEEE-conformant versions of scalar fmin* and fmax*
>
> Hi David,
>
> On 16/12/15 08:53, David Sherwood wrote:
>> Hi,
>>
>> Here is the last patch of the fmin/fmax change, which adds the optabs
>> to the arm backend.
>>
>> Tested:
>>
>> arm-none-eabi: no regressions
>>
>> Good to go?
>> David Sherwood.
>>
>> ChangeLog:
>>
>> 2015-12-08  David Sherwood  <david.sherwood@arm.com>
>>
>>       gcc/
>>           * config/arm/iterators.md: New iterators.
>>           * config/arm/unspecs.md: New unspecs.
>>           * config/arm/neon.md: New pattern.
>>           * config/arm/vfp.md: Likewise.
> Please list the new entities you add in parentheses.
> For example:
>       * config/arm/iterators.md (VMAXMINFNM): New int iterator.
>       (fmaxmin): New int attribute.
>       (fmaxmin): Likewise.
>
> Same for the other files. That way one can grep through the ChangeLogs to
> find when any particular pattern/iterator/whatever was modified.
>
> +;; Auto-vectorized forms for the IEEE-754 fmax()/fmin() functions
> +(define_insn "<fmaxmin><mode>3"
> +  [(set (match_operand:VCVTF 0 "s_register_operand" "=w")
> +       (unspec:VCVTF [(match_operand:VCVTF 1 "s_register_operand" "w")
> +                      (match_operand:VCVTF 2 "s_register_operand" "w")]
> +                      VMAXMINFNM))]
> +  "TARGET_NEON && TARGET_FPU_ARMV8"
> +  "<fmaxmin_op>.<V_s_elem>\t%<V_reg>0, %<V_reg>1, %<V_reg>2"
> +  [(set_attr "type" "neon_fp_minmax_s<q>")]
> +)
>
> I would just say "Vector forms" rather than "Auto-vectorized".
> In principle we can get vector types through other means besides
> auto-vectorisation.
>
> +;; Scalar forms for the IEEE-754 fmax()/fmin() functions
> +(define_insn "<fmaxmin><mode>3"
> +  [(set (match_operand:SDF 0 "s_register_operand" "=<F_constraint>")
> +       (unspec:SDF [(match_operand:SDF 1 "s_register_operand" "<F_constraint>")
> +                    (match_operand:SDF 2 "s_register_operand" "<F_constraint>")]
> +                    VMAXMINFNM))]
> +  "TARGET_HARD_FLOAT && TARGET_VFP5 <vfp_double_cond>"
> +  "<fmaxmin_op>.<V_if_elem>\\t%<V_reg>0, %<V_reg>1, %<V_reg>2"
> +  [(set_attr "type" "f_minmax<vfp_type>")
> +   (set_attr "conds" "unconditional")]
> +)
> +
>
> I notice your new test doesn't test the SF variant of this.
> Could you please add something to test it?
> Looks good to me otherwise.
>
> Thanks,
> Kyrill
>
diff mbox

Patch

diff --git a/gcc/config/arm/iterators.md b/gcc/config/arm/iterators.md
index c7a6880..6ff346c 100644
--- a/gcc/config/arm/iterators.md
+++ b/gcc/config/arm/iterators.md
@@ -308,6 +308,8 @@ 
 
 (define_int_iterator VMAXMINF [UNSPEC_VMAX UNSPEC_VMIN])
 
+(define_int_iterator VMAXMINFNM [UNSPEC_VMAXNM UNSPEC_VMINNM])
+
 (define_int_iterator VPADDL [UNSPEC_VPADDL_S UNSPEC_VPADDL_U])
 
 (define_int_iterator VPADAL [UNSPEC_VPADAL_S UNSPEC_VPADAL_U])
@@ -745,6 +747,13 @@ 
   (UNSPEC_VPMIN "min") (UNSPEC_VPMIN_U "min")
 ])
 
+(define_int_attr fmaxmin [
+  (UNSPEC_VMAXNM "fmax") (UNSPEC_VMINNM "fmin")])
+
+(define_int_attr fmaxmin_op [
+  (UNSPEC_VMAXNM "vmaxnm") (UNSPEC_VMINNM "vminnm")
+])
+
 (define_int_attr shift_op [
   (UNSPEC_VSHL_S "shl") (UNSPEC_VSHL_U "shl")
   (UNSPEC_VRSHL_S "rshl") (UNSPEC_VRSHL_U "rshl")
diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
index 844ef5e..d8cc686 100644
--- a/gcc/config/arm/neon.md
+++ b/gcc/config/arm/neon.md
@@ -2366,6 +2366,17 @@ 
   [(set_attr "type" "neon_fp_minmax_s<q>")]
 )
 
+;; Vector forms for the IEEE-754 fmax()/fmin() functions
+(define_insn "<fmaxmin><mode>3"
+  [(set (match_operand:VCVTF 0 "s_register_operand" "=w")
+	(unspec:VCVTF [(match_operand:VCVTF 1 "s_register_operand" "w")
+		       (match_operand:VCVTF 2 "s_register_operand" "w")]
+		       VMAXMINFNM))]
+  "TARGET_NEON && TARGET_FPU_ARMV8"
+  "<fmaxmin_op>.<V_s_elem>\t%<V_reg>0, %<V_reg>1, %<V_reg>2"
+  [(set_attr "type" "neon_fp_minmax_s<q>")]
+)
+
 (define_expand "neon_vpadd<mode>"
   [(match_operand:VD 0 "s_register_operand" "=w")
    (match_operand:VD 1 "s_register_operand" "w")
diff --git a/gcc/config/arm/unspecs.md b/gcc/config/arm/unspecs.md
index ffe703c..9c633c0 100644
--- a/gcc/config/arm/unspecs.md
+++ b/gcc/config/arm/unspecs.md
@@ -226,8 +226,10 @@ 
   UNSPEC_VLD4_LANE
   UNSPEC_VMAX
   UNSPEC_VMAX_U
+  UNSPEC_VMAXNM
   UNSPEC_VMIN
   UNSPEC_VMIN_U
+  UNSPEC_VMINNM
   UNSPEC_VMLA
   UNSPEC_VMLA_LANE
   UNSPEC_VMLAL_S
diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
index baeac62..3c89fe9 100644
--- a/gcc/config/arm/vfp.md
+++ b/gcc/config/arm/vfp.md
@@ -1366,6 +1366,18 @@ 
    (set_attr "conds" "unconditional")]
 )
 
+;; Scalar forms for the IEEE-754 fmax()/fmin() functions
+(define_insn "<fmaxmin><mode>3"
+  [(set (match_operand:SDF 0 "s_register_operand" "=<F_constraint>")
+	(unspec:SDF [(match_operand:SDF 1 "s_register_operand" "<F_constraint>")
+		     (match_operand:SDF 2 "s_register_operand" "<F_constraint>")]
+		     VMAXMINFNM))]
+  "TARGET_HARD_FLOAT && TARGET_VFP5 <vfp_double_cond>"
+  "<fmaxmin_op>.<V_if_elem>\\t%<V_reg>0, %<V_reg>1, %<V_reg>2"
+  [(set_attr "type" "f_minmax<vfp_type>")
+   (set_attr "conds" "unconditional")]
+)
+
 ;; Write Floating-point Status and Control Register.
 (define_insn "set_fpscr"
   [(unspec_volatile [(match_operand:SI 0 "register_operand" "r")] VUNSPEC_SET_FPSCR)]
diff --git a/gcc/testsuite/gcc.target/arm/fmaxmin.c b/gcc/testsuite/gcc.target/arm/fmaxmin.c
new file mode 100644
index 0000000..945c473
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/fmaxmin.c
@@ -0,0 +1,13 @@ 
+/* { dg-do run } */
+/* { dg-require-effective-target arm_v8_neon_ok } */
+/* { dg-options "-O2 -fno-inline -march=armv8-a -save-temps" } */
+/* { dg-add-options arm_v8_neon } */
+
+#include "fmaxmin.x"
+
+/* { dg-final { scan-assembler-times "vmaxnm.f32\ts\[0-9\]+, s\[0-9\]+, s\[0-9\]+" 1 } } */
+/* { dg-final { scan-assembler-times "vminnm.f32\ts\[0-9\]+, s\[0-9\]+, s\[0-9\]+" 1 } } */
+
+/* { dg-final { scan-assembler-times "vmaxnm.f64\td\[0-9\]+, d\[0-9\]+, d\[0-9\]+" 1 } } */
+/* { dg-final { scan-assembler-times "vminnm.f64\td\[0-9\]+, d\[0-9\]+, d\[0-9\]+" 1 } } */
+
diff --git a/gcc/testsuite/gcc.target/arm/fmaxmin.x b/gcc/testsuite/gcc.target/arm/fmaxmin.x
new file mode 100644
index 0000000..ccf832d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/fmaxmin.x
@@ -0,0 +1,54 @@ 
+extern void abort (void);
+double fmax (double, double);
+float fmaxf (float, float);
+double fmin (double, double);
+float fminf (float, float);
+
+#define isnan __builtin_isnan
+#define isinf __builtin_isinf
+
+#define NAN __builtin_nan ("")
+#define INFINITY __builtin_inf ()
+
+#define DEF_MAXMIN(TYPE,FUN)\
+void test_##FUN (TYPE *__restrict__ r, TYPE *__restrict__ a,\
+		 TYPE *__restrict__ b)\
+{\
+  int i;\
+  for (i = 0; i < 4; i++)\
+    r[i] = FUN (a[i], b[i]);\
+}\
+
+DEF_MAXMIN (float, fmaxf)
+DEF_MAXMIN (double, fmax)
+
+DEF_MAXMIN (float, fminf)
+DEF_MAXMIN (double, fmin)
+
+int main ()
+{
+  float a_f[4] = { 4, NAN, -3, INFINITY };
+  float b_f[4] = { 1,   7,NAN, 0 };
+  float r_f[4];
+  double a_d[4] = { 4, NAN,  -3,  INFINITY };
+  double b_d[4] = { 1,   7, NAN,  0 };
+  double r_d[4];
+
+  test_fmaxf (r_f, a_f, b_f);
+  if (r_f[0] != 4 || isnan (r_f[1]) || isnan (r_f[2]) || !isinf (r_f[3]))
+    abort ();
+
+  test_fminf (r_f, a_f, b_f);
+  if (r_f[0] != 1 || isnan (r_f[1]) || isnan (r_f[2]) || isinf (r_f[3]))
+    abort ();
+
+  test_fmax (r_d, a_d, b_d);
+  if (r_d[0] != 4 || isnan (r_d[1]) || isnan (r_d[2]) || !isinf (r_d[3]))
+    abort ();
+
+  test_fmin (r_d, a_d, b_d);
+  if (r_d[0] != 1 || isnan (r_d[1]) || isnan (r_d[2]) || isinf (r_d[3]))
+    abort ();
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/arm/vect-fmaxmin.c b/gcc/testsuite/gcc.target/arm/vect-fmaxmin.c
new file mode 100644
index 0000000..fd01cd9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/vect-fmaxmin.c
@@ -0,0 +1,14 @@ 
+/* { dg-do run } */
+/* { dg-require-effective-target arm_v8_neon_ok } */
+/* { dg-options "-O2 -ftree-vectorize -fno-inline -march=armv8-a -save-temps" } */
+/* { dg-add-options arm_v8_neon } */
+
+#include "fmaxmin.x"
+
+/* { dg-final { scan-assembler-times "vmaxnm.f32\tq\[0-9\]+, q\[0-9\]+, q\[0-9\]+" 1 } } */
+/* { dg-final { scan-assembler-times "vminnm.f32\tq\[0-9\]+, q\[0-9\]+, q\[0-9\]+" 1 } } */
+
+/* NOTE: There are no double precision vector versions of vmaxnm/vminnm.  */
+/* { dg-final { scan-assembler-times "vmaxnm.f64\td\[0-9\]+, d\[0-9\]+, d\[0-9\]+" 1 } } */
+/* { dg-final { scan-assembler-times "vminnm.f64\td\[0-9\]+, d\[0-9\]+, d\[0-9\]+" 1 } } */
+