[PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Benedikt Huber]

arch64 offers the instructions frsqrte and frsqrts, for rsqrt estimation and
a Newton-Raphson step, respectively.
There are ARMv8 implementations where this is faster than using fdiv and rsqrt.
It runs three steps for double and two steps for float to achieve the needed precision.

There is one caveat and open question.
Since -ffast-math enables flush to zero intermediate values between approximation steps
will be flushed to zero if they are denormal.
E.g. This happens in the case of rsqrt (DBL_MAX) and rsqrtf (FLT_MAX).
The test cases pass, but it is unclear to me whether this is expected behavior with -ffast-math.

The patch applies to commit:
svn+ssh://gcc.gnu.org/svn/gcc/trunk@224470

Please consider including this patch.
Thank you and best regards,
Benedikt Huber

Benedikt Huber (1):
  2015-06-15  Benedikt Huber  <benedikt.huber@theobroma-systems.com>

 gcc/ChangeLog                            |   9 +++
 gcc/config/aarch64/aarch64-builtins.c    |  60 ++++++++++++++++
 gcc/config/aarch64/aarch64-protos.h      |   2 +
 gcc/config/aarch64/aarch64-simd.md       |  27 ++++++++
 gcc/config/aarch64/aarch64.c             |  63 +++++++++++++++++
 gcc/config/aarch64/aarch64.md            |   3 +
 gcc/testsuite/gcc.target/aarch64/rsqrt.c | 113 +++++++++++++++++++++++++++++++
 7 files changed, 277 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/rsqrt.c

-- 
1.9.1

[PATCH] 2015-06-15 Benedikt Huber <benedikt.huber@theobroma-systems.com>

[Benedikt Huber]

       * config/aarch64/aarch64-builtins.c: Builtins for rsqrt and rsqrtf.
       * config/aarch64/aarch64-protos.h: Declare.
       * config/aarch64/aarch64-simd.md: Matching expressions for frsqrte and frsqrts.
       * config/aarch64/aarch64.c: New functions. Emit rsqrt estimation code in fast math mode.
       * config/aarch64/aarch64.md: Added enum entry.
       * testsuite/gcc.target/aarch64/rsqrt.c: Tests for single and double.
---
 gcc/ChangeLog                            |   9 +++
 gcc/config/aarch64/aarch64-builtins.c    |  60 ++++++++++++++++
 gcc/config/aarch64/aarch64-protos.h      |   2 +
 gcc/config/aarch64/aarch64-simd.md       |  27 ++++++++
 gcc/config/aarch64/aarch64.c             |  63 +++++++++++++++++
 gcc/config/aarch64/aarch64.md            |   3 +
 gcc/testsuite/gcc.target/aarch64/rsqrt.c | 113 +++++++++++++++++++++++++++++++
 7 files changed, 277 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/rsqrt.c

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index c9b156f..690ebba 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2015-06-15  Benedikt Huber  <benedikt.huber@theobroma-systems.com>
+
+	* config/aarch64/aarch64-builtins.c: Builtins for rsqrt and rsqrtf.
+	* config/aarch64/aarch64-protos.h: Declare.
+	* config/aarch64/aarch64-simd.md: Matching expressions for frsqrte and frsqrts.
+	* config/aarch64/aarch64.c: New functions. Emit rsqrt estimation code in fast math mode.
+	* config/aarch64/aarch64.md: Added enum entry.
+	* testsuite/gcc.target/aarch64/rsqrt.c: Tests for single and double.
+
 2015-06-14  Richard Sandiford  <richard.sandiford@arm.com>
 
 	* rtl.h (classify_insn): Declare.
diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
index f7a39ec..484bb84 100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -342,6 +342,8 @@ enum aarch64_builtins
   AARCH64_BUILTIN_GET_FPSR,
   AARCH64_BUILTIN_SET_FPSR,
 
+  AARCH64_BUILTIN_RSQRT,
+  AARCH64_BUILTIN_RSQRTF,
   AARCH64_SIMD_BUILTIN_BASE,
   AARCH64_SIMD_BUILTIN_LANE_CHECK,
 #include "aarch64-simd-builtins.def"
@@ -831,6 +833,32 @@ aarch64_init_crc32_builtins ()
 }
 
 void
+aarch64_add_builtin_rsqrt (void)
+{
+  tree fndecl = NULL;
+  tree ftype = NULL;
+  ftype = build_function_type_list (double_type_node, double_type_node, NULL_TREE);
+
+  fndecl = add_builtin_function ("__builtin_aarch64_rsqrt",
+                                 ftype,
+                                 AARCH64_BUILTIN_RSQRT,
+                                 BUILT_IN_MD,
+                                 NULL,
+                                 NULL_TREE);
+  aarch64_builtin_decls[AARCH64_BUILTIN_RSQRT] = fndecl;
+
+  tree ftypef = NULL;
+  ftypef = build_function_type_list (float_type_node, float_type_node, NULL_TREE);
+  fndecl = add_builtin_function ("__builtin_aarch64_rsqrtf",
+                                 ftypef,
+                                 AARCH64_BUILTIN_RSQRTF,
+                                 BUILT_IN_MD,
+                                 NULL,
+                                 NULL_TREE);
+  aarch64_builtin_decls[AARCH64_BUILTIN_RSQRTF] = fndecl;
+}
+
+void
 aarch64_init_builtins (void)
 {
   tree ftype_set_fpr
@@ -855,6 +883,7 @@ aarch64_init_builtins (void)
     aarch64_init_simd_builtins ();
   if (TARGET_CRC32)
     aarch64_init_crc32_builtins ();
+  aarch64_add_builtin_rsqrt ();
 }
 
 tree
@@ -1099,6 +1128,23 @@ aarch64_crc32_expand_builtin (int fcode, tree exp, rtx target)
   return target;
 }
 
+static rtx
+aarch64_expand_builtin_rsqrt (int fcode, tree exp, rtx target)
+{
+  rtx pat;
+  tree arg0 = CALL_EXPR_ARG (exp, 0);
+  rtx op0 = expand_normal (arg0);
+
+  enum insn_code c = CODE_FOR_rsqrtdf;
+  if (fcode == AARCH64_BUILTIN_RSQRTF)
+    c = CODE_FOR_rsqrtsf;
+
+  pat = GEN_FCN (c) (target, op0);
+  emit_insn (pat);
+
+  return target;
+}
+
 /* Expand an expression EXP that calls a built-in function,
    with result going to TARGET if that's convenient.  */
 rtx
@@ -1146,6 +1192,11 @@ aarch64_expand_builtin (tree exp,
   else if (fcode >= AARCH64_CRC32_BUILTIN_BASE && fcode <= AARCH64_CRC32_BUILTIN_MAX)
     return aarch64_crc32_expand_builtin (fcode, exp, target);
 
+  if (fcode == AARCH64_BUILTIN_RSQRT ||
+      fcode == AARCH64_BUILTIN_RSQRTF)
+    return aarch64_expand_builtin_rsqrt (fcode, exp, target);
+
+  return NULL_RTX;
   gcc_unreachable ();
 }
 
@@ -1303,6 +1354,15 @@ aarch64_builtin_vectorized_function (tree fndecl, tree type_out, tree type_in)
   return NULL_TREE;
 }
 
+tree
+aarch64_builtin_rsqrt (bool is_float)
+{
+  if (is_float)
+    return aarch64_builtin_decls[AARCH64_BUILTIN_RSQRTF];
+  else
+    return aarch64_builtin_decls[AARCH64_BUILTIN_RSQRT];
+}
+
 #undef VAR1
 #define VAR1(T, N, MAP, A) \
   case AARCH64_SIMD_BUILTIN_##T##_##N##A:
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 965a11b..4f1c8ce 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -270,6 +270,8 @@ void aarch64_print_operand (FILE *, rtx, char);
 void aarch64_print_operand_address (FILE *, rtx);
 void aarch64_emit_call_insn (rtx);
 
+void aarch64_emit_swrsqrt (rtx, rtx);
+
 /* Initialize builtins for SIMD intrinsics.  */
 void init_aarch64_simd_builtins (void);
 
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index b90f938..266800a 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -353,6 +353,33 @@
   [(set_attr "type" "neon_fp_mul_d_scalar_q")]
 )
 
+(define_insn "*rsqrte_simd"
+  [(set (match_operand:VALLF 0 "register_operand" "=w")
+	(unspec:VALLF [(match_operand:VALLF 1 "register_operand" "w")]
+		     UNSPEC_RSQRTE))]
+  "TARGET_SIMD"
+  "frsqrte\\t%<v>0<Vmtype>, %<v>1<Vmtype>"
+  [(set_attr "type" "neon_fp_rsqrte_<Vetype><q>")])
+
+(define_insn "*rsqrts_simd"
+  [(set (match_operand:VALLF 0 "register_operand" "=w")
+	(unspec:VALLF [(match_operand:VALLF 1 "register_operand" "w")
+               (match_operand:VALLF 2 "register_operand" "w")]
+		     UNSPEC_RSQRTS))]
+  "TARGET_SIMD"
+  "frsqrts\\t%<v>0<Vmtype>, %<v>1<Vmtype>, %<v>2<Vmtype>"
+  [(set_attr "type" "neon_fp_rsqrts_<Vetype><q>")])
+
+(define_expand "rsqrt<mode>"
+  [(set (match_operand:GPF 0 "register_operand" "=w")
+	(unspec:GPF [(match_operand:GPF 1 "register_operand" "w")]
+		     UNSPEC_RSQRT))]
+  "TARGET_FLOAT"
+{
+  aarch64_emit_swrsqrt (operands[0], operands[1]);
+  DONE;
+})
+
 (define_insn "*aarch64_mul3_elt_to_64v2df"
   [(set (match_operand:DF 0 "register_operand" "=w")
      (mult:DF
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index c3c2795..281f0b2 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -6816,6 +6816,66 @@ aarch64_memory_move_cost (machine_mode mode ATTRIBUTE_UNUSED,
   return aarch64_tune_params->memmov_cost;
 }
 
+extern tree aarch64_builtin_rsqrt (bool is_float);
+
+static tree
+aarch64_builtin_reciprocal (unsigned int fn,
+                            bool md_fn ATTRIBUTE_UNUSED,
+                            bool sqrt ATTRIBUTE_UNUSED)
+{
+  if (!fast_math_flags_set_p (&global_options))
+    return NULL_TREE;
+
+  if (fn == BUILT_IN_SQRTF)
+    return aarch64_builtin_rsqrt (true);
+  else if (fn == BUILT_IN_SQRT)
+    return aarch64_builtin_rsqrt (false);
+  else
+    return NULL_TREE;
+}
+
+void
+aarch64_emit_swrsqrt (rtx dst, rtx src)
+{
+  enum machine_mode mode = GET_MODE (src);
+  rtx xsrc = gen_reg_rtx (mode);
+  emit_set_insn (xsrc, src);
+
+  rtx x0 = gen_reg_rtx (mode);
+  emit_insn (gen_rtx_SET (x0,
+                          gen_rtx_UNSPEC (mode, gen_rtvec (1, xsrc),
+                                          UNSPEC_RSQRTE)));
+
+  bool double_mode = (DFmode   == mode ||
+                      V1DFmode == mode ||
+                      V2DFmode == mode ||
+                      V4DFmode == mode ||
+                      V6DFmode == mode ||
+                      V8DFmode == mode);
+
+  int iterations = 2;
+  if (double_mode)
+    iterations = 3;
+
+  for (int i = 0; i < iterations; ++i)
+    {
+      rtx x1 = gen_reg_rtx (mode);
+      rtx x2 = gen_reg_rtx (mode);
+      rtx x3 = gen_reg_rtx (mode);
+      emit_insn (gen_rtx_SET (x2,
+                              gen_rtx_MULT (mode, x0, x0)));
+      emit_insn (gen_rtx_SET (x3,
+                              gen_rtx_UNSPEC (mode, gen_rtvec (2, xsrc, x2),
+                                              UNSPEC_RSQRTS)));
+      emit_insn (gen_rtx_SET (x1,
+                              gen_rtx_MULT (mode, x0, x3)));
+      x0 = x1;
+    }
+
+  emit_move_insn (dst, x0);
+  return;
+}
+
 /* Return the number of instructions that can be issued per cycle.  */
 static int
 aarch64_sched_issue_rate (void)
@@ -11747,6 +11807,9 @@ aarch64_gen_adjusted_ldpstp (rtx *operands, bool load,
 #undef TARGET_USE_BLOCKS_FOR_CONSTANT_P
 #define TARGET_USE_BLOCKS_FOR_CONSTANT_P aarch64_use_blocks_for_constant_p
 
+#undef TARGET_BUILTIN_RECIPROCAL
+#define TARGET_BUILTIN_RECIPROCAL aarch64_builtin_reciprocal
+
 #undef TARGET_VECTOR_MODE_SUPPORTED_P
 #define TARGET_VECTOR_MODE_SUPPORTED_P aarch64_vector_mode_supported_p
 
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 11123d6..7272d05 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -120,6 +120,9 @@
     UNSPEC_VSTRUCTDUMMY
     UNSPEC_SP_SET
     UNSPEC_SP_TEST
+    UNSPEC_RSQRT
+    UNSPEC_RSQRTE
+    UNSPEC_RSQRTS
 ])
 
 (define_c_enum "unspecv" [
diff --git a/gcc/testsuite/gcc.target/aarch64/rsqrt.c b/gcc/testsuite/gcc.target/aarch64/rsqrt.c
new file mode 100644
index 0000000..6607483
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/rsqrt.c
@@ -0,0 +1,113 @@
+/* { dg-do run } */
+/* { dg-options "-O3 -fno-inline --save-temps -ffast-math" } */
+
+#include <math.h>
+#include <stdio.h>
+
+#include <values.h>
+
+#define PI    3.141592653589793
+#define SQRT2 1.4142135623730951
+
+#define PI_4 0.7853981633974483
+#define SQRT1_2 0.7071067811865475
+
+/* 2^25+1, float has 24 significand bits
+ *       according to Single-precision floating-point format.  */
+#define TESTA8_FLT 33554433
+/* 2^54+1, double has 53 significand bits
+ *       according to Double-precision floating-point format.  */
+#define TESTA8_DBL 18014398509481985
+
+#define SD(a, b) t_double ((#a), (a), (b));
+#define SF(a, b) t_float ((#a), (a), (b));
+
+#define EPSILON_double __DBL_EPSILON__
+#define EPSILON_float __FLT_EPSILON__
+#define ABS_double fabs
+#define ABS_float fabsf
+#define SQRT_double sqrt
+#define SQRT_float sqrtf
+
+extern void abort (void);
+
+#define TESTTYPE(TYPE)                                                     \
+TYPE rsqrt_##TYPE (TYPE a)                                                 \
+{                                                                          \
+  return 1.0/SQRT_##TYPE(a);                                               \
+}                                                                          \
+                                                                           \
+int equals_##TYPE (TYPE a, TYPE b)                                         \
+{                                                                          \
+  return (a == b ||                                                        \
+          (isnan (a) && isnan (b)) ||                                      \
+          (ABS_##TYPE (a - b) < EPSILON_##TYPE));                          \
+}                                                                          \
+                                                                           \
+void t_##TYPE (const char *s, TYPE a, TYPE result)                         \
+{                                                                          \
+  TYPE r = rsqrt_##TYPE (a);                                               \
+  if (!equals_##TYPE (r, result))                                          \
+  {                                                                        \
+    abort ();                                                              \
+  }                                                                        \
+}                                                                          \
+
+//  printf ("Problem in %20s: %30.18A should be %30.18A\n", s, r, result); \
+
+TESTTYPE(double)
+TESTTYPE(float)
+
+/* { dg-final { scan-assembler-times "frsqrte\\td\[0-9\]+, d\[0-9\]+" 1 } } */
+/* { dg-final { scan-assembler-times "frsqrts\\td\[0-9\]+, d\[0-9\]+, d\[0-9\]+" 3 } } */
+
+/* { dg-final { scan-assembler-times "frsqrte\\ts\[0-9\]+, s\[0-9\]+" 1 } } */
+/* { dg-final { scan-assembler-times "frsqrts\\ts\[0-9\]+, s\[0-9\]+, s\[0-9\]+" 2 } } */
+
+int main ()
+{
+  SD(    1.0/256, 0X1.00000000000000P+4  );
+  SD(        1.0, 0X1.00000000000000P+0  );
+  SD(       -1.0,                     NAN);
+  SD(       11.0, 0X1.34BF63D1568260P-2  );
+  SD(        0.0,                INFINITY);
+  SD(   INFINITY, 0X0.00000000000000P+0  );
+  SD(        NAN,                     NAN);
+  SD(       -NAN,                    -NAN);
+  SD(    DBL_MAX, 0X1.00000000000010P-512);
+  SD(    DBL_MIN, 0X1.00000000000000P+511);
+  SD(         PI, 0X1.20DD750429B6D0P-1  );
+  SD(       PI_4, 0X1.20DD750429B6D0P+0  );
+  SD(      SQRT2, 0X1.AE89F995AD3AE0P-1  );
+  SD(    SQRT1_2, 0X1.306FE0A31B7150P+0  );
+  SD(        -PI,                     NAN);
+  SD(     -SQRT2,                     NAN);
+  SD( TESTA8_DBL, 0X1.00000000000000P-27 );
+
+  SF(    1.0/256, 0X1.00000000000000P+4  );
+  SF(        1.0, 0X1.00000000000000P+0  );
+  SF(       -1.0,                     NAN);
+  SF(       11.0, 0X1.34BF6400000000P-2  );
+  SF(        0.0,                INFINITY);
+  SF(   INFINITY, 0X0.00000000000000P+0  );
+  SF(        NAN,                     NAN);
+  SF(       -NAN,                    -NAN);
+  SF(    FLT_MAX, 0X1.00000200000000P-64 );
+  SF(    FLT_MIN, 0X1.00000000000000P+63 );
+  SF(         PI, 0X1.20DD7400000000P-1  );
+  SF(       PI_4, 0X1.20DD7400000000P+0  );
+  SF(      SQRT2, 0X1.AE89FA00000000P-1  );
+  SF(    SQRT1_2, 0X1.306FE000000000P+0  );
+  SF(        -PI,                     NAN);
+  SF(     -SQRT2,                     NAN);
+  SF( TESTA8_FLT, 0X1.6A09E600000000P-13 );
+
+//   With -ffast-math these return positive INF.
+//   SD(       -0.0,               -INFINITY);
+//   SF(       -0.0,               -INFINITY);
+//   The reason here is that -ffast-math flushes to zero.
+//   SD(DBL_MIN/256, 0X1.00000000000000P+515);
+//   SF(FLT_MIN/256, 0X1.00000000000000P+67 );
+
+  return 0;
+}
-- 
1.9.1

RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Kumar, Venkataramanan]

Hi, 

is there a plan to support  -mrecip=rsqrt for Aarch64?

Regards,
Venkat.

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Benedikt Huber
> Sent: Thursday, June 18, 2015 5:34 PM
> To: gcc-patches@gcc.gnu.org
> Cc: benedikt.huber@theobroma-systems.com;
> philipp.tomsich@theobroma-systems.com
> Subject: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> estimation in -ffast-math
> 
> arch64 offers the instructions frsqrte and frsqrts, for rsqrt estimation and a
> Newton-Raphson step, respectively.
> There are ARMv8 implementations where this is faster than using fdiv and
> rsqrt.
> It runs three steps for double and two steps for float to achieve the needed
> precision.
> 
> There is one caveat and open question.
> Since -ffast-math enables flush to zero intermediate values between
> approximation steps will be flushed to zero if they are denormal.
> E.g. This happens in the case of rsqrt (DBL_MAX) and rsqrtf (FLT_MAX).
> The test cases pass, but it is unclear to me whether this is expected behavior
> with -ffast-math.
> 
> The patch applies to commit:
> svn+ssh://gcc.gnu.org/svn/gcc/trunk@224470
> 
> Please consider including this patch.
> Thank you and best regards,
> Benedikt Huber
> 
> Benedikt Huber (1):
>   2015-06-15  Benedikt Huber  <benedikt.huber@theobroma-systems.com>
> 
>  gcc/ChangeLog                            |   9 +++
>  gcc/config/aarch64/aarch64-builtins.c    |  60 ++++++++++++++++
>  gcc/config/aarch64/aarch64-protos.h      |   2 +
>  gcc/config/aarch64/aarch64-simd.md       |  27 ++++++++
>  gcc/config/aarch64/aarch64.c             |  63 +++++++++++++++++
>  gcc/config/aarch64/aarch64.md            |   3 +
>  gcc/testsuite/gcc.target/aarch64/rsqrt.c | 113
> +++++++++++++++++++++++++++++++
>  7 files changed, 277 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/rsqrt.c
> 
> --
> 1.9.1

RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Evandro Menezes]

[-- Attachment #1: Type: text/plain, Size: 2283 bytes --]

Benedikt,

You beat me to it! :-)  Do you have the implementation for dividing using
the Newton series as well?

I'm not sure that the series is always for all data types and on all
processors.  It would be useful to allow each AArch64 processor to enable
this or not depending on the data type.  BTW, do you have some tests showing
the speed up?

Thank you,

-- 
Evandro Menezes                              Austin, TX

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org]
On
> Behalf Of Benedikt Huber
> Sent: Thursday, June 18, 2015 7:04
> To: gcc-patches@gcc.gnu.org
> Cc: benedikt.huber@theobroma-systems.com; philipp.tomsich@theobroma-
> systems.com
> Subject: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> estimation in -ffast-math
> 
> arch64 offers the instructions frsqrte and frsqrts, for rsqrt estimation
and
> a Newton-Raphson step, respectively.
> There are ARMv8 implementations where this is faster than using fdiv and
> rsqrt.
> It runs three steps for double and two steps for float to achieve the
needed
> precision.
> 
> There is one caveat and open question.
> Since -ffast-math enables flush to zero intermediate values between
> approximation steps will be flushed to zero if they are denormal.
> E.g. This happens in the case of rsqrt (DBL_MAX) and rsqrtf (FLT_MAX).
> The test cases pass, but it is unclear to me whether this is expected
> behavior with -ffast-math.
> 
> The patch applies to commit:
> svn+ssh://gcc.gnu.org/svn/gcc/trunk@224470
> 
> Please consider including this patch.
> Thank you and best regards,
> Benedikt Huber
> 
> Benedikt Huber (1):
>   2015-06-15  Benedikt Huber  <benedikt.huber@theobroma-systems.com>
> 
>  gcc/ChangeLog                            |   9 +++
>  gcc/config/aarch64/aarch64-builtins.c    |  60 ++++++++++++++++
>  gcc/config/aarch64/aarch64-protos.h      |   2 +
>  gcc/config/aarch64/aarch64-simd.md       |  27 ++++++++
>  gcc/config/aarch64/aarch64.c             |  63 +++++++++++++++++
>  gcc/config/aarch64/aarch64.md            |   3 +
>  gcc/testsuite/gcc.target/aarch64/rsqrt.c | 113
> +++++++++++++++++++++++++++++++
>  7 files changed, 277 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/rsqrt.c
> 
> --
> 1.9.1

[-- Attachment #2: Type: message/rfc822, Size: 16261 bytes --]

From: "Benedikt Huber" <benedikt.huber@theobroma-systems.com>
To: <gcc-patches@gcc.gnu.org>
Cc: <benedikt.huber@theobroma-systems.com>, <philipp.tomsich@theobroma-systems.com>
Subject: [PATCH] 2015-06-15  Benedikt Huber <benedikt.huber@theobroma-systems.com>
Date: Thu, 18 Jun 2015 07:04:05 -0500
Message-ID: <1434629045-24650-2-git-send-email-benedikt.huber@theobroma-systems.com>

       * config/aarch64/aarch64-builtins.c: Builtins for rsqrt and rsqrtf.
       * config/aarch64/aarch64-protos.h: Declare.
       * config/aarch64/aarch64-simd.md: Matching expressions for frsqrte
and frsqrts.
       * config/aarch64/aarch64.c: New functions. Emit rsqrt estimation code
in fast math mode.
       * config/aarch64/aarch64.md: Added enum entry.
       * testsuite/gcc.target/aarch64/rsqrt.c: Tests for single and double.
---
 gcc/ChangeLog                            |   9 +++
 gcc/config/aarch64/aarch64-builtins.c    |  60 ++++++++++++++++
 gcc/config/aarch64/aarch64-protos.h      |   2 +
 gcc/config/aarch64/aarch64-simd.md       |  27 ++++++++
 gcc/config/aarch64/aarch64.c             |  63 +++++++++++++++++
 gcc/config/aarch64/aarch64.md            |   3 +
 gcc/testsuite/gcc.target/aarch64/rsqrt.c | 113
+++++++++++++++++++++++++++++++
 7 files changed, 277 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/rsqrt.c

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index c9b156f..690ebba 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2015-06-15  Benedikt Huber  <benedikt.huber@theobroma-systems.com>
+
+	* config/aarch64/aarch64-builtins.c: Builtins for rsqrt and rsqrtf.
+	* config/aarch64/aarch64-protos.h: Declare.
+	* config/aarch64/aarch64-simd.md: Matching expressions for frsqrte
and frsqrts.
+	* config/aarch64/aarch64.c: New functions. Emit rsqrt estimation
code in fast math mode.
+	* config/aarch64/aarch64.md: Added enum entry.
+	* testsuite/gcc.target/aarch64/rsqrt.c: Tests for single and double.
+
 2015-06-14  Richard Sandiford  <richard.sandiford@arm.com>
 
 	* rtl.h (classify_insn): Declare.
diff --git a/gcc/config/aarch64/aarch64-builtins.c
b/gcc/config/aarch64/aarch64-builtins.c
index f7a39ec..484bb84 100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -342,6 +342,8 @@ enum aarch64_builtins
   AARCH64_BUILTIN_GET_FPSR,
   AARCH64_BUILTIN_SET_FPSR,
 
+  AARCH64_BUILTIN_RSQRT,
+  AARCH64_BUILTIN_RSQRTF,
   AARCH64_SIMD_BUILTIN_BASE,
   AARCH64_SIMD_BUILTIN_LANE_CHECK,
 #include "aarch64-simd-builtins.def"
@@ -831,6 +833,32 @@ aarch64_init_crc32_builtins ()
 }
 
 void
+aarch64_add_builtin_rsqrt (void)
+{
+  tree fndecl = NULL;
+  tree ftype = NULL;
+  ftype = build_function_type_list (double_type_node, double_type_node,
NULL_TREE);
+
+  fndecl = add_builtin_function ("__builtin_aarch64_rsqrt",
+                                 ftype,
+                                 AARCH64_BUILTIN_RSQRT,
+                                 BUILT_IN_MD,
+                                 NULL,
+                                 NULL_TREE);
+  aarch64_builtin_decls[AARCH64_BUILTIN_RSQRT] = fndecl;
+
+  tree ftypef = NULL;
+  ftypef = build_function_type_list (float_type_node, float_type_node,
NULL_TREE);
+  fndecl = add_builtin_function ("__builtin_aarch64_rsqrtf",
+                                 ftypef,
+                                 AARCH64_BUILTIN_RSQRTF,
+                                 BUILT_IN_MD,
+                                 NULL,
+                                 NULL_TREE);
+  aarch64_builtin_decls[AARCH64_BUILTIN_RSQRTF] = fndecl;
+}
+
+void
 aarch64_init_builtins (void)
 {
   tree ftype_set_fpr
@@ -855,6 +883,7 @@ aarch64_init_builtins (void)
     aarch64_init_simd_builtins ();
   if (TARGET_CRC32)
     aarch64_init_crc32_builtins ();
+  aarch64_add_builtin_rsqrt ();
 }
 
 tree
@@ -1099,6 +1128,23 @@ aarch64_crc32_expand_builtin (int fcode, tree exp,
rtx target)
   return target;
 }
 
+static rtx
+aarch64_expand_builtin_rsqrt (int fcode, tree exp, rtx target)
+{
+  rtx pat;
+  tree arg0 = CALL_EXPR_ARG (exp, 0);
+  rtx op0 = expand_normal (arg0);
+
+  enum insn_code c = CODE_FOR_rsqrtdf;
+  if (fcode == AARCH64_BUILTIN_RSQRTF)
+    c = CODE_FOR_rsqrtsf;
+
+  pat = GEN_FCN (c) (target, op0);
+  emit_insn (pat);
+
+  return target;
+}
+
 /* Expand an expression EXP that calls a built-in function,
    with result going to TARGET if that's convenient.  */
 rtx
@@ -1146,6 +1192,11 @@ aarch64_expand_builtin (tree exp,
   else if (fcode >= AARCH64_CRC32_BUILTIN_BASE && fcode <=
AARCH64_CRC32_BUILTIN_MAX)
     return aarch64_crc32_expand_builtin (fcode, exp, target);
 
+  if (fcode == AARCH64_BUILTIN_RSQRT ||
+      fcode == AARCH64_BUILTIN_RSQRTF)
+    return aarch64_expand_builtin_rsqrt (fcode, exp, target);
+
+  return NULL_RTX;
   gcc_unreachable ();
 }
 
@@ -1303,6 +1354,15 @@ aarch64_builtin_vectorized_function (tree fndecl,
tree type_out, tree type_in)
   return NULL_TREE;
 }
 
+tree
+aarch64_builtin_rsqrt (bool is_float)
+{
+  if (is_float)
+    return aarch64_builtin_decls[AARCH64_BUILTIN_RSQRTF];
+  else
+    return aarch64_builtin_decls[AARCH64_BUILTIN_RSQRT];
+}
+
 #undef VAR1
 #define VAR1(T, N, MAP, A) \
   case AARCH64_SIMD_BUILTIN_##T##_##N##A:
diff --git a/gcc/config/aarch64/aarch64-protos.h
b/gcc/config/aarch64/aarch64-protos.h
index 965a11b..4f1c8ce 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -270,6 +270,8 @@ void aarch64_print_operand (FILE *, rtx, char);
 void aarch64_print_operand_address (FILE *, rtx);
 void aarch64_emit_call_insn (rtx);
 
+void aarch64_emit_swrsqrt (rtx, rtx);
+
 /* Initialize builtins for SIMD intrinsics.  */
 void init_aarch64_simd_builtins (void);
 
diff --git a/gcc/config/aarch64/aarch64-simd.md
b/gcc/config/aarch64/aarch64-simd.md
index b90f938..266800a 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -353,6 +353,33 @@
   [(set_attr "type" "neon_fp_mul_d_scalar_q")]
 )
 
+(define_insn "*rsqrte_simd"
+  [(set (match_operand:VALLF 0 "register_operand" "=w")
+	(unspec:VALLF [(match_operand:VALLF 1 "register_operand" "w")]
+		     UNSPEC_RSQRTE))]
+  "TARGET_SIMD"
+  "frsqrte\\t%<v>0<Vmtype>, %<v>1<Vmtype>"
+  [(set_attr "type" "neon_fp_rsqrte_<Vetype><q>")])
+
+(define_insn "*rsqrts_simd"
+  [(set (match_operand:VALLF 0 "register_operand" "=w")
+	(unspec:VALLF [(match_operand:VALLF 1 "register_operand" "w")
+               (match_operand:VALLF 2 "register_operand" "w")]
+		     UNSPEC_RSQRTS))]
+  "TARGET_SIMD"
+  "frsqrts\\t%<v>0<Vmtype>, %<v>1<Vmtype>, %<v>2<Vmtype>"
+  [(set_attr "type" "neon_fp_rsqrts_<Vetype><q>")])
+
+(define_expand "rsqrt<mode>"
+  [(set (match_operand:GPF 0 "register_operand" "=w")
+	(unspec:GPF [(match_operand:GPF 1 "register_operand" "w")]
+		     UNSPEC_RSQRT))]
+  "TARGET_FLOAT"
+{
+  aarch64_emit_swrsqrt (operands[0], operands[1]);
+  DONE;
+})
+
 (define_insn "*aarch64_mul3_elt_to_64v2df"
   [(set (match_operand:DF 0 "register_operand" "=w")
      (mult:DF
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index c3c2795..281f0b2 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -6816,6 +6816,66 @@ aarch64_memory_move_cost (machine_mode mode
ATTRIBUTE_UNUSED,
   return aarch64_tune_params->memmov_cost;
 }
 
+extern tree aarch64_builtin_rsqrt (bool is_float);
+
+static tree
+aarch64_builtin_reciprocal (unsigned int fn,
+                            bool md_fn ATTRIBUTE_UNUSED,
+                            bool sqrt ATTRIBUTE_UNUSED)
+{
+  if (!fast_math_flags_set_p (&global_options))
+    return NULL_TREE;
+
+  if (fn == BUILT_IN_SQRTF)
+    return aarch64_builtin_rsqrt (true);
+  else if (fn == BUILT_IN_SQRT)
+    return aarch64_builtin_rsqrt (false);
+  else
+    return NULL_TREE;
+}
+
+void
+aarch64_emit_swrsqrt (rtx dst, rtx src)
+{
+  enum machine_mode mode = GET_MODE (src);
+  rtx xsrc = gen_reg_rtx (mode);
+  emit_set_insn (xsrc, src);
+
+  rtx x0 = gen_reg_rtx (mode);
+  emit_insn (gen_rtx_SET (x0,
+                          gen_rtx_UNSPEC (mode, gen_rtvec (1, xsrc),
+                                          UNSPEC_RSQRTE)));
+
+  bool double_mode = (DFmode   == mode ||
+                      V1DFmode == mode ||
+                      V2DFmode == mode ||
+                      V4DFmode == mode ||
+                      V6DFmode == mode ||
+                      V8DFmode == mode);
+
+  int iterations = 2;
+  if (double_mode)
+    iterations = 3;
+
+  for (int i = 0; i < iterations; ++i)
+    {
+      rtx x1 = gen_reg_rtx (mode);
+      rtx x2 = gen_reg_rtx (mode);
+      rtx x3 = gen_reg_rtx (mode);
+      emit_insn (gen_rtx_SET (x2,
+                              gen_rtx_MULT (mode, x0, x0)));
+      emit_insn (gen_rtx_SET (x3,
+                              gen_rtx_UNSPEC (mode, gen_rtvec (2, xsrc,
x2),
+                                              UNSPEC_RSQRTS)));
+      emit_insn (gen_rtx_SET (x1,
+                              gen_rtx_MULT (mode, x0, x3)));
+      x0 = x1;
+    }
+
+  emit_move_insn (dst, x0);
+  return;
+}
+
 /* Return the number of instructions that can be issued per cycle.  */
 static int
 aarch64_sched_issue_rate (void)
@@ -11747,6 +11807,9 @@ aarch64_gen_adjusted_ldpstp (rtx *operands, bool
load,
 #undef TARGET_USE_BLOCKS_FOR_CONSTANT_P
 #define TARGET_USE_BLOCKS_FOR_CONSTANT_P aarch64_use_blocks_for_constant_p
 
+#undef TARGET_BUILTIN_RECIPROCAL
+#define TARGET_BUILTIN_RECIPROCAL aarch64_builtin_reciprocal
+
 #undef TARGET_VECTOR_MODE_SUPPORTED_P
 #define TARGET_VECTOR_MODE_SUPPORTED_P aarch64_vector_mode_supported_p
 
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 11123d6..7272d05 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -120,6 +120,9 @@
     UNSPEC_VSTRUCTDUMMY
     UNSPEC_SP_SET
     UNSPEC_SP_TEST
+    UNSPEC_RSQRT
+    UNSPEC_RSQRTE
+    UNSPEC_RSQRTS
 ])
 
 (define_c_enum "unspecv" [
diff --git a/gcc/testsuite/gcc.target/aarch64/rsqrt.c
b/gcc/testsuite/gcc.target/aarch64/rsqrt.c
new file mode 100644
index 0000000..6607483
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/rsqrt.c
@@ -0,0 +1,113 @@
+/* { dg-do run } */
+/* { dg-options "-O3 -fno-inline --save-temps -ffast-math" } */
+
+#include <math.h>
+#include <stdio.h>
+
+#include <values.h>
+
+#define PI    3.141592653589793
+#define SQRT2 1.4142135623730951
+
+#define PI_4 0.7853981633974483
+#define SQRT1_2 0.7071067811865475
+
+/* 2^25+1, float has 24 significand bits
+ *       according to Single-precision floating-point format.  */
+#define TESTA8_FLT 33554433
+/* 2^54+1, double has 53 significand bits
+ *       according to Double-precision floating-point format.  */
+#define TESTA8_DBL 18014398509481985
+
+#define SD(a, b) t_double ((#a), (a), (b));
+#define SF(a, b) t_float ((#a), (a), (b));
+
+#define EPSILON_double __DBL_EPSILON__
+#define EPSILON_float __FLT_EPSILON__
+#define ABS_double fabs
+#define ABS_float fabsf
+#define SQRT_double sqrt
+#define SQRT_float sqrtf
+
+extern void abort (void);
+
+#define TESTTYPE(TYPE)
\
+TYPE rsqrt_##TYPE (TYPE a)
\
+{
\
+  return 1.0/SQRT_##TYPE(a);
\
+}
\
+
\
+int equals_##TYPE (TYPE a, TYPE b)
\
+{
\
+  return (a == b ||
\
+          (isnan (a) && isnan (b)) ||
\
+          (ABS_##TYPE (a - b) < EPSILON_##TYPE));
\
+}
\
+
\
+void t_##TYPE (const char *s, TYPE a, TYPE result)
\
+{
\
+  TYPE r = rsqrt_##TYPE (a);
\
+  if (!equals_##TYPE (r, result))
\
+  {
\
+    abort ();
\
+  }
\
+}
\
+
+//  printf ("Problem in %20s: %30.18A should be %30.18A\n", s, r, result);
\
+
+TESTTYPE(double)
+TESTTYPE(float)
+
+/* { dg-final { scan-assembler-times "frsqrte\\td\[0-9\]+, d\[0-9\]+" 1 } }
*/
+/* { dg-final { scan-assembler-times "frsqrts\\td\[0-9\]+, d\[0-9\]+,
d\[0-9\]+" 3 } } */
+
+/* { dg-final { scan-assembler-times "frsqrte\\ts\[0-9\]+, s\[0-9\]+" 1 } }
*/
+/* { dg-final { scan-assembler-times "frsqrts\\ts\[0-9\]+, s\[0-9\]+,
s\[0-9\]+" 2 } } */
+
+int main ()
+{
+  SD(    1.0/256, 0X1.00000000000000P+4  );
+  SD(        1.0, 0X1.00000000000000P+0  );
+  SD(       -1.0,                     NAN);
+  SD(       11.0, 0X1.34BF63D1568260P-2  );
+  SD(        0.0,                INFINITY);
+  SD(   INFINITY, 0X0.00000000000000P+0  );
+  SD(        NAN,                     NAN);
+  SD(       -NAN,                    -NAN);
+  SD(    DBL_MAX, 0X1.00000000000010P-512);
+  SD(    DBL_MIN, 0X1.00000000000000P+511);
+  SD(         PI, 0X1.20DD750429B6D0P-1  );
+  SD(       PI_4, 0X1.20DD750429B6D0P+0  );
+  SD(      SQRT2, 0X1.AE89F995AD3AE0P-1  );
+  SD(    SQRT1_2, 0X1.306FE0A31B7150P+0  );
+  SD(        -PI,                     NAN);
+  SD(     -SQRT2,                     NAN);
+  SD( TESTA8_DBL, 0X1.00000000000000P-27 );
+
+  SF(    1.0/256, 0X1.00000000000000P+4  );
+  SF(        1.0, 0X1.00000000000000P+0  );
+  SF(       -1.0,                     NAN);
+  SF(       11.0, 0X1.34BF6400000000P-2  );
+  SF(        0.0,                INFINITY);
+  SF(   INFINITY, 0X0.00000000000000P+0  );
+  SF(        NAN,                     NAN);
+  SF(       -NAN,                    -NAN);
+  SF(    FLT_MAX, 0X1.00000200000000P-64 );
+  SF(    FLT_MIN, 0X1.00000000000000P+63 );
+  SF(         PI, 0X1.20DD7400000000P-1  );
+  SF(       PI_4, 0X1.20DD7400000000P+0  );
+  SF(      SQRT2, 0X1.AE89FA00000000P-1  );
+  SF(    SQRT1_2, 0X1.306FE000000000P+0  );
+  SF(        -PI,                     NAN);
+  SF(     -SQRT2,                     NAN);
+  SF( TESTA8_FLT, 0X1.6A09E600000000P-13 );
+
+//   With -ffast-math these return positive INF.
+//   SD(       -0.0,               -INFINITY);
+//   SF(       -0.0,               -INFINITY);
+//   The reason here is that -ffast-math flushes to zero.
+//   SD(DBL_MIN/256, 0X1.00000000000000P+515);
+//   SF(FLT_MIN/256, 0X1.00000000000000P+67 );
+
+  return 0;
+}
-- 
1.9.1

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Dr. Philipp Tomsich]

Evandro,

We’ve seen a 28% speed-up on gromacs in SPECfp for the (scalar) reciprocal sqrt.

Also, the “reciprocal divide” patches are floating around in various of our git-tree, but 
aren’t ready for public consumption, yet… I’ll leave Benedikt to comment on potential 
timelines for getting that pushed out.

Best,
Philipp.

> On 24 Jun 2015, at 18:42, Evandro Menezes <e.menezes@samsung.com> wrote:
> 
> Benedikt,
> 
> You beat me to it! :-)  Do you have the implementation for dividing using
> the Newton series as well?
> 
> I'm not sure that the series is always for all data types and on all
> processors.  It would be useful to allow each AArch64 processor to enable
> this or not depending on the data type.  BTW, do you have some tests showing
> the speed up?
> 
> Thank you,
> 
> -- 
> Evandro Menezes                              Austin, TX
> 
>> -----Original Message-----
>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org]
> On
>> Behalf Of Benedikt Huber
>> Sent: Thursday, June 18, 2015 7:04
>> To: gcc-patches@gcc.gnu.org
>> Cc: benedikt.huber@theobroma-systems.com; philipp.tomsich@theobroma-
>> systems.com
>> Subject: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
>> estimation in -ffast-math
>> 
>> arch64 offers the instructions frsqrte and frsqrts, for rsqrt estimation
> and
>> a Newton-Raphson step, respectively.
>> There are ARMv8 implementations where this is faster than using fdiv and
>> rsqrt.
>> It runs three steps for double and two steps for float to achieve the
> needed
>> precision.
>> 
>> There is one caveat and open question.
>> Since -ffast-math enables flush to zero intermediate values between
>> approximation steps will be flushed to zero if they are denormal.
>> E.g. This happens in the case of rsqrt (DBL_MAX) and rsqrtf (FLT_MAX).
>> The test cases pass, but it is unclear to me whether this is expected
>> behavior with -ffast-math.
>> 
>> The patch applies to commit:
>> svn+ssh://gcc.gnu.org/svn/gcc/trunk@224470
>> 
>> Please consider including this patch.
>> Thank you and best regards,
>> Benedikt Huber
>> 
>> Benedikt Huber (1):
>>  2015-06-15  Benedikt Huber  <benedikt.huber@theobroma-systems.com>
>> 
>> gcc/ChangeLog                            |   9 +++
>> gcc/config/aarch64/aarch64-builtins.c    |  60 ++++++++++++++++
>> gcc/config/aarch64/aarch64-protos.h      |   2 +
>> gcc/config/aarch64/aarch64-simd.md       |  27 ++++++++
>> gcc/config/aarch64/aarch64.c             |  63 +++++++++++++++++
>> gcc/config/aarch64/aarch64.md            |   3 +
>> gcc/testsuite/gcc.target/aarch64/rsqrt.c | 113
>> +++++++++++++++++++++++++++++++
>> 7 files changed, 277 insertions(+)
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/rsqrt.c
>> 
>> --
>> 1.9.1
> <Mail Attachment.eml>

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Benedikt Huber]

[-- Attachment #1: Type: text/plain, Size: 3392 bytes --]

Evandro,

Yes, we also have the 1/x approximation.
However we do not have the test cases yet, and it also would need some clean up.
I am going to provide a patch for that soon (say next week).
Also, for this optimization we have *not* yet found a benchmark with significant improvements.

Best Regards,
Benedikt


> On 24 Jun 2015, at 18:52, Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote:
> 
> Evandro,
> 
> We’ve seen a 28% speed-up on gromacs in SPECfp for the (scalar) reciprocal sqrt.
> 
> Also, the “reciprocal divide” patches are floating around in various of our git-tree, but
> aren’t ready for public consumption, yet… I’ll leave Benedikt to comment on potential
> timelines for getting that pushed out.
> 
> Best,
> Philipp.
> 
>> On 24 Jun 2015, at 18:42, Evandro Menezes <e.menezes@samsung.com> wrote:
>> 
>> Benedikt,
>> 
>> You beat me to it! :-)  Do you have the implementation for dividing using
>> the Newton series as well?
>> 
>> I'm not sure that the series is always for all data types and on all
>> processors.  It would be useful to allow each AArch64 processor to enable
>> this or not depending on the data type.  BTW, do you have some tests showing
>> the speed up?
>> 
>> Thank you,
>> 
>> --
>> Evandro Menezes                              Austin, TX
>> 
>>> -----Original Message-----
>>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org]
>> On
>>> Behalf Of Benedikt Huber
>>> Sent: Thursday, June 18, 2015 7:04
>>> To: gcc-patches@gcc.gnu.org
>>> Cc: benedikt.huber@theobroma-systems.com; philipp.tomsich@theobroma-
>>> systems.com
>>> Subject: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
>>> estimation in -ffast-math
>>> 
>>> arch64 offers the instructions frsqrte and frsqrts, for rsqrt estimation
>> and
>>> a Newton-Raphson step, respectively.
>>> There are ARMv8 implementations where this is faster than using fdiv and
>>> rsqrt.
>>> It runs three steps for double and two steps for float to achieve the
>> needed
>>> precision.
>>> 
>>> There is one caveat and open question.
>>> Since -ffast-math enables flush to zero intermediate values between
>>> approximation steps will be flushed to zero if they are denormal.
>>> E.g. This happens in the case of rsqrt (DBL_MAX) and rsqrtf (FLT_MAX).
>>> The test cases pass, but it is unclear to me whether this is expected
>>> behavior with -ffast-math.
>>> 
>>> The patch applies to commit:
>>> svn+ssh://gcc.gnu.org/svn/gcc/trunk@224470
>>> 
>>> Please consider including this patch.
>>> Thank you and best regards,
>>> Benedikt Huber
>>> 
>>> Benedikt Huber (1):
>>> 2015-06-15  Benedikt Huber  <benedikt.huber@theobroma-systems.com>
>>> 
>>> gcc/ChangeLog                            |   9 +++
>>> gcc/config/aarch64/aarch64-builtins.c    |  60 ++++++++++++++++
>>> gcc/config/aarch64/aarch64-protos.h      |   2 +
>>> gcc/config/aarch64/aarch64-simd.md       |  27 ++++++++
>>> gcc/config/aarch64/aarch64.c             |  63 +++++++++++++++++
>>> gcc/config/aarch64/aarch64.md            |   3 +
>>> gcc/testsuite/gcc.target/aarch64/rsqrt.c | 113
>>> +++++++++++++++++++++++++++++++
>>> 7 files changed, 277 insertions(+)
>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/rsqrt.c
>>> 
>>> --
>>> 1.9.1
>> <Mail Attachment.eml>
> 


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 496 bytes --]

RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Evandro Menezes]

Benedikt,

Are you developing the reciprocal approximation just for 1/x proper or for any division, as in x/y = x * 1/y?

Thank you,

-- 
Evandro Menezes                              Austin, TX


> -----Original Message-----
> From: Benedikt Huber [mailto:benedikt.huber@theobroma-systems.com]
> Sent: Wednesday, June 24, 2015 12:11
> To: Dr. Philipp Tomsich
> Cc: Evandro Menezes; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> estimation in -ffast-math
> 
> Evandro,
> 
> Yes, we also have the 1/x approximation.
> However we do not have the test cases yet, and it also would need some clean
> up.
> I am going to provide a patch for that soon (say next week).
> Also, for this optimization we have *not* yet found a benchmark with
> significant improvements.
> 
> Best Regards,
> Benedikt
> 
> 
> > On 24 Jun 2015, at 18:52, Dr. Philipp Tomsich <philipp.tomsich@theobroma-
> systems.com> wrote:
> >
> > Evandro,
> >
> > We’ve seen a 28% speed-up on gromacs in SPECfp for the (scalar) reciprocal
> sqrt.
> >
> > Also, the “reciprocal divide” patches are floating around in various
> > of our git-tree, but aren’t ready for public consumption, yet… I’ll
> > leave Benedikt to comment on potential timelines for getting that pushed
> out.
> >
> > Best,
> > Philipp.
> >
> >> On 24 Jun 2015, at 18:42, Evandro Menezes <e.menezes@samsung.com> wrote:
> >>
> >> Benedikt,
> >>
> >> You beat me to it! :-)  Do you have the implementation for dividing
> >> using the Newton series as well?
> >>
> >> I'm not sure that the series is always for all data types and on all
> >> processors.  It would be useful to allow each AArch64 processor to
> >> enable this or not depending on the data type.  BTW, do you have some
> >> tests showing the speed up?
> >>
> >> Thank you,
> >>
> >> --
> >> Evandro Menezes                              Austin, TX
> >>
> >>> -----Original Message-----
> >>> From: gcc-patches-owner@gcc.gnu.org
> >>> [mailto:gcc-patches-owner@gcc.gnu.org]
> >> On
> >>> Behalf Of Benedikt Huber
> >>> Sent: Thursday, June 18, 2015 7:04
> >>> To: gcc-patches@gcc.gnu.org
> >>> Cc: benedikt.huber@theobroma-systems.com; philipp.tomsich@theobroma-
> >>> systems.com
> >>> Subject: [PATCH] [aarch64] Implemented reciprocal square root
> >>> (rsqrt) estimation in -ffast-math
> >>>
> >>> arch64 offers the instructions frsqrte and frsqrts, for rsqrt
> >>> estimation
> >> and
> >>> a Newton-Raphson step, respectively.
> >>> There are ARMv8 implementations where this is faster than using fdiv
> >>> and rsqrt.
> >>> It runs three steps for double and two steps for float to achieve
> >>> the
> >> needed
> >>> precision.
> >>>
> >>> There is one caveat and open question.
> >>> Since -ffast-math enables flush to zero intermediate values between
> >>> approximation steps will be flushed to zero if they are denormal.
> >>> E.g. This happens in the case of rsqrt (DBL_MAX) and rsqrtf (FLT_MAX).
> >>> The test cases pass, but it is unclear to me whether this is
> >>> expected behavior with -ffast-math.
> >>>
> >>> The patch applies to commit:
> >>> svn+ssh://gcc.gnu.org/svn/gcc/trunk@224470
> >>>
> >>> Please consider including this patch.
> >>> Thank you and best regards,
> >>> Benedikt Huber
> >>>
> >>> Benedikt Huber (1):
> >>> 2015-06-15  Benedikt Huber  <benedikt.huber@theobroma-systems.com>
> >>>
> >>> gcc/ChangeLog                            |   9 +++
> >>> gcc/config/aarch64/aarch64-builtins.c    |  60 ++++++++++++++++
> >>> gcc/config/aarch64/aarch64-protos.h      |   2 +
> >>> gcc/config/aarch64/aarch64-simd.md       |  27 ++++++++
> >>> gcc/config/aarch64/aarch64.c             |  63 +++++++++++++++++
> >>> gcc/config/aarch64/aarch64.md            |   3 +
> >>> gcc/testsuite/gcc.target/aarch64/rsqrt.c | 113
> >>> +++++++++++++++++++++++++++++++
> >>> 7 files changed, 277 insertions(+)
> >>> create mode 100644 gcc/testsuite/gcc.target/aarch64/rsqrt.c
> >>>
> >>> --
> >>> 1.9.1
> >> <Mail Attachment.eml>
> >

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Dr. Philipp Tomsich]

Evandro,

Shouldn't ‘execute_cse_reciprocals_1’ take care of this, once the reciprocal-division is implemented?
Do you think there’s additional work needed to catch all cases/opportunities?

Best,
Philipp.

> On 24 Jun 2015, at 20:19, Evandro Menezes <e.menezes@samsung.com> wrote:
> 
> Benedikt,
> 
> Are you developing the reciprocal approximation just for 1/x proper or for any division, as in x/y = x * 1/y?
> 
> Thank you,
> 
> -- 
> Evandro Menezes                              Austin, TX
> 
> 
>> -----Original Message-----
>> From: Benedikt Huber [mailto:benedikt.huber@theobroma-systems.com]
>> Sent: Wednesday, June 24, 2015 12:11
>> To: Dr. Philipp Tomsich
>> Cc: Evandro Menezes; gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
>> estimation in -ffast-math
>> 
>> Evandro,
>> 
>> Yes, we also have the 1/x approximation.
>> However we do not have the test cases yet, and it also would need some clean
>> up.
>> I am going to provide a patch for that soon (say next week).
>> Also, for this optimization we have *not* yet found a benchmark with
>> significant improvements.
>> 
>> Best Regards,
>> Benedikt
>> 
>> 
>>> On 24 Jun 2015, at 18:52, Dr. Philipp Tomsich <philipp.tomsich@theobroma-
>> systems.com> wrote:
>>> 
>>> Evandro,
>>> 
>>> We’ve seen a 28% speed-up on gromacs in SPECfp for the (scalar) reciprocal
>> sqrt.
>>> 
>>> Also, the “reciprocal divide” patches are floating around in various
>>> of our git-tree, but aren’t ready for public consumption, yet… I’ll
>>> leave Benedikt to comment on potential timelines for getting that pushed
>> out.
>>> 
>>> Best,
>>> Philipp.
>>> 
>>>> On 24 Jun 2015, at 18:42, Evandro Menezes <e.menezes@samsung.com> wrote:
>>>> 
>>>> Benedikt,
>>>> 
>>>> You beat me to it! :-)  Do you have the implementation for dividing
>>>> using the Newton series as well?
>>>> 
>>>> I'm not sure that the series is always for all data types and on all
>>>> processors.  It would be useful to allow each AArch64 processor to
>>>> enable this or not depending on the data type.  BTW, do you have some
>>>> tests showing the speed up?
>>>> 
>>>> Thank you,
>>>> 
>>>> --
>>>> Evandro Menezes                              Austin, TX
>>>> 
>>>>> -----Original Message-----
>>>>> From: gcc-patches-owner@gcc.gnu.org
>>>>> [mailto:gcc-patches-owner@gcc.gnu.org]
>>>> On
>>>>> Behalf Of Benedikt Huber
>>>>> Sent: Thursday, June 18, 2015 7:04
>>>>> To: gcc-patches@gcc.gnu.org
>>>>> Cc: benedikt.huber@theobroma-systems.com; philipp.tomsich@theobroma-
>>>>> systems.com
>>>>> Subject: [PATCH] [aarch64] Implemented reciprocal square root
>>>>> (rsqrt) estimation in -ffast-math
>>>>> 
>>>>> arch64 offers the instructions frsqrte and frsqrts, for rsqrt
>>>>> estimation
>>>> and
>>>>> a Newton-Raphson step, respectively.
>>>>> There are ARMv8 implementations where this is faster than using fdiv
>>>>> and rsqrt.
>>>>> It runs three steps for double and two steps for float to achieve
>>>>> the
>>>> needed
>>>>> precision.
>>>>> 
>>>>> There is one caveat and open question.
>>>>> Since -ffast-math enables flush to zero intermediate values between
>>>>> approximation steps will be flushed to zero if they are denormal.
>>>>> E.g. This happens in the case of rsqrt (DBL_MAX) and rsqrtf (FLT_MAX).
>>>>> The test cases pass, but it is unclear to me whether this is
>>>>> expected behavior with -ffast-math.
>>>>> 
>>>>> The patch applies to commit:
>>>>> svn+ssh://gcc.gnu.org/svn/gcc/trunk@224470
>>>>> 
>>>>> Please consider including this patch.
>>>>> Thank you and best regards,
>>>>> Benedikt Huber
>>>>> 
>>>>> Benedikt Huber (1):
>>>>> 2015-06-15  Benedikt Huber  <benedikt.huber@theobroma-systems.com>
>>>>> 
>>>>> gcc/ChangeLog                            |   9 +++
>>>>> gcc/config/aarch64/aarch64-builtins.c    |  60 ++++++++++++++++
>>>>> gcc/config/aarch64/aarch64-protos.h      |   2 +
>>>>> gcc/config/aarch64/aarch64-simd.md       |  27 ++++++++
>>>>> gcc/config/aarch64/aarch64.c             |  63 +++++++++++++++++
>>>>> gcc/config/aarch64/aarch64.md            |   3 +
>>>>> gcc/testsuite/gcc.target/aarch64/rsqrt.c | 113
>>>>> +++++++++++++++++++++++++++++++
>>>>> 7 files changed, 277 insertions(+)
>>>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/rsqrt.c
>>>>> 
>>>>> --
>>>>> 1.9.1
>>>> <Mail Attachment.eml>
>>> 
> 
> 

RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Evandro Menezes]

Philipp,

I think that execute_cse_reciprocals_1() applies only when the denominator is known at compile-time, otherwise the division stays.  It doesn't seem to know whether the target supports the approximate reciprocal or not.

Cheers,

-- 
Evandro Menezes                              Austin, TX


> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org] On
> Behalf Of Dr. Philipp Tomsich
> Sent: Wednesday, June 24, 2015 15:08
> To: Evandro Menezes
> Cc: Benedikt Huber; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> estimation in -ffast-math
> 
> Evandro,
> 
> Shouldn't ‘execute_cse_reciprocals_1’ take care of this, once the reciprocal-
> division is implemented?
> Do you think there’s additional work needed to catch all cases/opportunities?
> 
> Best,
> Philipp.
> 
> > On 24 Jun 2015, at 20:19, Evandro Menezes <e.menezes@samsung.com> wrote:
> >
> > Benedikt,
> >
> > Are you developing the reciprocal approximation just for 1/x proper or for
> any division, as in x/y = x * 1/y?
> >
> > Thank you,
> >
> > --
> > Evandro Menezes                              Austin, TX
> >
> >
> >> -----Original Message-----
> >> From: Benedikt Huber [mailto:benedikt.huber@theobroma-systems.com]
> >> Sent: Wednesday, June 24, 2015 12:11
> >> To: Dr. Philipp Tomsich
> >> Cc: Evandro Menezes; gcc-patches@gcc.gnu.org
> >> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> >> (rsqrt) estimation in -ffast-math
> >>
> >> Evandro,
> >>
> >> Yes, we also have the 1/x approximation.
> >> However we do not have the test cases yet, and it also would need
> >> some clean up.
> >> I am going to provide a patch for that soon (say next week).
> >> Also, for this optimization we have *not* yet found a benchmark with
> >> significant improvements.
> >>
> >> Best Regards,
> >> Benedikt
> >>
> >>
> >>> On 24 Jun 2015, at 18:52, Dr. Philipp Tomsich
> >>> <philipp.tomsich@theobroma-
> >> systems.com> wrote:
> >>>
> >>> Evandro,
> >>>
> >>> We’ve seen a 28% speed-up on gromacs in SPECfp for the (scalar)
> >>> reciprocal
> >> sqrt.
> >>>
> >>> Also, the “reciprocal divide” patches are floating around in various
> >>> of our git-tree, but aren’t ready for public consumption, yet… I’ll
> >>> leave Benedikt to comment on potential timelines for getting that
> >>> pushed
> >> out.
> >>>
> >>> Best,
> >>> Philipp.
> >>>
> >>>> On 24 Jun 2015, at 18:42, Evandro Menezes <e.menezes@samsung.com> wrote:
> >>>>
> >>>> Benedikt,
> >>>>
> >>>> You beat me to it! :-)  Do you have the implementation for dividing
> >>>> using the Newton series as well?
> >>>>
> >>>> I'm not sure that the series is always for all data types and on
> >>>> all processors.  It would be useful to allow each AArch64 processor
> >>>> to enable this or not depending on the data type.  BTW, do you have
> >>>> some tests showing the speed up?
> >>>>
> >>>> Thank you,
> >>>>
> >>>> --
> >>>> Evandro Menezes                              Austin, TX
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: gcc-patches-owner@gcc.gnu.org
> >>>>> [mailto:gcc-patches-owner@gcc.gnu.org]
> >>>> On
> >>>>> Behalf Of Benedikt Huber
> >>>>> Sent: Thursday, June 18, 2015 7:04
> >>>>> To: gcc-patches@gcc.gnu.org
> >>>>> Cc: benedikt.huber@theobroma-systems.com;
> >>>>> philipp.tomsich@theobroma- systems.com
> >>>>> Subject: [PATCH] [aarch64] Implemented reciprocal square root
> >>>>> (rsqrt) estimation in -ffast-math
> >>>>>
> >>>>> arch64 offers the instructions frsqrte and frsqrts, for rsqrt
> >>>>> estimation
> >>>> and
> >>>>> a Newton-Raphson step, respectively.
> >>>>> There are ARMv8 implementations where this is faster than using
> >>>>> fdiv and rsqrt.
> >>>>> It runs three steps for double and two steps for float to achieve
> >>>>> the
> >>>> needed
> >>>>> precision.
> >>>>>
> >>>>> There is one caveat and open question.
> >>>>> Since -ffast-math enables flush to zero intermediate values
> >>>>> between approximation steps will be flushed to zero if they are
> denormal.
> >>>>> E.g. This happens in the case of rsqrt (DBL_MAX) and rsqrtf (FLT_MAX).
> >>>>> The test cases pass, but it is unclear to me whether this is
> >>>>> expected behavior with -ffast-math.
> >>>>>
> >>>>> The patch applies to commit:
> >>>>> svn+ssh://gcc.gnu.org/svn/gcc/trunk@224470
> >>>>>
> >>>>> Please consider including this patch.
> >>>>> Thank you and best regards,
> >>>>> Benedikt Huber
> >>>>>
> >>>>> Benedikt Huber (1):
> >>>>> 2015-06-15  Benedikt Huber  <benedikt.huber@theobroma-systems.com>
> >>>>>
> >>>>> gcc/ChangeLog                            |   9 +++
> >>>>> gcc/config/aarch64/aarch64-builtins.c    |  60 ++++++++++++++++
> >>>>> gcc/config/aarch64/aarch64-protos.h      |   2 +
> >>>>> gcc/config/aarch64/aarch64-simd.md       |  27 ++++++++
> >>>>> gcc/config/aarch64/aarch64.c             |  63 +++++++++++++++++
> >>>>> gcc/config/aarch64/aarch64.md            |   3 +
> >>>>> gcc/testsuite/gcc.target/aarch64/rsqrt.c | 113
> >>>>> +++++++++++++++++++++++++++++++
> >>>>> 7 files changed, 277 insertions(+) create mode 100644
> >>>>> gcc/testsuite/gcc.target/aarch64/rsqrt.c
> >>>>>
> >>>>> --
> >>>>> 1.9.1
> >>>> <Mail Attachment.eml>
> >>>
> >
> >

RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Kumar, Venkataramanan]

Hi, 

If I understand correct, current implementation replaces 

fdiv 
fsqrt

 by  
 frsqrte
for i=0 to 3
fmul
frsqrts  
fmul

So I think gains depends latency of  frsqrts  insn.

I see patch has patterns for  vector versions of frsqrts, but does not enable them?

Regards,
Venkat.

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Dr. Philipp Tomsich
> Sent: Wednesday, June 24, 2015 10:22 PM
> To: Evandro Menezes
> Cc: Benedikt Huber; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> estimation in -ffast-math
> 
> Evandro,
> 
> We’ve seen a 28% speed-up on gromacs in SPECfp for the (scalar) reciprocal
> sqrt.
> 
> Also, the “reciprocal divide” patches are floating around in various of our git-
> tree, but aren’t ready for public consumption, yet… I’ll leave Benedikt to
> comment on potential timelines for getting that pushed out.
> 
> Best,
> Philipp.
> 
> > On 24 Jun 2015, at 18:42, Evandro Menezes <e.menezes@samsung.com>
> wrote:
> >
> > Benedikt,
> >
> > You beat me to it! :-)  Do you have the implementation for dividing
> > using the Newton series as well?
> >
> > I'm not sure that the series is always for all data types and on all
> > processors.  It would be useful to allow each AArch64 processor to
> > enable this or not depending on the data type.  BTW, do you have some
> > tests showing the speed up?
> >
> > Thank you,
> >
> > --
> > Evandro Menezes                              Austin, TX
> >
> >> -----Original Message-----
> >> From: gcc-patches-owner@gcc.gnu.org
> >> [mailto:gcc-patches-owner@gcc.gnu.org]
> > On
> >> Behalf Of Benedikt Huber
> >> Sent: Thursday, June 18, 2015 7:04
> >> To: gcc-patches@gcc.gnu.org
> >> Cc: benedikt.huber@theobroma-systems.com;
> philipp.tomsich@theobroma-
> >> systems.com
> >> Subject: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> >> estimation in -ffast-math
> >>
> >> arch64 offers the instructions frsqrte and frsqrts, for rsqrt
> >> estimation
> > and
> >> a Newton-Raphson step, respectively.
> >> There are ARMv8 implementations where this is faster than using fdiv
> >> and rsqrt.
> >> It runs three steps for double and two steps for float to achieve the
> > needed
> >> precision.
> >>
> >> There is one caveat and open question.
> >> Since -ffast-math enables flush to zero intermediate values between
> >> approximation steps will be flushed to zero if they are denormal.
> >> E.g. This happens in the case of rsqrt (DBL_MAX) and rsqrtf (FLT_MAX).
> >> The test cases pass, but it is unclear to me whether this is expected
> >> behavior with -ffast-math.
> >>
> >> The patch applies to commit:
> >> svn+ssh://gcc.gnu.org/svn/gcc/trunk@224470
> >>
> >> Please consider including this patch.
> >> Thank you and best regards,
> >> Benedikt Huber
> >>
> >> Benedikt Huber (1):
> >>  2015-06-15  Benedikt Huber  <benedikt.huber@theobroma-
> systems.com>
> >>
> >> gcc/ChangeLog                            |   9 +++
> >> gcc/config/aarch64/aarch64-builtins.c    |  60 ++++++++++++++++
> >> gcc/config/aarch64/aarch64-protos.h      |   2 +
> >> gcc/config/aarch64/aarch64-simd.md       |  27 ++++++++
> >> gcc/config/aarch64/aarch64.c             |  63 +++++++++++++++++
> >> gcc/config/aarch64/aarch64.md            |   3 +
> >> gcc/testsuite/gcc.target/aarch64/rsqrt.c | 113
> >> +++++++++++++++++++++++++++++++
> >> 7 files changed, 277 insertions(+)
> >> create mode 100644 gcc/testsuite/gcc.target/aarch64/rsqrt.c
> >>
> >> --
> >> 1.9.1
> > <Mail Attachment.eml>

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[pinskia]

> On Jun 18, 2015, at 5:04 AM, Benedikt Huber <benedikt.huber@theobroma-systems.com> wrote:
> 
> arch64 offers the instructions frsqrte and frsqrts, for rsqrt estimation and
> a Newton-Raphson step, respectively.
> There are ARMv8 implementations where this is faster than using fdiv and rsqrt.
> It runs three steps for double and two steps for float to achieve the needed precision.

This is NOT a win on thunderX at least for single precision because you have to do the divide and sqrt in the same time as it takes 5 multiples (estimate and step are multiplies in the thunderX pipeline).  Doubles is 10 multiplies which is just the same as what the patch does (but it is really slightly less than 10, I rounded up). So in the end this is NOT a win at all for thunderX unless we do one less step for both single and double. 

Thanks,
Andrew


> 
> There is one caveat and open question.
> Since -ffast-math enables flush to zero intermediate values between approximation steps
> will be flushed to zero if they are denormal.
> E.g. This happens in the case of rsqrt (DBL_MAX) and rsqrtf (FLT_MAX).
> The test cases pass, but it is unclear to me whether this is expected behavior with -ffast-math.
> 
> The patch applies to commit:
> svn+ssh://gcc.gnu.org/svn/gcc/trunk@224470
> 
> Please consider including this patch.
> Thank you and best regards,
> Benedikt Huber
> 
> Benedikt Huber (1):
>  2015-06-15  Benedikt Huber  <benedikt.huber@theobroma-systems.com>
> 
> gcc/ChangeLog                            |   9 +++
> gcc/config/aarch64/aarch64-builtins.c    |  60 ++++++++++++++++
> gcc/config/aarch64/aarch64-protos.h      |   2 +
> gcc/config/aarch64/aarch64-simd.md       |  27 ++++++++
> gcc/config/aarch64/aarch64.c             |  63 +++++++++++++++++
> gcc/config/aarch64/aarch64.md            |   3 +
> gcc/testsuite/gcc.target/aarch64/rsqrt.c | 113 +++++++++++++++++++++++++++++++
> 7 files changed, 277 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/rsqrt.c
> 
> -- 
> 1.9.1
> 

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Ramana Radhakrishnan]

Benedikt,

On 25/06/15 08:01, pinskia@gmail.com wrote:
>
>
>
>
>> On Jun 18, 2015, at 5:04 AM, Benedikt Huber <benedikt.huber@theobroma-systems.com> wrote:
>>
>> arch64 offers the instructions frsqrte and frsqrts, for rsqrt estimation and
>> a Newton-Raphson step, respectively.
>> There are ARMv8 implementations where this is faster than using fdiv and rsqrt.
>> It runs three steps for double and two steps for float to achieve the needed precision.
>
> This is NOT a win on thunderX at least for single precision because you have to do the divide and sqrt in the same time as it takes 5 multiples (estimate and step are multiplies in the thunderX pipeline).  Doubles is 10 multiplies which is just the same as what the patch does (but it is really slightly less than 10, I rounded up). So in the end this is NOT a win at all for thunderX unless we do one less step for both single and double.
>


Have you seen this 
https://gcc.gnu.org/ml/gcc-patches/2015-03/msg00164.html ? Really this 
is something that should be gated by the costs infrastructure .


regards
Ramana





> Thanks,
> Andrew
>
>
>>
>> There is one caveat and open question.
>> Since -ffast-math enables flush to zero intermediate values between approximation steps
>> will be flushed to zero if they are denormal.
>> E.g. This happens in the case of rsqrt (DBL_MAX) and rsqrtf (FLT_MAX).
>> The test cases pass, but it is unclear to me whether this is expected behavior with -ffast-math.
>>
>> The patch applies to commit:
>> svn+ssh://gcc.gnu.org/svn/gcc/trunk@224470
>>
>> Please consider including this patch.
>> Thank you and best regards,
>> Benedikt Huber
>>
>> Benedikt Huber (1):
>>   2015-06-15  Benedikt Huber  <benedikt.huber@theobroma-systems.com>
>>
>> gcc/ChangeLog                            |   9 +++
>> gcc/config/aarch64/aarch64-builtins.c    |  60 ++++++++++++++++
>> gcc/config/aarch64/aarch64-protos.h      |   2 +
>> gcc/config/aarch64/aarch64-simd.md       |  27 ++++++++
>> gcc/config/aarch64/aarch64.c             |  63 +++++++++++++++++
>> gcc/config/aarch64/aarch64.md            |   3 +
>> gcc/testsuite/gcc.target/aarch64/rsqrt.c | 113 +++++++++++++++++++++++++++++++
>> 7 files changed, 277 insertions(+)
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/rsqrt.c
>>
>> --
>> 1.9.1
>>

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Benedikt Huber]

[-- Attachment #1: Type: text/plain, Size: 822 bytes --]

Andrew,

> This is NOT a win on thunderX at least for single precision because you have to do the divide and sqrt in the same time as it takes 5 multiples (estimate and step are multiplies in the thunderX pipeline).  Doubles is 10 multiplies which is just the same as what the patch does (but it is really slightly less than 10, I rounded up). So in the end this is NOT a win at all for thunderX unless we do one less step for both single and double.

Yes, the expected benefit from rsqrt estimation is implementation specific. If one has a better initial rsqrte or an application that can trade precision for execution time, we could offer a command line option to do only 2 steps for doulbe and 1 step for float; similar to -mrecip-precision for PowerPC.
What are your thoughts on that?

Best regards,
Benedikt

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 496 bytes --]

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Benedikt Huber]

[-- Attachment #1: Type: text/plain, Size: 6243 bytes --]

I did not look at execute_cse_reciprocals_1(), yet.

However, with the recip-patch applied:

double recipd (double a, double b)
{
  return a/b;
}

translates to

recipd:
  frecpe  d2, d1
  frecps  d3, d2, d1
  fmul  d2, d2, d3
  frecps  d3, d2, d1
  fmul  d2, d2, d3
  frecps  d1, d2, d1
  fmul  d2, d2, d1
  fmul  d0, d2, d0
  ret

float recipf (float a, float b)
{
  return a/b;
}

translates to

recipf:
  frecpe  s2, s1
  frecps  s3, s2, s1
  fmul  s2, s2, s3
  frecps  s1, s2, s1
  fmul  s2, s2, s1
  fmul  s0, s2, s0
  ret

So it seems, that it works also for a generic division.

Best regards,
Benedikt

> On 24 Jun 2015, at 22:39, Evandro Menezes <e.menezes@samsung.com> wrote:
> 
> Philipp,
> 
> I think that execute_cse_reciprocals_1() applies only when the denominator is known at compile-time, otherwise the division stays.  It doesn't seem to know whether the target supports the approximate reciprocal or not.
> 
> Cheers,
> 
> --
> Evandro Menezes                              Austin, TX
> 
> 
>> -----Original Message-----
>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org] On
>> Behalf Of Dr. Philipp Tomsich
>> Sent: Wednesday, June 24, 2015 15:08
>> To: Evandro Menezes
>> Cc: Benedikt Huber; gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
>> estimation in -ffast-math
>> 
>> Evandro,
>> 
>> Shouldn't ‘execute_cse_reciprocals_1’ take care of this, once the reciprocal-
>> division is implemented?
>> Do you think there’s additional work needed to catch all cases/opportunities?
>> 
>> Best,
>> Philipp.
>> 
>>> On 24 Jun 2015, at 20:19, Evandro Menezes <e.menezes@samsung.com> wrote:
>>> 
>>> Benedikt,
>>> 
>>> Are you developing the reciprocal approximation just for 1/x proper or for
>> any division, as in x/y = x * 1/y?
>>> 
>>> Thank you,
>>> 
>>> --
>>> Evandro Menezes                              Austin, TX
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: Benedikt Huber [mailto:benedikt.huber@theobroma-systems.com]
>>>> Sent: Wednesday, June 24, 2015 12:11
>>>> To: Dr. Philipp Tomsich
>>>> Cc: Evandro Menezes; gcc-patches@gcc.gnu.org
>>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
>>>> (rsqrt) estimation in -ffast-math
>>>> 
>>>> Evandro,
>>>> 
>>>> Yes, we also have the 1/x approximation.
>>>> However we do not have the test cases yet, and it also would need
>>>> some clean up.
>>>> I am going to provide a patch for that soon (say next week).
>>>> Also, for this optimization we have *not* yet found a benchmark with
>>>> significant improvements.
>>>> 
>>>> Best Regards,
>>>> Benedikt
>>>> 
>>>> 
>>>>> On 24 Jun 2015, at 18:52, Dr. Philipp Tomsich
>>>>> <philipp.tomsich@theobroma-
>>>> systems.com> wrote:
>>>>> 
>>>>> Evandro,
>>>>> 
>>>>> We’ve seen a 28% speed-up on gromacs in SPECfp for the (scalar)
>>>>> reciprocal
>>>> sqrt.
>>>>> 
>>>>> Also, the “reciprocal divide” patches are floating around in various
>>>>> of our git-tree, but aren’t ready for public consumption, yet… I’ll
>>>>> leave Benedikt to comment on potential timelines for getting that
>>>>> pushed
>>>> out.
>>>>> 
>>>>> Best,
>>>>> Philipp.
>>>>> 
>>>>>> On 24 Jun 2015, at 18:42, Evandro Menezes <e.menezes@samsung.com> wrote:
>>>>>> 
>>>>>> Benedikt,
>>>>>> 
>>>>>> You beat me to it! :-)  Do you have the implementation for dividing
>>>>>> using the Newton series as well?
>>>>>> 
>>>>>> I'm not sure that the series is always for all data types and on
>>>>>> all processors.  It would be useful to allow each AArch64 processor
>>>>>> to enable this or not depending on the data type.  BTW, do you have
>>>>>> some tests showing the speed up?
>>>>>> 
>>>>>> Thank you,
>>>>>> 
>>>>>> --
>>>>>> Evandro Menezes                              Austin, TX
>>>>>> 
>>>>>>> -----Original Message-----
>>>>>>> From: gcc-patches-owner@gcc.gnu.org
>>>>>>> [mailto:gcc-patches-owner@gcc.gnu.org]
>>>>>> On
>>>>>>> Behalf Of Benedikt Huber
>>>>>>> Sent: Thursday, June 18, 2015 7:04
>>>>>>> To: gcc-patches@gcc.gnu.org
>>>>>>> Cc: benedikt.huber@theobroma-systems.com;
>>>>>>> philipp.tomsich@theobroma- systems.com
>>>>>>> Subject: [PATCH] [aarch64] Implemented reciprocal square root
>>>>>>> (rsqrt) estimation in -ffast-math
>>>>>>> 
>>>>>>> arch64 offers the instructions frsqrte and frsqrts, for rsqrt
>>>>>>> estimation
>>>>>> and
>>>>>>> a Newton-Raphson step, respectively.
>>>>>>> There are ARMv8 implementations where this is faster than using
>>>>>>> fdiv and rsqrt.
>>>>>>> It runs three steps for double and two steps for float to achieve
>>>>>>> the
>>>>>> needed
>>>>>>> precision.
>>>>>>> 
>>>>>>> There is one caveat and open question.
>>>>>>> Since -ffast-math enables flush to zero intermediate values
>>>>>>> between approximation steps will be flushed to zero if they are
>> denormal.
>>>>>>> E.g. This happens in the case of rsqrt (DBL_MAX) and rsqrtf (FLT_MAX).
>>>>>>> The test cases pass, but it is unclear to me whether this is
>>>>>>> expected behavior with -ffast-math.
>>>>>>> 
>>>>>>> The patch applies to commit:
>>>>>>> svn+ssh://gcc.gnu.org/svn/gcc/trunk@224470
>>>>>>> 
>>>>>>> Please consider including this patch.
>>>>>>> Thank you and best regards,
>>>>>>> Benedikt Huber
>>>>>>> 
>>>>>>> Benedikt Huber (1):
>>>>>>> 2015-06-15  Benedikt Huber  <benedikt.huber@theobroma-systems.com>
>>>>>>> 
>>>>>>> gcc/ChangeLog                            |   9 +++
>>>>>>> gcc/config/aarch64/aarch64-builtins.c    |  60 ++++++++++++++++
>>>>>>> gcc/config/aarch64/aarch64-protos.h      |   2 +
>>>>>>> gcc/config/aarch64/aarch64-simd.md       |  27 ++++++++
>>>>>>> gcc/config/aarch64/aarch64.c             |  63 +++++++++++++++++
>>>>>>> gcc/config/aarch64/aarch64.md            |   3 +
>>>>>>> gcc/testsuite/gcc.target/aarch64/rsqrt.c | 113
>>>>>>> +++++++++++++++++++++++++++++++
>>>>>>> 7 files changed, 277 insertions(+) create mode 100644
>>>>>>> gcc/testsuite/gcc.target/aarch64/rsqrt.c
>>>>>>> 
>>>>>>> --
>>>>>>> 1.9.1
>>>>>> <Mail Attachment.eml>
>>>>> 
>>> 
>>> 
> 


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 496 bytes --]

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Michael Matz]

Hi,

On Thu, 25 Jun 2015, Benedikt Huber wrote:

> > This is NOT a win on thunderX at least for single precision because 
> > you have to do the divide and sqrt in the same time as it takes 5 
> > multiples (estimate and step are multiplies in the thunderX pipeline).  
> > Doubles is 10 multiplies which is just the same as what the patch does 
> > (but it is really slightly less than 10, I rounded up). So in the end 
> > this is NOT a win at all for thunderX unless we do one less step for 
> > both single and double.
> 
> Yes, the expected benefit from rsqrt estimation is implementation 
> specific. If one has a better initial rsqrte or an application that can 
> trade precision for execution time, we could offer a command line option 
> to do only 2 steps for doulbe and 1 step for float; similar to 
> -mrecip-precision for PowerPC. What are your thoughts on that?

On x86-64, under -ffast-math we only do one NR step.  Generally the 
rule-of-thumb take on fast-math is, that common benchmarks should still 
validate with that option in effect.

(And yes, I also never found a speedup for approximated reciprocals so 
that benchmarks would still generally validate, you always had to do two 
NR steps, and then it became as slow as a general divide).  See also 
http://gcc.gnu.org/ml/gcc-patches/2009-11/msg00099.html and the followup 
thread.

RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Kumar, Venkataramanan]

Changing to  "1 step for float" and "2 steps for double" gives better gains now for gromacs on cortex-a57.

Regards,
Venkat.
> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Benedikt Huber
> Sent: Thursday, June 25, 2015 4:09 PM
> To: pinskia@gmail.com
> Cc: gcc-patches@gcc.gnu.org; philipp.tomsich@theobroma-systems.com
> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> estimation in -ffast-math
> 
> Andrew,
> 
> > This is NOT a win on thunderX at least for single precision because you have
> to do the divide and sqrt in the same time as it takes 5 multiples (estimate
> and step are multiplies in the thunderX pipeline).  Doubles is 10 multiplies
> which is just the same as what the patch does (but it is really slightly less than
> 10, I rounded up). So in the end this is NOT a win at all for thunderX unless
> we do one less step for both single and double.
> 
> Yes, the expected benefit from rsqrt estimation is implementation specific. If
> one has a better initial rsqrte or an application that can trade precision for
> execution time, we could offer a command line option to do only 2 steps for
> doulbe and 1 step for float; similar to -mrecip-precision for PowerPC.
> What are your thoughts on that?
> 
> Best regards,
> Benedikt

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Dr. Philipp Tomsich]

Kumar,

what is the relative gain that you see on Cortex-A57?

Thanks,
Philipp.

> On 25 Jun 2015, at 17:35, Kumar, Venkataramanan <Venkataramanan.Kumar@amd.com> wrote:
> 
> Changing to  "1 step for float" and "2 steps for double" gives better gains now for gromacs on cortex-a57.
> 
> Regards,
> Venkat.
>> -----Original Message-----
>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
>> owner@gcc.gnu.org] On Behalf Of Benedikt Huber
>> Sent: Thursday, June 25, 2015 4:09 PM
>> To: pinskia@gmail.com
>> Cc: gcc-patches@gcc.gnu.org; philipp.tomsich@theobroma-systems.com
>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
>> estimation in -ffast-math
>> 
>> Andrew,
>> 
>>> This is NOT a win on thunderX at least for single precision because you have
>> to do the divide and sqrt in the same time as it takes 5 multiples (estimate
>> and step are multiplies in the thunderX pipeline).  Doubles is 10 multiplies
>> which is just the same as what the patch does (but it is really slightly less than
>> 10, I rounded up). So in the end this is NOT a win at all for thunderX unless
>> we do one less step for both single and double.
>> 
>> Yes, the expected benefit from rsqrt estimation is implementation specific. If
>> one has a better initial rsqrte or an application that can trade precision for
>> execution time, we could offer a command line option to do only 2 steps for
>> doulbe and 1 step for float; similar to -mrecip-precision for PowerPC.
>> What are your thoughts on that?
>> 
>> Best regards,
>> Benedikt

RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Kumar, Venkataramanan]

I got around ~12% gain with -Ofast -mcpu=cortex-a57.

Regards,
Venkat.

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-	
> owner@gcc.gnu.org] On Behalf Of Dr. Philipp Tomsich
> Sent: Thursday, June 25, 2015 9:13 PM
> To: Kumar, Venkataramanan
> Cc: Benedikt Huber; pinskia@gmail.com; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> estimation in -ffast-math
> 
> Kumar,
> 
> what is the relative gain that you see on Cortex-A57?
> 
> Thanks,
> Philipp.
> 
> > On 25 Jun 2015, at 17:35, Kumar, Venkataramanan
> <Venkataramanan.Kumar@amd.com> wrote:
> >
> > Changing to  "1 step for float" and "2 steps for double" gives better gains
> now for gromacs on cortex-a57.
> >
> > Regards,
> > Venkat.
> >> -----Original Message-----
> >> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> >> owner@gcc.gnu.org] On Behalf Of Benedikt Huber
> >> Sent: Thursday, June 25, 2015 4:09 PM
> >> To: pinskia@gmail.com
> >> Cc: gcc-patches@gcc.gnu.org; philipp.tomsich@theobroma-systems.com
> >> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> >> (rsqrt) estimation in -ffast-math
> >>
> >> Andrew,
> >>
> >>> This is NOT a win on thunderX at least for single precision because
> >>> you have
> >> to do the divide and sqrt in the same time as it takes 5 multiples
> >> (estimate and step are multiplies in the thunderX pipeline).  Doubles
> >> is 10 multiplies which is just the same as what the patch does (but
> >> it is really slightly less than 10, I rounded up). So in the end this
> >> is NOT a win at all for thunderX unless we do one less step for both single
> and double.
> >>
> >> Yes, the expected benefit from rsqrt estimation is implementation
> >> specific. If one has a better initial rsqrte or an application that
> >> can trade precision for execution time, we could offer a command line
> >> option to do only 2 steps for doulbe and 1 step for float; similar to -
> mrecip-precision for PowerPC.
> >> What are your thoughts on that?
> >>
> >> Best regards,
> >> Benedikt

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Andrew Pinski]

On Thu, Jun 25, 2015 at 1:24 AM, Ramana Radhakrishnan
<ramana.radhakrishnan@foss.arm.com> wrote:
> Benedikt,
>
> On 25/06/15 08:01, pinskia@gmail.com wrote:
>>
>>
>>
>>
>>
>>> On Jun 18, 2015, at 5:04 AM, Benedikt Huber
>>> <benedikt.huber@theobroma-systems.com> wrote:
>>>
>>> arch64 offers the instructions frsqrte and frsqrts, for rsqrt estimation
>>> and
>>> a Newton-Raphson step, respectively.
>>> There are ARMv8 implementations where this is faster than using fdiv and
>>> rsqrt.
>>> It runs three steps for double and two steps for float to achieve the
>>> needed precision.
>>
>>
>> This is NOT a win on thunderX at least for single precision because you
>> have to do the divide and sqrt in the same time as it takes 5 multiples
>> (estimate and step are multiplies in the thunderX pipeline).  Doubles is 10
>> multiplies which is just the same as what the patch does (but it is really
>> slightly less than 10, I rounded up). So in the end this is NOT a win at all
>> for thunderX unless we do one less step for both single and double.
>>
>
>
> Have you seen this https://gcc.gnu.org/ml/gcc-patches/2015-03/msg00164.html
> ? Really this is something that should be gated by the costs infrastructure


Yes I saw that in fact I did not look into the latencies of our core
until this patch came out.  But yes this should be gated by a cost
infrastructure and most likely not as part of the -mcpu=generic cost
(well the rsqrt if we change it to 1 iterations and 2 iterations).

Thanks,
Andrew

> .
>
>
> regards
> Ramana
>
>
>
>
>
>
>> Thanks,
>> Andrew
>>
>>
>>>
>>> There is one caveat and open question.
>>> Since -ffast-math enables flush to zero intermediate values between
>>> approximation steps
>>> will be flushed to zero if they are denormal.
>>> E.g. This happens in the case of rsqrt (DBL_MAX) and rsqrtf (FLT_MAX).
>>> The test cases pass, but it is unclear to me whether this is expected
>>> behavior with -ffast-math.
>>>
>>> The patch applies to commit:
>>> svn+ssh://gcc.gnu.org/svn/gcc/trunk@224470
>>>
>>> Please consider including this patch.
>>> Thank you and best regards,
>>> Benedikt Huber
>>>
>>> Benedikt Huber (1):
>>>   2015-06-15  Benedikt Huber  <benedikt.huber@theobroma-systems.com>
>>>
>>> gcc/ChangeLog                            |   9 +++
>>> gcc/config/aarch64/aarch64-builtins.c    |  60 ++++++++++++++++
>>> gcc/config/aarch64/aarch64-protos.h      |   2 +
>>> gcc/config/aarch64/aarch64-simd.md       |  27 ++++++++
>>> gcc/config/aarch64/aarch64.c             |  63 +++++++++++++++++
>>> gcc/config/aarch64/aarch64.md            |   3 +
>>> gcc/testsuite/gcc.target/aarch64/rsqrt.c | 113
>>> +++++++++++++++++++++++++++++++
>>> 7 files changed, 277 insertions(+)
>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/rsqrt.c
>>>
>>> --
>>> 1.9.1
>>>
>

Re: [PATCH] 2015-06-15 Benedikt Huber <benedikt.huber@theobroma-systems.com>

[Andrew Pinski]

On Thu, Jun 18, 2015 at 5:04 AM, Benedikt Huber
<benedikt.huber@theobroma-systems.com> wrote:
A few comments about the patch itself besides the cost model issue and
iteration issue mentioned in the other email thread.

>        * config/aarch64/aarch64-builtins.c: Builtins for rsqrt and rsqrtf.
>        * config/aarch64/aarch64-protos.h: Declare.
>        * config/aarch64/aarch64-simd.md: Matching expressions for frsqrte and frsqrts.
>        * config/aarch64/aarch64.c: New functions. Emit rsqrt estimation code in fast math mode.
>        * config/aarch64/aarch64.md: Added enum entry.
>        * testsuite/gcc.target/aarch64/rsqrt.c: Tests for single and double.
> ---
>  gcc/ChangeLog                            |   9 +++
>  gcc/config/aarch64/aarch64-builtins.c    |  60 ++++++++++++++++
>  gcc/config/aarch64/aarch64-protos.h      |   2 +
>  gcc/config/aarch64/aarch64-simd.md       |  27 ++++++++
>  gcc/config/aarch64/aarch64.c             |  63 +++++++++++++++++
>  gcc/config/aarch64/aarch64.md            |   3 +
>  gcc/testsuite/gcc.target/aarch64/rsqrt.c | 113 +++++++++++++++++++++++++++++++
>  7 files changed, 277 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/rsqrt.c
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index c9b156f..690ebba 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,12 @@
> +2015-06-15  Benedikt Huber  <benedikt.huber@theobroma-systems.com>
> +
> +       * config/aarch64/aarch64-builtins.c: Builtins for rsqrt and rsqrtf.
> +       * config/aarch64/aarch64-protos.h: Declare.
> +       * config/aarch64/aarch64-simd.md: Matching expressions for frsqrte and frsqrts.
> +       * config/aarch64/aarch64.c: New functions. Emit rsqrt estimation code in fast math mode.
> +       * config/aarch64/aarch64.md: Added enum entry.
> +       * testsuite/gcc.target/aarch64/rsqrt.c: Tests for single and double.
> +
>  2015-06-14  Richard Sandiford  <richard.sandiford@arm.com>
>
>         * rtl.h (classify_insn): Declare.
> diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
> index f7a39ec..484bb84 100644
> --- a/gcc/config/aarch64/aarch64-builtins.c
> +++ b/gcc/config/aarch64/aarch64-builtins.c
> @@ -342,6 +342,8 @@ enum aarch64_builtins
>    AARCH64_BUILTIN_GET_FPSR,
>    AARCH64_BUILTIN_SET_FPSR,
>
> +  AARCH64_BUILTIN_RSQRT,
> +  AARCH64_BUILTIN_RSQRTF,
>    AARCH64_SIMD_BUILTIN_BASE,
>    AARCH64_SIMD_BUILTIN_LANE_CHECK,
>  #include "aarch64-simd-builtins.def"
> @@ -831,6 +833,32 @@ aarch64_init_crc32_builtins ()
>  }
>
>  void
> +aarch64_add_builtin_rsqrt (void)
> +{
> +  tree fndecl = NULL;
> +  tree ftype = NULL;
> +  ftype = build_function_type_list (double_type_node, double_type_node, NULL_TREE);
> +
> +  fndecl = add_builtin_function ("__builtin_aarch64_rsqrt",
> +                                 ftype,
> +                                 AARCH64_BUILTIN_RSQRT,
> +                                 BUILT_IN_MD,
> +                                 NULL,
> +                                 NULL_TREE);
> +  aarch64_builtin_decls[AARCH64_BUILTIN_RSQRT] = fndecl;
> +
> +  tree ftypef = NULL;
> +  ftypef = build_function_type_list (float_type_node, float_type_node, NULL_TREE);
> +  fndecl = add_builtin_function ("__builtin_aarch64_rsqrtf",
> +                                 ftypef,
> +                                 AARCH64_BUILTIN_RSQRTF,
> +                                 BUILT_IN_MD,
> +                                 NULL,
> +                                 NULL_TREE);
> +  aarch64_builtin_decls[AARCH64_BUILTIN_RSQRTF] = fndecl;
> +}
> +
> +void
>  aarch64_init_builtins (void)
>  {
>    tree ftype_set_fpr
> @@ -855,6 +883,7 @@ aarch64_init_builtins (void)
>      aarch64_init_simd_builtins ();
>    if (TARGET_CRC32)
>      aarch64_init_crc32_builtins ();
> +  aarch64_add_builtin_rsqrt ();
>  }
>
>  tree
> @@ -1099,6 +1128,23 @@ aarch64_crc32_expand_builtin (int fcode, tree exp, rtx target)
>    return target;
>  }
>
> +static rtx
> +aarch64_expand_builtin_rsqrt (int fcode, tree exp, rtx target)
> +{
> +  rtx pat;
> +  tree arg0 = CALL_EXPR_ARG (exp, 0);
> +  rtx op0 = expand_normal (arg0);
> +
> +  enum insn_code c = CODE_FOR_rsqrtdf;
> +  if (fcode == AARCH64_BUILTIN_RSQRTF)
> +    c = CODE_FOR_rsqrtsf;
> +
> +  pat = GEN_FCN (c) (target, op0);
> +  emit_insn (pat);
> +
> +  return target;
> +}
> +
>  /* Expand an expression EXP that calls a built-in function,
>     with result going to TARGET if that's convenient.  */
>  rtx
> @@ -1146,6 +1192,11 @@ aarch64_expand_builtin (tree exp,
>    else if (fcode >= AARCH64_CRC32_BUILTIN_BASE && fcode <= AARCH64_CRC32_BUILTIN_MAX)
>      return aarch64_crc32_expand_builtin (fcode, exp, target);
>
> +  if (fcode == AARCH64_BUILTIN_RSQRT ||
> +      fcode == AARCH64_BUILTIN_RSQRTF)
> +    return aarch64_expand_builtin_rsqrt (fcode, exp, target);
> +
> +  return NULL_RTX;
>    gcc_unreachable ();
>  }
>
> @@ -1303,6 +1354,15 @@ aarch64_builtin_vectorized_function (tree fndecl, tree type_out, tree type_in)
>    return NULL_TREE;
>  }
>
> +tree
> +aarch64_builtin_rsqrt (bool is_float)
> +{
> +  if (is_float)
> +    return aarch64_builtin_decls[AARCH64_BUILTIN_RSQRTF];
> +  else
> +    return aarch64_builtin_decls[AARCH64_BUILTIN_RSQRT];
> +}
> +
>  #undef VAR1
>  #define VAR1(T, N, MAP, A) \
>    case AARCH64_SIMD_BUILTIN_##T##_##N##A:
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 965a11b..4f1c8ce 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -270,6 +270,8 @@ void aarch64_print_operand (FILE *, rtx, char);
>  void aarch64_print_operand_address (FILE *, rtx);
>  void aarch64_emit_call_insn (rtx);
>
> +void aarch64_emit_swrsqrt (rtx, rtx);
> +
>  /* Initialize builtins for SIMD intrinsics.  */
>  void init_aarch64_simd_builtins (void);
>
> diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
> index b90f938..266800a 100644
> --- a/gcc/config/aarch64/aarch64-simd.md
> +++ b/gcc/config/aarch64/aarch64-simd.md
> @@ -353,6 +353,33 @@
>    [(set_attr "type" "neon_fp_mul_d_scalar_q")]
>  )
>
> +(define_insn "*rsqrte_simd"
> +  [(set (match_operand:VALLF 0 "register_operand" "=w")
> +       (unspec:VALLF [(match_operand:VALLF 1 "register_operand" "w")]
> +                    UNSPEC_RSQRTE))]
> +  "TARGET_SIMD"
> +  "frsqrte\\t%<v>0<Vmtype>, %<v>1<Vmtype>"
> +  [(set_attr "type" "neon_fp_rsqrte_<Vetype><q>")])

I think it might be useful to have the mode on the pattern name.

> +
> +(define_insn "*rsqrts_simd"

Likewise.

> +  [(set (match_operand:VALLF 0 "register_operand" "=w")
> +       (unspec:VALLF [(match_operand:VALLF 1 "register_operand" "w")
> +               (match_operand:VALLF 2 "register_operand" "w")]
> +                    UNSPEC_RSQRTS))]
> +  "TARGET_SIMD"
> +  "frsqrts\\t%<v>0<Vmtype>, %<v>1<Vmtype>, %<v>2<Vmtype>"
> +  [(set_attr "type" "neon_fp_rsqrts_<Vetype><q>")])
> +
> +(define_expand "rsqrt<mode>"
> +  [(set (match_operand:GPF 0 "register_operand" "=w")
> +       (unspec:GPF [(match_operand:GPF 1 "register_operand" "w")]
> +                    UNSPEC_RSQRT))]
> +  "TARGET_FLOAT"
> +{
> +  aarch64_emit_swrsqrt (operands[0], operands[1]);
> +  DONE;
> +})
> +
>  (define_insn "*aarch64_mul3_elt_to_64v2df"
>    [(set (match_operand:DF 0 "register_operand" "=w")
>       (mult:DF
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index c3c2795..281f0b2 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -6816,6 +6816,66 @@ aarch64_memory_move_cost (machine_mode mode ATTRIBUTE_UNUSED,
>    return aarch64_tune_params->memmov_cost;
>  }
>
> +extern tree aarch64_builtin_rsqrt (bool is_float);
> +
> +static tree
> +aarch64_builtin_reciprocal (unsigned int fn,
> +                            bool md_fn ATTRIBUTE_UNUSED,
> +                            bool sqrt ATTRIBUTE_UNUSED)

You don't need ATTRIBUTE_UNUSED any more if you are not using the
variable at all.  Just remove the variable name, this is C++ code
after all :).

> +{
> +  if (!fast_math_flags_set_p (&global_options))
> +    return NULL_TREE;

Is this correct check here?  I suspect you want
!(flag_finite_math_only && !flag_trapping_math &&
flag_unsafe_math_optimizations) instead so someone can do -ffast-math
-fno-finite-math-only.


is there a reason why you don't check md_fn here at all because if
md_fn is true, and you get the same value for fn as
BUILT_IN_SQRTF/BUILT_IN_SQRT, we would get incorrect code.
Also I think you should check md_fn and support the vectorized functions here.


> +
> +  if (fn == BUILT_IN_SQRTF)
> +    return aarch64_builtin_rsqrt (true);
> +  else if (fn == BUILT_IN_SQRT)
> +    return aarch64_builtin_rsqrt (false);
> +  else
> +    return NULL_TREE;
> +}
> +
> +void
> +aarch64_emit_swrsqrt (rtx dst, rtx src)
> +{
> +  enum machine_mode mode = GET_MODE (src);
> +  rtx xsrc = gen_reg_rtx (mode);
> +  emit_set_insn (xsrc, src);

Why do you use emit_set_insn here instead of emit_move_insn?

> +
> +  rtx x0 = gen_reg_rtx (mode);
> +  emit_insn (gen_rtx_SET (x0,
> +                          gen_rtx_UNSPEC (mode, gen_rtvec (1, xsrc),
> +                                          UNSPEC_RSQRTE)));

And not emit_set_insn here?

> +
> +  bool double_mode = (DFmode   == mode ||
> +                      V1DFmode == mode ||
> +                      V2DFmode == mode ||
> +                      V4DFmode == mode ||
> +                      V6DFmode == mode ||
> +                      V8DFmode == mode);

Why not use:
mode == DFmode || (VECTOR_MODE_P (mode) && GET_MODE_INNER (mode) == DFmode);
At least it becomes more obvious what is done here.
Or just:
mode == DFmode || mode == V2DFmode || mode == V4DFmode

Also it might make sense to have an assert on the modes we accept at
the beginning of the function.

> +
> +  int iterations = 2;
> +  if (double_mode)
> +    iterations = 3;
> +
> +  for (int i = 0; i < iterations; ++i)
> +    {
> +      rtx x1 = gen_reg_rtx (mode);
> +      rtx x2 = gen_reg_rtx (mode);
> +      rtx x3 = gen_reg_rtx (mode);
> +      emit_insn (gen_rtx_SET (x2,
> +                              gen_rtx_MULT (mode, x0, x0)));
> +      emit_insn (gen_rtx_SET (x3,
> +                              gen_rtx_UNSPEC (mode, gen_rtvec (2, xsrc, x2),
> +                                              UNSPEC_RSQRTS)));
> +      emit_insn (gen_rtx_SET (x1,
> +                              gen_rtx_MULT (mode, x0, x3)));

More of emit_set_insn usage.

> +      x0 = x1;
> +    }
> +
> +  emit_move_insn (dst, x0);
> +  return;
> +}
> +
>  /* Return the number of instructions that can be issued per cycle.  */
>  static int
>  aarch64_sched_issue_rate (void)
> @@ -11747,6 +11807,9 @@ aarch64_gen_adjusted_ldpstp (rtx *operands, bool load,
>  #undef TARGET_USE_BLOCKS_FOR_CONSTANT_P
>  #define TARGET_USE_BLOCKS_FOR_CONSTANT_P aarch64_use_blocks_for_constant_p
>
> +#undef TARGET_BUILTIN_RECIPROCAL
> +#define TARGET_BUILTIN_RECIPROCAL aarch64_builtin_reciprocal
> +
>  #undef TARGET_VECTOR_MODE_SUPPORTED_P
>  #define TARGET_VECTOR_MODE_SUPPORTED_P aarch64_vector_mode_supported_p
>
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 11123d6..7272d05 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -120,6 +120,9 @@
>      UNSPEC_VSTRUCTDUMMY
>      UNSPEC_SP_SET
>      UNSPEC_SP_TEST
> +    UNSPEC_RSQRT
> +    UNSPEC_RSQRTE
> +    UNSPEC_RSQRTS
>  ])
>
>  (define_c_enum "unspecv" [
> diff --git a/gcc/testsuite/gcc.target/aarch64/rsqrt.c b/gcc/testsuite/gcc.target/aarch64/rsqrt.c
> new file mode 100644
> index 0000000..6607483
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/rsqrt.c
> @@ -0,0 +1,113 @@
> +/* { dg-do run } */
> +/* { dg-options "-O3 -fno-inline --save-temps -ffast-math" } */

Maybe instead of -fno-inline, you can add some volatile to some
variables.  We really should have separate tests for the assembly scan
vs the runtime tests too.

> +
> +#include <math.h>
> +#include <stdio.h>
> +
> +#include <values.h>

Is values.h a standard header file.  I suspect you should be using
float.h instead which is the standard header file.
> +
> +#define PI    3.141592653589793
> +#define SQRT2 1.4142135623730951
> +
> +#define PI_4 0.7853981633974483
> +#define SQRT1_2 0.7071067811865475
> +
> +/* 2^25+1, float has 24 significand bits
> + *       according to Single-precision floating-point format.  */
> +#define TESTA8_FLT 33554433
> +/* 2^54+1, double has 53 significand bits
> + *       according to Double-precision floating-point format.  */
> +#define TESTA8_DBL 18014398509481985
> +
> +#define SD(a, b) t_double ((#a), (a), (b));
> +#define SF(a, b) t_float ((#a), (a), (b));
> +
> +#define EPSILON_double __DBL_EPSILON__
> +#define EPSILON_float __FLT_EPSILON__
> +#define ABS_double fabs
> +#define ABS_float fabsf
> +#define SQRT_double sqrt
> +#define SQRT_float sqrtf

I suspect you should have those using the builtin versions of the
above functions instead of the named ones.

Thanks,
Andrew Pinski

> +
> +extern void abort (void);
> +
> +#define TESTTYPE(TYPE)                                                     \
> +TYPE rsqrt_##TYPE (TYPE a)                                                 \
> +{                                                                          \
> +  return 1.0/SQRT_##TYPE(a);                                               \
> +}                                                                          \
> +                                                                           \
> +int equals_##TYPE (TYPE a, TYPE b)                                         \
> +{                                                                          \
> +  return (a == b ||                                                        \
> +          (isnan (a) && isnan (b)) ||                                      \
> +          (ABS_##TYPE (a - b) < EPSILON_##TYPE));                          \
> +}                                                                          \
> +                                                                           \
> +void t_##TYPE (const char *s, TYPE a, TYPE result)                         \
> +{                                                                          \
> +  TYPE r = rsqrt_##TYPE (a);                                               \
> +  if (!equals_##TYPE (r, result))                                          \
> +  {                                                                        \
> +    abort ();                                                              \
> +  }                                                                        \
> +}                                                                          \
> +
> +//  printf ("Problem in %20s: %30.18A should be %30.18A\n", s, r, result); \
> +
> +TESTTYPE(double)
> +TESTTYPE(float)
> +
> +/* { dg-final { scan-assembler-times "frsqrte\\td\[0-9\]+, d\[0-9\]+" 1 } } */
> +/* { dg-final { scan-assembler-times "frsqrts\\td\[0-9\]+, d\[0-9\]+, d\[0-9\]+" 3 } } */
> +
> +/* { dg-final { scan-assembler-times "frsqrte\\ts\[0-9\]+, s\[0-9\]+" 1 } } */
> +/* { dg-final { scan-assembler-times "frsqrts\\ts\[0-9\]+, s\[0-9\]+, s\[0-9\]+" 2 } } */
> +
> +int main ()
> +{
> +  SD(    1.0/256, 0X1.00000000000000P+4  );
> +  SD(        1.0, 0X1.00000000000000P+0  );
> +  SD(       -1.0,                     NAN);
> +  SD(       11.0, 0X1.34BF63D1568260P-2  );
> +  SD(        0.0,                INFINITY);
> +  SD(   INFINITY, 0X0.00000000000000P+0  );
> +  SD(        NAN,                     NAN);
> +  SD(       -NAN,                    -NAN);
> +  SD(    DBL_MAX, 0X1.00000000000010P-512);
> +  SD(    DBL_MIN, 0X1.00000000000000P+511);
> +  SD(         PI, 0X1.20DD750429B6D0P-1  );
> +  SD(       PI_4, 0X1.20DD750429B6D0P+0  );
> +  SD(      SQRT2, 0X1.AE89F995AD3AE0P-1  );
> +  SD(    SQRT1_2, 0X1.306FE0A31B7150P+0  );
> +  SD(        -PI,                     NAN);
> +  SD(     -SQRT2,                     NAN);
> +  SD( TESTA8_DBL, 0X1.00000000000000P-27 );
> +
> +  SF(    1.0/256, 0X1.00000000000000P+4  );
> +  SF(        1.0, 0X1.00000000000000P+0  );
> +  SF(       -1.0,                     NAN);
> +  SF(       11.0, 0X1.34BF6400000000P-2  );
> +  SF(        0.0,                INFINITY);
> +  SF(   INFINITY, 0X0.00000000000000P+0  );
> +  SF(        NAN,                     NAN);
> +  SF(       -NAN,                    -NAN);
> +  SF(    FLT_MAX, 0X1.00000200000000P-64 );
> +  SF(    FLT_MIN, 0X1.00000000000000P+63 );
> +  SF(         PI, 0X1.20DD7400000000P-1  );
> +  SF(       PI_4, 0X1.20DD7400000000P+0  );
> +  SF(      SQRT2, 0X1.AE89FA00000000P-1  );
> +  SF(    SQRT1_2, 0X1.306FE000000000P+0  );
> +  SF(        -PI,                     NAN);
> +  SF(     -SQRT2,                     NAN);
> +  SF( TESTA8_FLT, 0X1.6A09E600000000P-13 );
> +
> +//   With -ffast-math these return positive INF.
> +//   SD(       -0.0,               -INFINITY);
> +//   SF(       -0.0,               -INFINITY);
> +//   The reason here is that -ffast-math flushes to zero.
> +//   SD(DBL_MIN/256, 0X1.00000000000000P+515);
> +//   SF(FLT_MIN/256, 0X1.00000000000000P+67 );
> +
> +  return 0;
> +}
> --
> 1.9.1
>

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[pinskia]

> On Jun 25, 2015, at 9:44 AM, Kumar, Venkataramanan <Venkataramanan.Kumar@amd.com> wrote:
> 
> I got around ~12% gain with -Ofast -mcpu=cortex-a57.

I get around 11/12% on thunderX with the patch and the decreasing the iterations change (1/2) compared to without the patch. 

Thanks,
Andrew


> 
> Regards,
> Venkat.
> 
>> -----Original Message-----
>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-    
>> owner@gcc.gnu.org] On Behalf Of Dr. Philipp Tomsich
>> Sent: Thursday, June 25, 2015 9:13 PM
>> To: Kumar, Venkataramanan
>> Cc: Benedikt Huber; pinskia@gmail.com; gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
>> estimation in -ffast-math
>> 
>> Kumar,
>> 
>> what is the relative gain that you see on Cortex-A57?
>> 
>> Thanks,
>> Philipp.
>> 
>>>> On 25 Jun 2015, at 17:35, Kumar, Venkataramanan
>>> <Venkataramanan.Kumar@amd.com> wrote:
>>> 
>>> Changing to  "1 step for float" and "2 steps for double" gives better gains
>> now for gromacs on cortex-a57.
>>> 
>>> Regards,
>>> Venkat.
>>>> -----Original Message-----
>>>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
>>>> owner@gcc.gnu.org] On Behalf Of Benedikt Huber
>>>> Sent: Thursday, June 25, 2015 4:09 PM
>>>> To: pinskia@gmail.com
>>>> Cc: gcc-patches@gcc.gnu.org; philipp.tomsich@theobroma-systems.com
>>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
>>>> (rsqrt) estimation in -ffast-math
>>>> 
>>>> Andrew,
>>>> 
>>>>> This is NOT a win on thunderX at least for single precision because
>>>>> you have
>>>> to do the divide and sqrt in the same time as it takes 5 multiples
>>>> (estimate and step are multiplies in the thunderX pipeline).  Doubles
>>>> is 10 multiplies which is just the same as what the patch does (but
>>>> it is really slightly less than 10, I rounded up). So in the end this
>>>> is NOT a win at all for thunderX unless we do one less step for both single
>> and double.
>>>> 
>>>> Yes, the expected benefit from rsqrt estimation is implementation
>>>> specific. If one has a better initial rsqrte or an application that
>>>> can trade precision for execution time, we could offer a command line
>>>> option to do only 2 steps for doulbe and 1 step for float; similar to -
>> mrecip-precision for PowerPC.
>>>> What are your thoughts on that?
>>>> 
>>>> Best regards,
>>>> Benedikt
> 

RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Kumar, Venkataramanan]

Hmm,  Reducing the iterations to "1 step for float" and "2 steps for double"

 I got VE (miscompares) on following benchmarks
416.gamess 
453.povray         
454.calculix   
459.GemsFDTD  

Benedikt , I have ICE for 444.namd with your patch,  not sure if something wrong in my local tree.  

Regards,
Venkat.

> -----Original Message-----
> From: pinskia@gmail.com [mailto:pinskia@gmail.com]
> Sent: Sunday, June 28, 2015 8:35 PM
> To: Kumar, Venkataramanan
> Cc: Dr. Philipp Tomsich; Benedikt Huber; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> estimation in -ffast-math
> 
> 
> 
> 
> 
> > On Jun 25, 2015, at 9:44 AM, Kumar, Venkataramanan
> <Venkataramanan.Kumar@amd.com> wrote:
> >
> > I got around ~12% gain with -Ofast -mcpu=cortex-a57.
> 
> I get around 11/12% on thunderX with the patch and the decreasing the
> iterations change (1/2) compared to without the patch.
> 
> Thanks,
> Andrew
> 
> 
> >
> > Regards,
> > Venkat.
> >
> >> -----Original Message-----
> >> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> >> owner@gcc.gnu.org] On Behalf Of Dr. Philipp Tomsich
> >> Sent: Thursday, June 25, 2015 9:13 PM
> >> To: Kumar, Venkataramanan
> >> Cc: Benedikt Huber; pinskia@gmail.com; gcc-patches@gcc.gnu.org
> >> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> >> (rsqrt) estimation in -ffast-math
> >>
> >> Kumar,
> >>
> >> what is the relative gain that you see on Cortex-A57?
> >>
> >> Thanks,
> >> Philipp.
> >>
> >>>> On 25 Jun 2015, at 17:35, Kumar, Venkataramanan
> >>> <Venkataramanan.Kumar@amd.com> wrote:
> >>>
> >>> Changing to  "1 step for float" and "2 steps for double" gives
> >>> better gains
> >> now for gromacs on cortex-a57.
> >>>
> >>> Regards,
> >>> Venkat.
> >>>> -----Original Message-----
> >>>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> >>>> owner@gcc.gnu.org] On Behalf Of Benedikt Huber
> >>>> Sent: Thursday, June 25, 2015 4:09 PM
> >>>> To: pinskia@gmail.com
> >>>> Cc: gcc-patches@gcc.gnu.org; philipp.tomsich@theobroma-
> systems.com
> >>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> >>>> (rsqrt) estimation in -ffast-math
> >>>>
> >>>> Andrew,
> >>>>
> >>>>> This is NOT a win on thunderX at least for single precision
> >>>>> because you have
> >>>> to do the divide and sqrt in the same time as it takes 5 multiples
> >>>> (estimate and step are multiplies in the thunderX pipeline).
> >>>> Doubles is 10 multiplies which is just the same as what the patch
> >>>> does (but it is really slightly less than 10, I rounded up). So in
> >>>> the end this is NOT a win at all for thunderX unless we do one less
> >>>> step for both single
> >> and double.
> >>>>
> >>>> Yes, the expected benefit from rsqrt estimation is implementation
> >>>> specific. If one has a better initial rsqrte or an application that
> >>>> can trade precision for execution time, we could offer a command
> >>>> line option to do only 2 steps for doulbe and 1 step for float;
> >>>> similar to -
> >> mrecip-precision for PowerPC.
> >>>> What are your thoughts on that?
> >>>>
> >>>> Best regards,
> >>>> Benedikt
> >

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Dr. Philipp Tomsich]

Kumar,

This does not come unexpected, as the initial estimation and each iteration will add an architecturally-defined number of bits of precision (ARMv8 guarantuees only a minimum number of bits provided per operation… the exact number is specific to each micro-arch, though).
Depending on your architecture and on the required number of precise bits by any given benchmark, one may see miscompares.

Do you know the exact number of bits that the initial estimate and the subsequent refinement steps add for your micro-arch?

Thanks,
Philipp.

> On 29 Jun 2015, at 10:17, Kumar, Venkataramanan <Venkataramanan.Kumar@amd.com> wrote:
> 
> 
> Hmm,  Reducing the iterations to "1 step for float" and "2 steps for double"
> 
> I got VE (miscompares) on following benchmarks
> 416.gamess 
> 453.povray         
> 454.calculix   
> 459.GemsFDTD  
> 
> Benedikt , I have ICE for 444.namd with your patch,  not sure if something wrong in my local tree.  
> 
> Regards,
> Venkat.
> 
>> -----Original Message-----
>> From: pinskia@gmail.com [mailto:pinskia@gmail.com]
>> Sent: Sunday, June 28, 2015 8:35 PM
>> To: Kumar, Venkataramanan
>> Cc: Dr. Philipp Tomsich; Benedikt Huber; gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
>> estimation in -ffast-math
>> 
>> 
>> 
>> 
>> 
>>> On Jun 25, 2015, at 9:44 AM, Kumar, Venkataramanan
>> <Venkataramanan.Kumar@amd.com> wrote:
>>> 
>>> I got around ~12% gain with -Ofast -mcpu=cortex-a57.
>> 
>> I get around 11/12% on thunderX with the patch and the decreasing the
>> iterations change (1/2) compared to without the patch.
>> 
>> Thanks,
>> Andrew
>> 
>> 
>>> 
>>> Regards,
>>> Venkat.
>>> 
>>>> -----Original Message-----
>>>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
>>>> owner@gcc.gnu.org] On Behalf Of Dr. Philipp Tomsich
>>>> Sent: Thursday, June 25, 2015 9:13 PM
>>>> To: Kumar, Venkataramanan
>>>> Cc: Benedikt Huber; pinskia@gmail.com; gcc-patches@gcc.gnu.org
>>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
>>>> (rsqrt) estimation in -ffast-math
>>>> 
>>>> Kumar,
>>>> 
>>>> what is the relative gain that you see on Cortex-A57?
>>>> 
>>>> Thanks,
>>>> Philipp.
>>>> 
>>>>>> On 25 Jun 2015, at 17:35, Kumar, Venkataramanan
>>>>> <Venkataramanan.Kumar@amd.com> wrote:
>>>>> 
>>>>> Changing to  "1 step for float" and "2 steps for double" gives
>>>>> better gains
>>>> now for gromacs on cortex-a57.
>>>>> 
>>>>> Regards,
>>>>> Venkat.
>>>>>> -----Original Message-----
>>>>>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
>>>>>> owner@gcc.gnu.org] On Behalf Of Benedikt Huber
>>>>>> Sent: Thursday, June 25, 2015 4:09 PM
>>>>>> To: pinskia@gmail.com
>>>>>> Cc: gcc-patches@gcc.gnu.org; philipp.tomsich@theobroma-
>> systems.com
>>>>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
>>>>>> (rsqrt) estimation in -ffast-math
>>>>>> 
>>>>>> Andrew,
>>>>>> 
>>>>>>> This is NOT a win on thunderX at least for single precision
>>>>>>> because you have
>>>>>> to do the divide and sqrt in the same time as it takes 5 multiples
>>>>>> (estimate and step are multiplies in the thunderX pipeline).
>>>>>> Doubles is 10 multiplies which is just the same as what the patch
>>>>>> does (but it is really slightly less than 10, I rounded up). So in
>>>>>> the end this is NOT a win at all for thunderX unless we do one less
>>>>>> step for both single
>>>> and double.
>>>>>> 
>>>>>> Yes, the expected benefit from rsqrt estimation is implementation
>>>>>> specific. If one has a better initial rsqrte or an application that
>>>>>> can trade precision for execution time, we could offer a command
>>>>>> line option to do only 2 steps for doulbe and 1 step for float;
>>>>>> similar to -
>>>> mrecip-precision for PowerPC.
>>>>>> What are your thoughts on that?
>>>>>> 
>>>>>> Best regards,
>>>>>> Benedikt
>>> 

RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Kumar, Venkataramanan]

> -----Original Message-----
> From: Dr. Philipp Tomsich [mailto:philipp.tomsich@theobroma-systems.com]
> Sent: Monday, June 29, 2015 2:17 PM
> To: Kumar, Venkataramanan
> Cc: pinskia@gmail.com; Benedikt Huber; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> estimation in -ffast-math
> 
> Kumar,
> 
> This does not come unexpected, as the initial estimation and each iteration
> will add an architecturally-defined number of bits of precision (ARMv8
> guarantuees only a minimum number of bits provided per operation… the
> exact number is specific to each micro-arch, though).
> Depending on your architecture and on the required number of precise bits
> by any given benchmark, one may see miscompares.

True.  
 

> 
> Do you know the exact number of bits that the initial estimate and the
> subsequent refinement steps add for your micro-arch?

I am not sure on this. I  need to check for cortex-a57 case.  

But best thing for "cortex-a57" case is not to use this optimization by default. 
If we get an -mrecip-sqrt command line , then we can add it for "gromacs" kind application to get gains.

Any thoughts on this?
    
Regards,
Venkat.
 

> 
> Thanks,
> Philipp.
> 
> > On 29 Jun 2015, at 10:17, Kumar, Venkataramanan
> <Venkataramanan.Kumar@amd.com> wrote:
> >
> >
> > Hmm,  Reducing the iterations to "1 step for float" and "2 steps for double"
> >
> > I got VE (miscompares) on following benchmarks 416.gamess
> > 453.povray
> > 454.calculix
> > 459.GemsFDTD
> >
> > Benedikt , I have ICE for 444.namd with your patch,  not sure if something
> wrong in my local tree.
> >
> > Regards,
> > Venkat.
> >
> >> -----Original Message-----
> >> From: pinskia@gmail.com [mailto:pinskia@gmail.com]
> >> Sent: Sunday, June 28, 2015 8:35 PM
> >> To: Kumar, Venkataramanan
> >> Cc: Dr. Philipp Tomsich; Benedikt Huber; gcc-patches@gcc.gnu.org
> >> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> >> (rsqrt) estimation in -ffast-math
> >>
> >>
> >>
> >>
> >>
> >>> On Jun 25, 2015, at 9:44 AM, Kumar, Venkataramanan
> >> <Venkataramanan.Kumar@amd.com> wrote:
> >>>
> >>> I got around ~12% gain with -Ofast -mcpu=cortex-a57.
> >>
> >> I get around 11/12% on thunderX with the patch and the decreasing the
> >> iterations change (1/2) compared to without the patch.
> >>
> >> Thanks,
> >> Andrew
> >>
> >>
> >>>
> >>> Regards,
> >>> Venkat.
> >>>
> >>>> -----Original Message-----
> >>>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> >>>> owner@gcc.gnu.org] On Behalf Of Dr. Philipp Tomsich
> >>>> Sent: Thursday, June 25, 2015 9:13 PM
> >>>> To: Kumar, Venkataramanan
> >>>> Cc: Benedikt Huber; pinskia@gmail.com; gcc-patches@gcc.gnu.org
> >>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> >>>> (rsqrt) estimation in -ffast-math
> >>>>
> >>>> Kumar,
> >>>>
> >>>> what is the relative gain that you see on Cortex-A57?
> >>>>
> >>>> Thanks,
> >>>> Philipp.
> >>>>
> >>>>>> On 25 Jun 2015, at 17:35, Kumar, Venkataramanan
> >>>>> <Venkataramanan.Kumar@amd.com> wrote:
> >>>>>
> >>>>> Changing to  "1 step for float" and "2 steps for double" gives
> >>>>> better gains
> >>>> now for gromacs on cortex-a57.
> >>>>>
> >>>>> Regards,
> >>>>> Venkat.
> >>>>>> -----Original Message-----
> >>>>>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> >>>>>> owner@gcc.gnu.org] On Behalf Of Benedikt Huber
> >>>>>> Sent: Thursday, June 25, 2015 4:09 PM
> >>>>>> To: pinskia@gmail.com
> >>>>>> Cc: gcc-patches@gcc.gnu.org; philipp.tomsich@theobroma-
> >> systems.com
> >>>>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> >>>>>> (rsqrt) estimation in -ffast-math
> >>>>>>
> >>>>>> Andrew,
> >>>>>>
> >>>>>>> This is NOT a win on thunderX at least for single precision
> >>>>>>> because you have
> >>>>>> to do the divide and sqrt in the same time as it takes 5
> >>>>>> multiples (estimate and step are multiplies in the thunderX pipeline).
> >>>>>> Doubles is 10 multiplies which is just the same as what the patch
> >>>>>> does (but it is really slightly less than 10, I rounded up). So
> >>>>>> in the end this is NOT a win at all for thunderX unless we do one
> >>>>>> less step for both single
> >>>> and double.
> >>>>>>
> >>>>>> Yes, the expected benefit from rsqrt estimation is implementation
> >>>>>> specific. If one has a better initial rsqrte or an application
> >>>>>> that can trade precision for execution time, we could offer a
> >>>>>> command line option to do only 2 steps for doulbe and 1 step for
> >>>>>> float; similar to -
> >>>> mrecip-precision for PowerPC.
> >>>>>> What are your thoughts on that?
> >>>>>>
> >>>>>> Best regards,
> >>>>>> Benedikt
> >>>

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[James Greenhalgh]

On Mon, Jun 29, 2015 at 10:18:23AM +0100, Kumar, Venkataramanan wrote:
> 
> > -----Original Message-----
> > From: Dr. Philipp Tomsich [mailto:philipp.tomsich@theobroma-systems.com]
> > Sent: Monday, June 29, 2015 2:17 PM
> > To: Kumar, Venkataramanan
> > Cc: pinskia@gmail.com; Benedikt Huber; gcc-patches@gcc.gnu.org
> > Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> > estimation in -ffast-math
> > 
> > Kumar,
> > 
> > This does not come unexpected, as the initial estimation and each iteration
> > will add an architecturally-defined number of bits of precision (ARMv8
> > guarantuees only a minimum number of bits provided per operation… the
> > exact number is specific to each micro-arch, though).
> > Depending on your architecture and on the required number of precise bits
> > by any given benchmark, one may see miscompares.
> 
> True.  

I would be very uncomfortable with this approach.

From Richard Biener's post in the thread Michael Matz linked earlier
in the thread:

    It would follow existing practice of things we allow in
    -funsafe-math-optimizations.  Existing practice in that we
    want to allow -ffast-math use with common benchmarks we care
    about.

    https://gcc.gnu.org/ml/gcc-patches/2009-11/msg00100.html

With the solution you seem to be converging on (2-steps for some
microarchitectures, 3 for others), a binary generated for one micro-arch
may drop below a minimum guarantee of precision when run on another. This
seems to go against the spirit of the practice above. I would only support
adding this optimization to -Ofast if we could keep to architectural
guarantees of precision in the generated code (i.e. 3-steps everywhere).

I don't object to adding a "-mlow-precision-recip-sqrt" style option,
which would be off by default, would enable the 2-step mode, and would
need to be explicitly enabled (i.e. not implied by -mcpu=foo) but I don't
see what this buys you beyond the Gromacs boost (and even there you would
be creating an Invalid Run as optimization flags must be applied across
all workloads). 

For the 3-step optimization, it is clear to me that for "generic" tuning
we don't want this to be enabled by default experimental results and advice
in this thread argues against it for thunderx and cortex-a57 targets.
However, enabling it based on the CPU tuning selected seems fine to me.

Thanks,
James

> > 
> > Do you know the exact number of bits that the initial estimate and the
> > subsequent refinement steps add for your micro-arch?
> 
> I am not sure on this. I  need to check for cortex-a57 case.  
> 
> But best thing for "cortex-a57" case is not to use this optimization by default. 
> If we get an -mrecip-sqrt command line , then we can add it for "gromacs" kind application to get gains.
> 
> Any thoughts on this?
>     
> Regards,
> Venkat.
>  
> 
> > 
> > Thanks,
> > Philipp.
> > 
> > > On 29 Jun 2015, at 10:17, Kumar, Venkataramanan
> > <Venkataramanan.Kumar@amd.com> wrote:
> > >
> > >
> > > Hmm,  Reducing the iterations to "1 step for float" and "2 steps for double"
> > >
> > > I got VE (miscompares) on following benchmarks 416.gamess
> > > 453.povray
> > > 454.calculix
> > > 459.GemsFDTD
> > >
> > > Benedikt , I have ICE for 444.namd with your patch,  not sure if something
> > wrong in my local tree.
> > >
> > > Regards,
> > > Venkat.
> > >
> > >> -----Original Message-----
> > >> From: pinskia@gmail.com [mailto:pinskia@gmail.com]
> > >> Sent: Sunday, June 28, 2015 8:35 PM
> > >> To: Kumar, Venkataramanan
> > >> Cc: Dr. Philipp Tomsich; Benedikt Huber; gcc-patches@gcc.gnu.org
> > >> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> > >> (rsqrt) estimation in -ffast-math
> > >>
> > >>
> > >>
> > >>
> > >>
> > >>> On Jun 25, 2015, at 9:44 AM, Kumar, Venkataramanan
> > >> <Venkataramanan.Kumar@amd.com> wrote:
> > >>>
> > >>> I got around ~12% gain with -Ofast -mcpu=cortex-a57.
> > >>
> > >> I get around 11/12% on thunderX with the patch and the decreasing the
> > >> iterations change (1/2) compared to without the patch.
> > >>
> > >> Thanks,
> > >> Andrew
> > >>
> > >>
> > >>>
> > >>> Regards,
> > >>> Venkat.
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> > >>>> owner@gcc.gnu.org] On Behalf Of Dr. Philipp Tomsich
> > >>>> Sent: Thursday, June 25, 2015 9:13 PM
> > >>>> To: Kumar, Venkataramanan
> > >>>> Cc: Benedikt Huber; pinskia@gmail.com; gcc-patches@gcc.gnu.org
> > >>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> > >>>> (rsqrt) estimation in -ffast-math
> > >>>>
> > >>>> Kumar,
> > >>>>
> > >>>> what is the relative gain that you see on Cortex-A57?
> > >>>>
> > >>>> Thanks,
> > >>>> Philipp.
> > >>>>
> > >>>>>> On 25 Jun 2015, at 17:35, Kumar, Venkataramanan
> > >>>>> <Venkataramanan.Kumar@amd.com> wrote:
> > >>>>>
> > >>>>> Changing to  "1 step for float" and "2 steps for double" gives
> > >>>>> better gains
> > >>>> now for gromacs on cortex-a57.
> > >>>>>
> > >>>>> Regards,
> > >>>>> Venkat.
> > >>>>>> -----Original Message-----
> > >>>>>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> > >>>>>> owner@gcc.gnu.org] On Behalf Of Benedikt Huber
> > >>>>>> Sent: Thursday, June 25, 2015 4:09 PM
> > >>>>>> To: pinskia@gmail.com
> > >>>>>> Cc: gcc-patches@gcc.gnu.org; philipp.tomsich@theobroma-
> > >> systems.com
> > >>>>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> > >>>>>> (rsqrt) estimation in -ffast-math
> > >>>>>>
> > >>>>>> Andrew,
> > >>>>>>
> > >>>>>>> This is NOT a win on thunderX at least for single precision
> > >>>>>>> because you have
> > >>>>>> to do the divide and sqrt in the same time as it takes 5
> > >>>>>> multiples (estimate and step are multiplies in the thunderX pipeline).
> > >>>>>> Doubles is 10 multiplies which is just the same as what the patch
> > >>>>>> does (but it is really slightly less than 10, I rounded up). So
> > >>>>>> in the end this is NOT a win at all for thunderX unless we do one
> > >>>>>> less step for both single
> > >>>> and double.
> > >>>>>>
> > >>>>>> Yes, the expected benefit from rsqrt estimation is implementation
> > >>>>>> specific. If one has a better initial rsqrte or an application
> > >>>>>> that can trade precision for execution time, we could offer a
> > >>>>>> command line option to do only 2 steps for doulbe and 1 step for
> > >>>>>> float; similar to -
> > >>>> mrecip-precision for PowerPC.
> > >>>>>> What are your thoughts on that?
> > >>>>>>
> > >>>>>> Best regards,
> > >>>>>> Benedikt
> > >>>
> 

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Dr. Philipp Tomsich]

James,

On 29 Jun 2015, at 13:36, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> 
> On Mon, Jun 29, 2015 at 10:18:23AM +0100, Kumar, Venkataramanan wrote:
>> 
>>> -----Original Message-----
>>> From: Dr. Philipp Tomsich [mailto:philipp.tomsich@theobroma-systems.com]
>>> Sent: Monday, June 29, 2015 2:17 PM
>>> To: Kumar, Venkataramanan
>>> Cc: pinskia@gmail.com; Benedikt Huber; gcc-patches@gcc.gnu.org
>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
>>> estimation in -ffast-math
>>> 
>>> Kumar,
>>> 
>>> This does not come unexpected, as the initial estimation and each iteration
>>> will add an architecturally-defined number of bits of precision (ARMv8
>>> guarantuees only a minimum number of bits provided per operation… the
>>> exact number is specific to each micro-arch, though).
>>> Depending on your architecture and on the required number of precise bits
>>> by any given benchmark, one may see miscompares.
>> 
>> True.  
> 
> I would be very uncomfortable with this approach.

Same here. The default must be safe. Always.
Unlike other architectures, we don’t have a problem with making the proper
defaults for “safety”, as the ARMv8 ISA guarantees a minimum number of
precise bits per iteration.

> From Richard Biener's post in the thread Michael Matz linked earlier
> in the thread:
> 
>    It would follow existing practice of things we allow in
>    -funsafe-math-optimizations.  Existing practice in that we
>    want to allow -ffast-math use with common benchmarks we care
>    about.
> 
>    https://gcc.gnu.org/ml/gcc-patches/2009-11/msg00100.html
> 
> With the solution you seem to be converging on (2-steps for some
> microarchitectures, 3 for others), a binary generated for one micro-arch
> may drop below a minimum guarantee of precision when run on another. This
> seems to go against the spirit of the practice above. I would only support
> adding this optimization to -Ofast if we could keep to architectural
> guarantees of precision in the generated code (i.e. 3-steps everywhere).
> 
> I don't object to adding a "-mlow-precision-recip-sqrt" style option,
> which would be off by default, would enable the 2-step mode, and would
> need to be explicitly enabled (i.e. not implied by -mcpu=foo) but I don't
> see what this buys you beyond the Gromacs boost (and even there you would
> be creating an Invalid Run as optimization flags must be applied across
> all workloads). 

Any flag that reduces precision (and thus breaks IEEE floating-point semantics)
needs to be gated with an “unsafe” flag (i.e. one that is never on by default).
As a consequence, the “peak”-tuning for SPEC will turn this on… but barely 
anyone else would.

> For the 3-step optimization, it is clear to me that for "generic" tuning
> we don't want this to be enabled by default experimental results and advice
> in this thread argues against it for thunderx and cortex-a57 targets.
> However, enabling it based on the CPU tuning selected seems fine to me.

I do not agree on this one, as I would like to see the safe form (i.e. 3 and 5
iterations respectively) to become the default. Most “server-type” chips
should not see a performance regression, while it will be easier to optimise for
this in hardware than for a (potentially microcoded) sqrt-instruction (and 
subsequent, dependent divide).

I have not heard anyone claim a performance regression (either on thunderx
or on cortex-a57), but merely heard a “no speed-up”.

So I am strongly in favor of defaulting to the ‘safe’ number of iterations, even
when compiling for a generic target.

Best,
Philipp.

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Benedikt Huber]

[-- Attachment #1: Type: text/plain, Size: 513 bytes --]

> Hmm,  Reducing the iterations to "1 step for float" and "2 steps for double"
> 
> I got VE (miscompares) on following benchmarks
> 416.gamess
> 453.povray
> 454.calculix
> 459.GemsFDTD
> 
> Benedikt , I have ICE for 444.namd with your patch,  not sure if something wrong in my local tree.

I could not reproduce the ICE for namd.
Could you provide the spec config entry or the command line options, please.
Which ICE did you get? Please provide the output.

Thank you and best regards,
Benedikt

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 496 bytes --]

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[pinskia]

> On Jun 29, 2015, at 4:44 AM, Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote:
> 
> James,
> 
>> On 29 Jun 2015, at 13:36, James Greenhalgh <james.greenhalgh@arm.com> wrote:
>> 
>>> On Mon, Jun 29, 2015 at 10:18:23AM +0100, Kumar, Venkataramanan wrote:
>>> 
>>>> -----Original Message-----
>>>> From: Dr. Philipp Tomsich [mailto:philipp.tomsich@theobroma-systems.com]
>>>> Sent: Monday, June 29, 2015 2:17 PM
>>>> To: Kumar, Venkataramanan
>>>> Cc: pinskia@gmail.com; Benedikt Huber; gcc-patches@gcc.gnu.org
>>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
>>>> estimation in -ffast-math
>>>> 
>>>> Kumar,
>>>> 
>>>> This does not come unexpected, as the initial estimation and each iteration
>>>> will add an architecturally-defined number of bits of precision (ARMv8
>>>> guarantuees only a minimum number of bits provided per operation… the
>>>> exact number is specific to each micro-arch, though).
>>>> Depending on your architecture and on the required number of precise bits
>>>> by any given benchmark, one may see miscompares.
>>> 
>>> True.  
>> 
>> I would be very uncomfortable with this approach.
> 
> Same here. The default must be safe. Always.
> Unlike other architectures, we don’t have a problem with making the proper
> defaults for “safety”, as the ARMv8 ISA guarantees a minimum number of
> precise bits per iteration.
> 
>> From Richard Biener's post in the thread Michael Matz linked earlier
>> in the thread:
>> 
>>   It would follow existing practice of things we allow in
>>   -funsafe-math-optimizations.  Existing practice in that we
>>   want to allow -ffast-math use with common benchmarks we care
>>   about.
>> 
>>   https://gcc.gnu.org/ml/gcc-patches/2009-11/msg00100.html
>> 
>> With the solution you seem to be converging on (2-steps for some
>> microarchitectures, 3 for others), a binary generated for one micro-arch
>> may drop below a minimum guarantee of precision when run on another. This
>> seems to go against the spirit of the practice above. I would only support
>> adding this optimization to -Ofast if we could keep to architectural
>> guarantees of precision in the generated code (i.e. 3-steps everywhere).
>> 
>> I don't object to adding a "-mlow-precision-recip-sqrt" style option,
>> which would be off by default, would enable the 2-step mode, and would
>> need to be explicitly enabled (i.e. not implied by -mcpu=foo) but I don't
>> see what this buys you beyond the Gromacs boost (and even there you would
>> be creating an Invalid Run as optimization flags must be applied across
>> all workloads).
> 
> Any flag that reduces precision (and thus breaks IEEE floating-point semantics)
> needs to be gated with an “unsafe” flag (i.e. one that is never on by default).
> As a consequence, the “peak”-tuning for SPEC will turn this on… but barely 
> anyone else would.
> 
>> For the 3-step optimization, it is clear to me that for "generic" tuning
>> we don't want this to be enabled by default experimental results and advice
>> in this thread argues against it for thunderx and cortex-a57 targets.
>> However, enabling it based on the CPU tuning selected seems fine to me.
> 
> I do not agree on this one, as I would like to see the safe form (i.e. 3 and 5
> iterations respectively) to become the default. Most “server-type” chips
> should not see a performance regression, while it will be easier to optimise for
> this in hardware than for a (potentially microcoded) sqrt-instruction (and 
> subsequent, dependent divide).
> 
> I have not heard anyone claim a performance regression (either on thunderx
> or on cortex-a57), but merely heard a “no speed-up”.

Actually it does regress performance on thunderX, I just assumed that when I said not going to be a win it was taken as a slow down. It regress gromacs by more than 10% on thunderX but I can't remember how much as i had someone else run it. The latency difference is also over 40%; for example single precision: 29 cycles with div (12) sqrt(17) directly vs 42 cycles with the rsqrte and 2 iterations of 2mul/rsqrts (double is 53 vs 60). That is huge difference right there.  ThunderX has a fast div and a fast sqrt for 32bit and a reasonable one for double.   So again this is not just not a win but rather a regression for thunderX. I suspect cortex-a57 is also true. 

Thanks,
Andrew

> 
> So I am strongly in favor of defaulting to the ‘safe’ number of iterations, even
> when compiling for a generic target.
> 
> Best,
> Philipp.
> 

Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Benedikt Huber]

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]

> 
> Benedikt , I have ICE for 444.namd with your patch,  not sure if something wrong in my local tree.

Venkat, now I could reproduce it.
Strangely it does not happen with -flto.
I will try to find out the reason for that.

Thank you for the catch,
Benedikt


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 496 bytes --]

RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Kumar, Venkataramanan]

Hi,

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Benedikt Huber
> Sent: Monday, June 29, 2015 11:04 PM
> To: Kumar, Venkataramanan
> Cc: pinskia@gmail.com; Dr. Philipp Tomsich; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> estimation in -ffast-math
> 
> >
> > Benedikt , I have ICE for 444.namd with your patch,  not sure if something
> wrong in my local tree.
> 
> Venkat, now I could reproduce it.
> Strangely it does not happen with -flto.
> I will try to find out the reason for that.
> 
Sure,  I kind of remember that RTX for target in builtin rsqrt was 0.  But not sure if we need to create reg rtx in such cases.
 

> Thank you for the catch,
> Benedikt

RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Kumar, Venkataramanan]

Hi,

> -----Original Message-----
> From: pinskia@gmail.com [mailto:pinskia@gmail.com]
> Sent: Monday, June 29, 2015 10:23 PM
> To: Dr. Philipp Tomsich
> Cc: James Greenhalgh; Kumar, Venkataramanan; Benedikt Huber; gcc-
> patches@gcc.gnu.org; Marcus Shawcroft; Ramana Radhakrishnan; Richard
> Earnshaw
> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> estimation in -ffast-math
> 
> 
> 
> 
> 
> > On Jun 29, 2015, at 4:44 AM, Dr. Philipp Tomsich
> <philipp.tomsich@theobroma-systems.com> wrote:
> >
> > James,
> >
> >> On 29 Jun 2015, at 13:36, James Greenhalgh
> <james.greenhalgh@arm.com> wrote:
> >>
> >>> On Mon, Jun 29, 2015 at 10:18:23AM +0100, Kumar, Venkataramanan
> wrote:
> >>>
> >>>> -----Original Message-----
> >>>> From: Dr. Philipp Tomsich
> >>>> [mailto:philipp.tomsich@theobroma-systems.com]
> >>>> Sent: Monday, June 29, 2015 2:17 PM
> >>>> To: Kumar, Venkataramanan
> >>>> Cc: pinskia@gmail.com; Benedikt Huber; gcc-patches@gcc.gnu.org
> >>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> >>>> (rsqrt) estimation in -ffast-math
> >>>>
> >>>> Kumar,
> >>>>
> >>>> This does not come unexpected, as the initial estimation and each
> >>>> iteration will add an architecturally-defined number of bits of
> >>>> precision (ARMv8 guarantuees only a minimum number of bits
> provided
> >>>> per operation… the exact number is specific to each micro-arch,
> though).
> >>>> Depending on your architecture and on the required number of
> >>>> precise bits by any given benchmark, one may see miscompares.
> >>>
> >>> True.
> >>
> >> I would be very uncomfortable with this approach.
> >
> > Same here. The default must be safe. Always.
> > Unlike other architectures, we don’t have a problem with making the
> > proper defaults for “safety”, as the ARMv8 ISA guarantees a minimum
> > number of precise bits per iteration.
> >
> >> From Richard Biener's post in the thread Michael Matz linked earlier
> >> in the thread:
> >>
> >>   It would follow existing practice of things we allow in
> >>   -funsafe-math-optimizations.  Existing practice in that we
> >>   want to allow -ffast-math use with common benchmarks we care
> >>   about.
> >>
> >>   https://gcc.gnu.org/ml/gcc-patches/2009-11/msg00100.html
> >>
> >> With the solution you seem to be converging on (2-steps for some
> >> microarchitectures, 3 for others), a binary generated for one
> >> micro-arch may drop below a minimum guarantee of precision when run
> >> on another. This seems to go against the spirit of the practice
> >> above. I would only support adding this optimization to -Ofast if we
> >> could keep to architectural guarantees of precision in the generated code
> (i.e. 3-steps everywhere).
> >>
> >> I don't object to adding a "-mlow-precision-recip-sqrt" style option,
> >> which would be off by default, would enable the 2-step mode, and
> >> would need to be explicitly enabled (i.e. not implied by -mcpu=foo)
> >> but I don't see what this buys you beyond the Gromacs boost (and even
> >> there you would be creating an Invalid Run as optimization flags must
> >> be applied across all workloads).
> >
> > Any flag that reduces precision (and thus breaks IEEE floating-point
> > semantics) needs to be gated with an “unsafe” flag (i.e. one that is never
> on by default).
> > As a consequence, the “peak”-tuning for SPEC will turn this on… but
> > barely anyone else would.
> >
> >> For the 3-step optimization, it is clear to me that for "generic"
> >> tuning we don't want this to be enabled by default experimental
> >> results and advice in this thread argues against it for thunderx and cortex-
> a57 targets.
> >> However, enabling it based on the CPU tuning selected seems fine to me.
> >
> > I do not agree on this one, as I would like to see the safe form (i.e.
> > 3 and 5 iterations respectively) to become the default. Most
> > “server-type” chips should not see a performance regression, while it
> > will be easier to optimise for this in hardware than for a
> > (potentially microcoded) sqrt-instruction (and subsequent, dependent
> divide).
> >
> > I have not heard anyone claim a performance regression (either on
> > thunderx or on cortex-a57), but merely heard a “no speed-up”.
> 
> Actually it does regress performance on thunderX, I just assumed that when
> I said not going to be a win it was taken as a slow down. It regress gromacs by
> more than 10% on thunderX but I can't remember how much as i had
> someone else run it. The latency difference is also over 40%; for example
> single precision: 29 cycles with div (12) sqrt(17) directly vs 42 cycles with the
> rsqrte and 2 iterations of 2mul/rsqrts (double is 53 vs 60). That is huge
> difference right there.  ThunderX has a fast div and a fast sqrt for 32bit and a
> reasonable one for double.   So again this is not just not a win but rather a
> regression for thunderX. I suspect cortex-a57 is also true.
> 
> Thanks,
> Andrew
> 

Yes theoretically  should be  true for cortex-57 case as well.   But  I believe hardware pipelining with instruction scheduling in compiler helps a little for gromacs case  ~3% to 4% with the original patch.

I have not tested other FP benchmarks.   As James said a flag -mlow-precision-recip-sqrt if allowed can be used as a peak flag. 

> >
> > So I am strongly in favor of defaulting to the ‘safe’ number of
> > iterations, even when compiling for a generic target.
> >
> > Best,
> > Philipp.
> >

Regards,
Venkat.

RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Evandro Menezes]

FWIW, I was curious about the precision of the results using such instructions for the standard sqrt{,f} functions.  This is not a wide sample, but it does point to a floor of series iterations to 3 for DP and 2 for SP:

x               sqrt(x)         1 Step (ulps)           2 Steps (ulps)          3 Steps (ulps)
2.2251e-308     1.4917e-154     1.4917e-154 (999)       1.4917e-154 (999)       1.4917e-154 (000)
1.6022e-19      4.0027e-10      4.0027e-10 (999)        4.0027e-10 (999)        4.0027e-10 (000)
1.0000e+00      1.0000e+00      1.0000e+00 (001)        1.0000e+00 (001)        1.0000e+00 (001)
1.0000e+00      1.0000e+00      9.9999e-01 (999)        1.0000e+00 (999)        1.0000e+00 (000)
1.0000e+00      1.0000e+00      9.9999e-01 (999)        1.0000e+00 (999)        1.0000e+00 (000)
2.0000e+00      1.4142e+00      1.4142e+00 (999)        1.4142e+00 (999)        1.4142e+00 (000)
2.2500e+00      1.5000e+00      1.5000e+00 (999)        1.5000e+00 (999)        1.5000e+00 (000)
2.5600e+00      1.6000e+00      1.6000e+00 (000)        1.6000e+00 (000)        1.6000e+00 (000)
3.1416e+00      1.7725e+00      1.7725e+00 (999)        1.7725e+00 (999)        1.7725e+00 (000)
6.0221e+23      7.7602e+11      7.7602e+11 (999)        7.7602e+11 (999)        7.7602e+11 (000)
1.7977e+308     1.3408e+154     1.3408e+154 (000)       1.3408e+154 (000)       1.3408e+154 (000)

x               sqrtf(x)        1 Step (ulps)           2 Steps (ulps)          3 Steps (ulps)
1.1755e-38      1.0842e-19      1.0842e-19 (096)        1.0842e-19 (000)        1.0842e-19 (000)
1.6022e-19      4.0027e-10      4.0027e-10 (008)        4.0027e-10 (000)        4.0027e-10 (000)
1.0000e+00      1.0000e+00      1.0000e+00 (001)        1.0000e+00 (001)        1.0000e+00 (001)
1.0000e+00      1.0000e+00      9.9999e-01 (096)        1.0000e+00 (000)        1.0000e+00 (000)
1.0000e+00      1.0000e+00      9.9999e-01 (094)        1.0000e+00 (001)        1.0000e+00 (000)
2.0000e+00      1.4142e+00      1.4142e+00 (146)        1.4142e+00 (001)        1.4142e+00 (000)
2.2500e+00      1.5000e+00      1.5000e+00 (018)        1.5000e+00 (000)        1.5000e+00 (001)
2.5600e+00      1.6000e+00      1.6000e+00 (001)        1.6000e+00 (001)        1.6000e+00 (001)
3.1416e+00      1.7725e+00      1.7725e+00 (006)        1.7725e+00 (001)        1.7725e+00 (001)
6.0221e+23      7.7602e+11      7.7602e+11 (069)        7.7602e+11 (001)        7.7602e+11 (000)
3.4028e+38      1.8447e+19      1.8447e+19 (000)        1.8447e+19 (000)        1.8447e+19 (000)

The error in ULPs saturates at 999 above.

The result of having to use so many iterations to achieve accuracy would defeat using the Newton series, as it would likely be slower than the FSQRT instruction.

Unlike in x86, I have the impression that the initial estimate in AArch64 is meant to be used in applications that do not require precision, like graphics, etc.  Then, a single series iteration for SP would perhaps be good enough.

-- 
Evandro Menezes                              Austin, TX


> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org] On
> Behalf Of Dr. Philipp Tomsich
> Sent: Monday, June 29, 2015 6:45
> To: James Greenhalgh
> Cc: Kumar, Venkataramanan; pinskia@gmail.com; Benedikt Huber; gcc-
> patches@gcc.gnu.org; Marcus Shawcroft; Ramana Radhakrishnan; Richard Earnshaw
> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> estimation in -ffast-math
> 
> James,
> 
> On 29 Jun 2015, at 13:36, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> >
> > On Mon, Jun 29, 2015 at 10:18:23AM +0100, Kumar, Venkataramanan wrote:
> >>
> >>> -----Original Message-----
> >>> From: Dr. Philipp Tomsich
> >>> [mailto:philipp.tomsich@theobroma-systems.com]
> >>> Sent: Monday, June 29, 2015 2:17 PM
> >>> To: Kumar, Venkataramanan
> >>> Cc: pinskia@gmail.com; Benedikt Huber; gcc-patches@gcc.gnu.org
> >>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> >>> (rsqrt) estimation in -ffast-math
> >>>
> >>> Kumar,
> >>>
> >>> This does not come unexpected, as the initial estimation and each
> >>> iteration will add an architecturally-defined number of bits of
> >>> precision (ARMv8 guarantuees only a minimum number of bits provided
> >>> per operation… the exact number is specific to each micro-arch, though).
> >>> Depending on your architecture and on the required number of precise
> >>> bits by any given benchmark, one may see miscompares.
> >>
> >> True.
> >
> > I would be very uncomfortable with this approach.
> 
> Same here. The default must be safe. Always.
> Unlike other architectures, we don’t have a problem with making the proper
> defaults for “safety”, as the ARMv8 ISA guarantees a minimum number of
> precise bits per iteration.
> 
> > From Richard Biener's post in the thread Michael Matz linked earlier
> > in the thread:
> >
> >    It would follow existing practice of things we allow in
> >    -funsafe-math-optimizations.  Existing practice in that we
> >    want to allow -ffast-math use with common benchmarks we care
> >    about.
> >
> >    https://gcc.gnu.org/ml/gcc-patches/2009-11/msg00100.html
> >
> > With the solution you seem to be converging on (2-steps for some
> > microarchitectures, 3 for others), a binary generated for one
> > micro-arch may drop below a minimum guarantee of precision when run on
> > another. This seems to go against the spirit of the practice above. I
> > would only support adding this optimization to -Ofast if we could keep
> > to architectural guarantees of precision in the generated code (i.e. 3-
> steps everywhere).
> >
> > I don't object to adding a "-mlow-precision-recip-sqrt" style option,
> > which would be off by default, would enable the 2-step mode, and would
> > need to be explicitly enabled (i.e. not implied by -mcpu=foo) but I
> > don't see what this buys you beyond the Gromacs boost (and even there
> > you would be creating an Invalid Run as optimization flags must be
> > applied across all workloads).
> 
> Any flag that reduces precision (and thus breaks IEEE floating-point
> semantics) needs to be gated with an “unsafe” flag (i.e. one that is never on
> by default).
> As a consequence, the “peak”-tuning for SPEC will turn this on… but barely
> anyone else would.
> 
> > For the 3-step optimization, it is clear to me that for "generic"
> > tuning we don't want this to be enabled by default experimental
> > results and advice in this thread argues against it for thunderx and
> cortex-a57 targets.
> > However, enabling it based on the CPU tuning selected seems fine to me.
> 
> I do not agree on this one, as I would like to see the safe form (i.e. 3 and
> 5 iterations respectively) to become the default. Most “server-type” chips
> should not see a performance regression, while it will be easier to optimise
> for this in hardware than for a (potentially microcoded) sqrt-instruction
> (and subsequent, dependent divide).
> 
> I have not heard anyone claim a performance regression (either on thunderx or
> on cortex-a57), but merely heard a “no speed-up”.
> 
> So I am strongly in favor of defaulting to the ‘safe’ number of iterations,
> even when compiling for a generic target.
> 
> Best,
> Philipp.

RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Evandro Menezes]

For both FRECPE and FRSQRTE the ARMv8 ISA guide states in their pseudo-code that:

"Result is double-precision and a multiple of 1/256 in the range 1 to 511/256."

This suggests that the estimate is merely 8 bits long.

IIRC, x86 returns 12 bits for its equivalent insns, requiring then a single series iteration for both SP and DP to achieve a precise enough result.

-- 
Evandro Menezes                              Austin, TX


> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org] On
> Behalf Of Dr. Philipp Tomsich
> Sent: Monday, June 29, 2015 3:47
> To: Kumar, Venkataramanan
> Cc: pinskia@gmail.com; Benedikt Huber; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> estimation in -ffast-math
> 
> Kumar,
> 
> This does not come unexpected, as the initial estimation and each iteration
> will add an architecturally-defined number of bits of precision (ARMv8
> guarantuees only a minimum number of bits provided per operation… the exact
> number is specific to each micro-arch, though).
> Depending on your architecture and on the required number of precise bits by
> any given benchmark, one may see miscompares.
> 
> Do you know the exact number of bits that the initial estimate and the
> subsequent refinement steps add for your micro-arch?
> 
> Thanks,
> Philipp.
> 
> > On 29 Jun 2015, at 10:17, Kumar, Venkataramanan
> <Venkataramanan.Kumar@amd.com> wrote:
> >
> >
> > Hmm,  Reducing the iterations to "1 step for float" and "2 steps for
> double"
> >
> > I got VE (miscompares) on following benchmarks 416.gamess
> > 453.povray
> > 454.calculix
> > 459.GemsFDTD
> >
> > Benedikt , I have ICE for 444.namd with your patch,  not sure if something
> wrong in my local tree.
> >
> > Regards,
> > Venkat.
> >
> >> -----Original Message-----
> >> From: pinskia@gmail.com [mailto:pinskia@gmail.com]
> >> Sent: Sunday, June 28, 2015 8:35 PM
> >> To: Kumar, Venkataramanan
> >> Cc: Dr. Philipp Tomsich; Benedikt Huber; gcc-patches@gcc.gnu.org
> >> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> >> (rsqrt) estimation in -ffast-math
> >>
> >>
> >>
> >>
> >>
> >>> On Jun 25, 2015, at 9:44 AM, Kumar, Venkataramanan
> >> <Venkataramanan.Kumar@amd.com> wrote:
> >>>
> >>> I got around ~12% gain with -Ofast -mcpu=cortex-a57.
> >>
> >> I get around 11/12% on thunderX with the patch and the decreasing the
> >> iterations change (1/2) compared to without the patch.
> >>
> >> Thanks,
> >> Andrew
> >>
> >>
> >>>
> >>> Regards,
> >>> Venkat.
> >>>
> >>>> -----Original Message-----
> >>>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> >>>> owner@gcc.gnu.org] On Behalf Of Dr. Philipp Tomsich
> >>>> Sent: Thursday, June 25, 2015 9:13 PM
> >>>> To: Kumar, Venkataramanan
> >>>> Cc: Benedikt Huber; pinskia@gmail.com; gcc-patches@gcc.gnu.org
> >>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> >>>> (rsqrt) estimation in -ffast-math
> >>>>
> >>>> Kumar,
> >>>>
> >>>> what is the relative gain that you see on Cortex-A57?
> >>>>
> >>>> Thanks,
> >>>> Philipp.
> >>>>
> >>>>>> On 25 Jun 2015, at 17:35, Kumar, Venkataramanan
> >>>>> <Venkataramanan.Kumar@amd.com> wrote:
> >>>>>
> >>>>> Changing to  "1 step for float" and "2 steps for double" gives
> >>>>> better gains
> >>>> now for gromacs on cortex-a57.
> >>>>>
> >>>>> Regards,
> >>>>> Venkat.
> >>>>>> -----Original Message-----
> >>>>>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> >>>>>> owner@gcc.gnu.org] On Behalf Of Benedikt Huber
> >>>>>> Sent: Thursday, June 25, 2015 4:09 PM
> >>>>>> To: pinskia@gmail.com
> >>>>>> Cc: gcc-patches@gcc.gnu.org; philipp.tomsich@theobroma-
> >> systems.com
> >>>>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> >>>>>> (rsqrt) estimation in -ffast-math
> >>>>>>
> >>>>>> Andrew,
> >>>>>>
> >>>>>>> This is NOT a win on thunderX at least for single precision
> >>>>>>> because you have
> >>>>>> to do the divide and sqrt in the same time as it takes 5
> >>>>>> multiples (estimate and step are multiplies in the thunderX pipeline).
> >>>>>> Doubles is 10 multiplies which is just the same as what the patch
> >>>>>> does (but it is really slightly less than 10, I rounded up). So
> >>>>>> in the end this is NOT a win at all for thunderX unless we do one
> >>>>>> less step for both single
> >>>> and double.
> >>>>>>
> >>>>>> Yes, the expected benefit from rsqrt estimation is implementation
> >>>>>> specific. If one has a better initial rsqrte or an application
> >>>>>> that can trade precision for execution time, we could offer a
> >>>>>> command line option to do only 2 steps for doulbe and 1 step for
> >>>>>> float; similar to -
> >>>> mrecip-precision for PowerPC.
> >>>>>> What are your thoughts on that?
> >>>>>>
> >>>>>> Best regards,
> >>>>>> Benedikt
> >>>

RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Evandro Menezes]

I ran a simple test on A57 rev. 0, looping a million times around sqrt{,f} and the respective series iterations with the values in the sequence 1..1000000 and got these results:

sqrt(x):        36593844/s      1/sqrt(x):      18283875/s
3 Steps:        47922557/s      3 Steps:        49005194/s

sqrtf(x):       143988480/s     1/sqrtf(x):     69516857/s
2 Steps:        78740157/s      2 Steps:        80385852/s

I'm a bit surprised that the 3-iteration series for DP is faster than sqrt(), but not that it's much faster for the reciprocal of sqrt().  As for SP, the 2-iteration series is faster only for the reciprocal for sqrtf().

There might still be some leg for this patch in real-world cases which I'd like to investigate.

-- 
Evandro Menezes                              Austin, TX


> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org] On
> Behalf Of Kumar, Venkataramanan
> Sent: Monday, June 29, 2015 13:50
> To: pinskia@gmail.com; Dr. Philipp Tomsich
> Cc: James Greenhalgh; Benedikt Huber; gcc-patches@gcc.gnu.org; Marcus
> Shawcroft; Ramana Radhakrishnan; Richard Earnshaw
> Subject: RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> estimation in -ffast-math
> 
> Hi,
> 
> > -----Original Message-----
> > From: pinskia@gmail.com [mailto:pinskia@gmail.com]
> > Sent: Monday, June 29, 2015 10:23 PM
> > To: Dr. Philipp Tomsich
> > Cc: James Greenhalgh; Kumar, Venkataramanan; Benedikt Huber; gcc-
> > patches@gcc.gnu.org; Marcus Shawcroft; Ramana Radhakrishnan; Richard
> > Earnshaw
> > Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> > (rsqrt) estimation in -ffast-math
> >
> >
> >
> >
> >
> > > On Jun 29, 2015, at 4:44 AM, Dr. Philipp Tomsich
> > <philipp.tomsich@theobroma-systems.com> wrote:
> > >
> > > James,
> > >
> > >> On 29 Jun 2015, at 13:36, James Greenhalgh
> > <james.greenhalgh@arm.com> wrote:
> > >>
> > >>> On Mon, Jun 29, 2015 at 10:18:23AM +0100, Kumar, Venkataramanan
> > wrote:
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: Dr. Philipp Tomsich
> > >>>> [mailto:philipp.tomsich@theobroma-systems.com]
> > >>>> Sent: Monday, June 29, 2015 2:17 PM
> > >>>> To: Kumar, Venkataramanan
> > >>>> Cc: pinskia@gmail.com; Benedikt Huber; gcc-patches@gcc.gnu.org
> > >>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> > >>>> (rsqrt) estimation in -ffast-math
> > >>>>
> > >>>> Kumar,
> > >>>>
> > >>>> This does not come unexpected, as the initial estimation and each
> > >>>> iteration will add an architecturally-defined number of bits of
> > >>>> precision (ARMv8 guarantuees only a minimum number of bits
> > provided
> > >>>> per operation… the exact number is specific to each micro-arch,
> > though).
> > >>>> Depending on your architecture and on the required number of
> > >>>> precise bits by any given benchmark, one may see miscompares.
> > >>>
> > >>> True.
> > >>
> > >> I would be very uncomfortable with this approach.
> > >
> > > Same here. The default must be safe. Always.
> > > Unlike other architectures, we don’t have a problem with making the
> > > proper defaults for “safety”, as the ARMv8 ISA guarantees a minimum
> > > number of precise bits per iteration.
> > >
> > >> From Richard Biener's post in the thread Michael Matz linked
> > >> earlier in the thread:
> > >>
> > >>   It would follow existing practice of things we allow in
> > >>   -funsafe-math-optimizations.  Existing practice in that we
> > >>   want to allow -ffast-math use with common benchmarks we care
> > >>   about.
> > >>
> > >>   https://gcc.gnu.org/ml/gcc-patches/2009-11/msg00100.html
> > >>
> > >> With the solution you seem to be converging on (2-steps for some
> > >> microarchitectures, 3 for others), a binary generated for one
> > >> micro-arch may drop below a minimum guarantee of precision when run
> > >> on another. This seems to go against the spirit of the practice
> > >> above. I would only support adding this optimization to -Ofast if
> > >> we could keep to architectural guarantees of precision in the
> > >> generated code
> > (i.e. 3-steps everywhere).
> > >>
> > >> I don't object to adding a "-mlow-precision-recip-sqrt" style
> > >> option, which would be off by default, would enable the 2-step
> > >> mode, and would need to be explicitly enabled (i.e. not implied by
> > >> -mcpu=foo) but I don't see what this buys you beyond the Gromacs
> > >> boost (and even there you would be creating an Invalid Run as
> > >> optimization flags must be applied across all workloads).
> > >
> > > Any flag that reduces precision (and thus breaks IEEE floating-point
> > > semantics) needs to be gated with an “unsafe” flag (i.e. one that is
> > > never
> > on by default).
> > > As a consequence, the “peak”-tuning for SPEC will turn this on… but
> > > barely anyone else would.
> > >
> > >> For the 3-step optimization, it is clear to me that for "generic"
> > >> tuning we don't want this to be enabled by default experimental
> > >> results and advice in this thread argues against it for thunderx
> > >> and cortex-
> > a57 targets.
> > >> However, enabling it based on the CPU tuning selected seems fine to me.
> > >
> > > I do not agree on this one, as I would like to see the safe form (i.e.
> > > 3 and 5 iterations respectively) to become the default. Most
> > > “server-type” chips should not see a performance regression, while
> > > it will be easier to optimise for this in hardware than for a
> > > (potentially microcoded) sqrt-instruction (and subsequent, dependent
> > divide).
> > >
> > > I have not heard anyone claim a performance regression (either on
> > > thunderx or on cortex-a57), but merely heard a “no speed-up”.
> >
> > Actually it does regress performance on thunderX, I just assumed that
> > when I said not going to be a win it was taken as a slow down. It
> > regress gromacs by more than 10% on thunderX but I can't remember how
> > much as i had someone else run it. The latency difference is also over
> > 40%; for example single precision: 29 cycles with div (12) sqrt(17)
> > directly vs 42 cycles with the rsqrte and 2 iterations of 2mul/rsqrts
> > (double is 53 vs 60). That is huge difference right there.  ThunderX has a
> fast div and a fast sqrt for 32bit and a
> > reasonable one for double.   So again this is not just not a win but rather
> a
> > regression for thunderX. I suspect cortex-a57 is also true.
> >
> > Thanks,
> > Andrew
> >
> 
> Yes theoretically  should be  true for cortex-57 case as well.   But  I
> believe hardware pipelining with instruction scheduling in compiler helps a
> little for gromacs case  ~3% to 4% with the original patch.
> 
> I have not tested other FP benchmarks.   As James said a flag -mlow-
> precision-recip-sqrt if allowed can be used as a peak flag.
> 
> > >
> > > So I am strongly in favor of defaulting to the ‘safe’ number of
> > > iterations, even when compiling for a generic target.
> > >
> > > Best,
> > > Philipp.
> > >
> 
> Regards,
> Venkat.

RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Kumar, Venkataramanan]

Hi, 

I missed your email and noticed it this week.

What does column 2  tests?  Are you trying to implement square roots  using reciprocal estimate and step? 

But reciprocal square root  using reciprocal estimate and (2 for fp 3 for dp) step seems  to be better that using fdiv and fsqrt in your case.   

Regards,
Venkat.

> -----Original Message-----
> From: Evandro Menezes [mailto:e.menezes@samsung.com]
> Sent: Wednesday, July 15, 2015 3:45 AM
> To: Kumar, Venkataramanan; pinskia@gmail.com; 'Dr. Philipp Tomsich'
> Cc: 'James Greenhalgh'; 'Benedikt Huber'; gcc-patches@gcc.gnu.org; 'Marcus
> Shawcroft'; 'Ramana Radhakrishnan'; 'Richard Earnshaw'
> Subject: RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> estimation in -ffast-math
> 
> I ran a simple test on A57 rev. 0, looping a million times around sqrt{,f} and
> the respective series iterations with the values in the sequence 1..1000000
> and got these results:
> 
> sqrt(x):        36593844/s      1/sqrt(x):      18283875/s
> 3 Steps:        47922557/s      3 Steps:        49005194/s
> 
> sqrtf(x):       143988480/s     1/sqrtf(x):     69516857/s
> 2 Steps:        78740157/s      2 Steps:        80385852/s
> 
> I'm a bit surprised that the 3-iteration series for DP is faster than sqrt(), but
> not that it's much faster for the reciprocal of sqrt().  As for SP, the 2-iteration
> series is faster only for the reciprocal for sqrtf().
> 
> There might still be some leg for this patch in real-world cases which I'd like to
> investigate.
> 
> --
> Evandro Menezes                              Austin, TX
> 
> 
> > -----Original Message-----
> > From: gcc-patches-owner@gcc.gnu.org
> > [mailto:gcc-patches-owner@gcc.gnu.org] On Behalf Of Kumar,
> > Venkataramanan
> > Sent: Monday, June 29, 2015 13:50
> > To: pinskia@gmail.com; Dr. Philipp Tomsich
> > Cc: James Greenhalgh; Benedikt Huber; gcc-patches@gcc.gnu.org; Marcus
> > Shawcroft; Ramana Radhakrishnan; Richard Earnshaw
> > Subject: RE: [PATCH] [aarch64] Implemented reciprocal square root
> > (rsqrt) estimation in -ffast-math
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: pinskia@gmail.com [mailto:pinskia@gmail.com]
> > > Sent: Monday, June 29, 2015 10:23 PM
> > > To: Dr. Philipp Tomsich
> > > Cc: James Greenhalgh; Kumar, Venkataramanan; Benedikt Huber; gcc-
> > > patches@gcc.gnu.org; Marcus Shawcroft; Ramana Radhakrishnan;
> Richard
> > > Earnshaw
> > > Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> > > (rsqrt) estimation in -ffast-math
> > >
> > >
> > >
> > >
> > >
> > > > On Jun 29, 2015, at 4:44 AM, Dr. Philipp Tomsich
> > > <philipp.tomsich@theobroma-systems.com> wrote:
> > > >
> > > > James,
> > > >
> > > >> On 29 Jun 2015, at 13:36, James Greenhalgh
> > > <james.greenhalgh@arm.com> wrote:
> > > >>
> > > >>> On Mon, Jun 29, 2015 at 10:18:23AM +0100, Kumar, Venkataramanan
> > > wrote:
> > > >>>
> > > >>>> -----Original Message-----
> > > >>>> From: Dr. Philipp Tomsich
> > > >>>> [mailto:philipp.tomsich@theobroma-systems.com]
> > > >>>> Sent: Monday, June 29, 2015 2:17 PM
> > > >>>> To: Kumar, Venkataramanan
> > > >>>> Cc: pinskia@gmail.com; Benedikt Huber; gcc-patches@gcc.gnu.org
> > > >>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square
> > > >>>> root
> > > >>>> (rsqrt) estimation in -ffast-math
> > > >>>>
> > > >>>> Kumar,
> > > >>>>
> > > >>>> This does not come unexpected, as the initial estimation and
> > > >>>> each iteration will add an architecturally-defined number of
> > > >>>> bits of precision (ARMv8 guarantuees only a minimum number of
> > > >>>> bits
> > > provided
> > > >>>> per operation… the exact number is specific to each micro-arch,
> > > though).
> > > >>>> Depending on your architecture and on the required number of
> > > >>>> precise bits by any given benchmark, one may see miscompares.
> > > >>>
> > > >>> True.
> > > >>
> > > >> I would be very uncomfortable with this approach.
> > > >
> > > > Same here. The default must be safe. Always.
> > > > Unlike other architectures, we don’t have a problem with making
> > > > the proper defaults for “safety”, as the ARMv8 ISA guarantees a
> > > > minimum number of precise bits per iteration.
> > > >
> > > >> From Richard Biener's post in the thread Michael Matz linked
> > > >> earlier in the thread:
> > > >>
> > > >>   It would follow existing practice of things we allow in
> > > >>   -funsafe-math-optimizations.  Existing practice in that we
> > > >>   want to allow -ffast-math use with common benchmarks we care
> > > >>   about.
> > > >>
> > > >>   https://gcc.gnu.org/ml/gcc-patches/2009-11/msg00100.html
> > > >>
> > > >> With the solution you seem to be converging on (2-steps for some
> > > >> microarchitectures, 3 for others), a binary generated for one
> > > >> micro-arch may drop below a minimum guarantee of precision when
> > > >> run on another. This seems to go against the spirit of the
> > > >> practice above. I would only support adding this optimization to
> > > >> -Ofast if we could keep to architectural guarantees of precision
> > > >> in the generated code
> > > (i.e. 3-steps everywhere).
> > > >>
> > > >> I don't object to adding a "-mlow-precision-recip-sqrt" style
> > > >> option, which would be off by default, would enable the 2-step
> > > >> mode, and would need to be explicitly enabled (i.e. not implied
> > > >> by
> > > >> -mcpu=foo) but I don't see what this buys you beyond the Gromacs
> > > >> boost (and even there you would be creating an Invalid Run as
> > > >> optimization flags must be applied across all workloads).
> > > >
> > > > Any flag that reduces precision (and thus breaks IEEE
> > > > floating-point
> > > > semantics) needs to be gated with an “unsafe” flag (i.e. one that
> > > > is never
> > > on by default).
> > > > As a consequence, the “peak”-tuning for SPEC will turn this on…
> > > > but barely anyone else would.
> > > >
> > > >> For the 3-step optimization, it is clear to me that for "generic"
> > > >> tuning we don't want this to be enabled by default experimental
> > > >> results and advice in this thread argues against it for thunderx
> > > >> and cortex-
> > > a57 targets.
> > > >> However, enabling it based on the CPU tuning selected seems fine to
> me.
> > > >
> > > > I do not agree on this one, as I would like to see the safe form (i.e.
> > > > 3 and 5 iterations respectively) to become the default. Most
> > > > “server-type” chips should not see a performance regression, while
> > > > it will be easier to optimise for this in hardware than for a
> > > > (potentially microcoded) sqrt-instruction (and subsequent,
> > > > dependent
> > > divide).
> > > >
> > > > I have not heard anyone claim a performance regression (either on
> > > > thunderx or on cortex-a57), but merely heard a “no speed-up”.
> > >
> > > Actually it does regress performance on thunderX, I just assumed
> > > that when I said not going to be a win it was taken as a slow down.
> > > It regress gromacs by more than 10% on thunderX but I can't remember
> > > how much as i had someone else run it. The latency difference is
> > > also over 40%; for example single precision: 29 cycles with div (12)
> > > sqrt(17) directly vs 42 cycles with the rsqrte and 2 iterations of
> > > 2mul/rsqrts (double is 53 vs 60). That is huge difference right
> > > there.  ThunderX has a
> > fast div and a fast sqrt for 32bit and a
> > > reasonable one for double.   So again this is not just not a win but rather
> > a
> > > regression for thunderX. I suspect cortex-a57 is also true.
> > >
> > > Thanks,
> > > Andrew
> > >
> >
> > Yes theoretically  should be  true for cortex-57 case as well.   But  I
> > believe hardware pipelining with instruction scheduling in compiler
> > helps a little for gromacs case  ~3% to 4% with the original patch.
> >
> > I have not tested other FP benchmarks.   As James said a flag -mlow-
> > precision-recip-sqrt if allowed can be used as a peak flag.
> >
> > > >
> > > > So I am strongly in favor of defaulting to the ‘safe’ number of
> > > > iterations, even when compiling for a generic target.
> > > >
> > > > Best,
> > > > Philipp.
> > > >
> >
> > Regards,
> > Venkat.

RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt) estimation in -ffast-math

[Evandro Menezes]

Hi, Venkat.

Since x^1/2 = x * x^-1/2, the Newton series can also be used for the regular square root with an extra multiplication, as it is done in x86.  That's what I was trying to estimate below.

Cheers,

-- 
Evandro Menezes                              Austin, TX

> -----Original Message-----
> From: Kumar, Venkataramanan [mailto:Venkataramanan.Kumar@amd.com]
> Sent: Monday, July 20, 2015 2:53
> To: Evandro Menezes; pinskia@gmail.com; 'Dr. Philipp Tomsich'
> Cc: 'James Greenhalgh'; 'Benedikt Huber'; gcc-patches@gcc.gnu.org; 'Marcus
> Shawcroft'; 'Ramana Radhakrishnan'; 'Richard Earnshaw'
> Subject: RE: [PATCH] [aarch64] Implemented reciprocal square root (rsqrt)
> estimation in -ffast-math
> 
> Hi,
> 
> I missed your email and noticed it this week.
> 
> What does column 2  tests?  Are you trying to implement square roots  using
> reciprocal estimate and step?
> 
> But reciprocal square root  using reciprocal estimate and (2 for fp 3 for dp)
> step seems  to be better that using fdiv and fsqrt in your case.
> 
> Regards,
> Venkat.
> 
> > -----Original Message-----
> > From: Evandro Menezes [mailto:e.menezes@samsung.com]
> > Sent: Wednesday, July 15, 2015 3:45 AM
> > To: Kumar, Venkataramanan; pinskia@gmail.com; 'Dr. Philipp Tomsich'
> > Cc: 'James Greenhalgh'; 'Benedikt Huber'; gcc-patches@gcc.gnu.org;
> > 'Marcus Shawcroft'; 'Ramana Radhakrishnan'; 'Richard Earnshaw'
> > Subject: RE: [PATCH] [aarch64] Implemented reciprocal square root
> > (rsqrt) estimation in -ffast-math
> >
> > I ran a simple test on A57 rev. 0, looping a million times around
> > sqrt{,f} and the respective series iterations with the values in the
> > sequence 1..1000000 and got these results:
> >
> > sqrt(x):        36593844/s      1/sqrt(x):      18283875/s
> > 3 Steps:        47922557/s      3 Steps:        49005194/s
> >
> > sqrtf(x):       143988480/s     1/sqrtf(x):     69516857/s
> > 2 Steps:        78740157/s      2 Steps:        80385852/s
> >
> > I'm a bit surprised that the 3-iteration series for DP is faster than
> > sqrt(), but not that it's much faster for the reciprocal of sqrt().
> > As for SP, the 2-iteration series is faster only for the reciprocal for
> sqrtf().
> >
> > There might still be some leg for this patch in real-world cases which
> > I'd like to investigate.
> >
> > --
> > Evandro Menezes                              Austin, TX
> >
> >
> > > -----Original Message-----
> > > From: gcc-patches-owner@gcc.gnu.org
> > > [mailto:gcc-patches-owner@gcc.gnu.org] On Behalf Of Kumar,
> > > Venkataramanan
> > > Sent: Monday, June 29, 2015 13:50
> > > To: pinskia@gmail.com; Dr. Philipp Tomsich
> > > Cc: James Greenhalgh; Benedikt Huber; gcc-patches@gcc.gnu.org;
> > > Marcus Shawcroft; Ramana Radhakrishnan; Richard Earnshaw
> > > Subject: RE: [PATCH] [aarch64] Implemented reciprocal square root
> > > (rsqrt) estimation in -ffast-math
> > >
> > > Hi,
> > >
> > > > -----Original Message-----
> > > > From: pinskia@gmail.com [mailto:pinskia@gmail.com]
> > > > Sent: Monday, June 29, 2015 10:23 PM
> > > > To: Dr. Philipp Tomsich
> > > > Cc: James Greenhalgh; Kumar, Venkataramanan; Benedikt Huber; gcc-
> > > > patches@gcc.gnu.org; Marcus Shawcroft; Ramana Radhakrishnan;
> > Richard
> > > > Earnshaw
> > > > Subject: Re: [PATCH] [aarch64] Implemented reciprocal square root
> > > > (rsqrt) estimation in -ffast-math
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > > On Jun 29, 2015, at 4:44 AM, Dr. Philipp Tomsich
> > > > <philipp.tomsich@theobroma-systems.com> wrote:
> > > > >
> > > > > James,
> > > > >
> > > > >> On 29 Jun 2015, at 13:36, James Greenhalgh
> > > > <james.greenhalgh@arm.com> wrote:
> > > > >>
> > > > >>> On Mon, Jun 29, 2015 at 10:18:23AM +0100, Kumar,
> > > > >>> Venkataramanan
> > > > wrote:
> > > > >>>
> > > > >>>> -----Original Message-----
> > > > >>>> From: Dr. Philipp Tomsich
> > > > >>>> [mailto:philipp.tomsich@theobroma-systems.com]
> > > > >>>> Sent: Monday, June 29, 2015 2:17 PM
> > > > >>>> To: Kumar, Venkataramanan
> > > > >>>> Cc: pinskia@gmail.com; Benedikt Huber;
> > > > >>>> gcc-patches@gcc.gnu.org
> > > > >>>> Subject: Re: [PATCH] [aarch64] Implemented reciprocal square
> > > > >>>> root
> > > > >>>> (rsqrt) estimation in -ffast-math
> > > > >>>>
> > > > >>>> Kumar,
> > > > >>>>
> > > > >>>> This does not come unexpected, as the initial estimation and
> > > > >>>> each iteration will add an architecturally-defined number of
> > > > >>>> bits of precision (ARMv8 guarantuees only a minimum number of
> > > > >>>> bits
> > > > provided
> > > > >>>> per operation… the exact number is specific to each
> > > > >>>> micro-arch,
> > > > though).
> > > > >>>> Depending on your architecture and on the required number of
> > > > >>>> precise bits by any given benchmark, one may see miscompares.
> > > > >>>
> > > > >>> True.
> > > > >>
> > > > >> I would be very uncomfortable with this approach.
> > > > >
> > > > > Same here. The default must be safe. Always.
> > > > > Unlike other architectures, we don’t have a problem with making
> > > > > the proper defaults for “safety”, as the ARMv8 ISA guarantees a
> > > > > minimum number of precise bits per iteration.
> > > > >
> > > > >> From Richard Biener's post in the thread Michael Matz linked
> > > > >> earlier in the thread:
> > > > >>
> > > > >>   It would follow existing practice of things we allow in
> > > > >>   -funsafe-math-optimizations.  Existing practice in that we
> > > > >>   want to allow -ffast-math use with common benchmarks we care
> > > > >>   about.
> > > > >>
> > > > >>   https://gcc.gnu.org/ml/gcc-patches/2009-11/msg00100.html
> > > > >>
> > > > >> With the solution you seem to be converging on (2-steps for
> > > > >> some microarchitectures, 3 for others), a binary generated for
> > > > >> one micro-arch may drop below a minimum guarantee of precision
> > > > >> when run on another. This seems to go against the spirit of the
> > > > >> practice above. I would only support adding this optimization
> > > > >> to -Ofast if we could keep to architectural guarantees of
> > > > >> precision in the generated code
> > > > (i.e. 3-steps everywhere).
> > > > >>
> > > > >> I don't object to adding a "-mlow-precision-recip-sqrt" style
> > > > >> option, which would be off by default, would enable the 2-step
> > > > >> mode, and would need to be explicitly enabled (i.e. not implied
> > > > >> by
> > > > >> -mcpu=foo) but I don't see what this buys you beyond the
> > > > >> Gromacs boost (and even there you would be creating an Invalid
> > > > >> Run as optimization flags must be applied across all workloads).
> > > > >
> > > > > Any flag that reduces precision (and thus breaks IEEE
> > > > > floating-point
> > > > > semantics) needs to be gated with an “unsafe” flag (i.e. one
> > > > > that is never
> > > > on by default).
> > > > > As a consequence, the “peak”-tuning for SPEC will turn this on…
> > > > > but barely anyone else would.
> > > > >
> > > > >> For the 3-step optimization, it is clear to me that for "generic"
> > > > >> tuning we don't want this to be enabled by default experimental
> > > > >> results and advice in this thread argues against it for
> > > > >> thunderx and cortex-
> > > > a57 targets.
> > > > >> However, enabling it based on the CPU tuning selected seems
> > > > >> fine to
> > me.
> > > > >
> > > > > I do not agree on this one, as I would like to see the safe form
> (i.e.
> > > > > 3 and 5 iterations respectively) to become the default. Most
> > > > > “server-type” chips should not see a performance regression,
> > > > > while it will be easier to optimise for this in hardware than
> > > > > for a (potentially microcoded) sqrt-instruction (and subsequent,
> > > > > dependent
> > > > divide).
> > > > >
> > > > > I have not heard anyone claim a performance regression (either
> > > > > on thunderx or on cortex-a57), but merely heard a “no speed-up”.
> > > >
> > > > Actually it does regress performance on thunderX, I just assumed
> > > > that when I said not going to be a win it was taken as a slow down.
> > > > It regress gromacs by more than 10% on thunderX but I can't
> > > > remember how much as i had someone else run it. The latency
> > > > difference is also over 40%; for example single precision: 29
> > > > cycles with div (12)
> > > > sqrt(17) directly vs 42 cycles with the rsqrte and 2 iterations of
> > > > 2mul/rsqrts (double is 53 vs 60). That is huge difference right
> > > > there.  ThunderX has a
> > > fast div and a fast sqrt for 32bit and a
> > > > reasonable one for double.   So again this is not just not a win but
> rather
> > > a
> > > > regression for thunderX. I suspect cortex-a57 is also true.
> > > >
> > > > Thanks,
> > > > Andrew
> > > >
> > >
> > > Yes theoretically  should be  true for cortex-57 case as well.   But  I
> > > believe hardware pipelining with instruction scheduling in compiler
> > > helps a little for gromacs case  ~3% to 4% with the original patch.
> > >
> > > I have not tested other FP benchmarks.   As James said a flag -mlow-
> > > precision-recip-sqrt if allowed can be used as a peak flag.
> > >
> > > > >
> > > > > So I am strongly in favor of defaulting to the ‘safe’ number of
> > > > > iterations, even when compiling for a generic target.
> > > > >
> > > > > Best,
> > > > > Philipp.
> > > > >
> > >
> > > Regards,
> > > Venkat.