Re: move sincos after pre (was: Re: [Ada, FYI] revamp ada.numerics.aux)

[Richard Biener <richard.guenther@gmail.com>]

On October 23, 2020 4:23:35 PM GMT+02:00, Alexandre Oliva <oliva@adacore.com> wrote:
>On Oct 22, 2020, Alexandre Oliva <oliva@adacore.com> wrote:
>
>> On Oct 18, 2020, Alexandre Oliva <oliva@adacore.com> wrote:
>>> The option is provided by default, but there is an alternate version
>>> that doesn't, that is used for vxworks targets.
>
>> vxworks float EFs not precise enough -> use long float
>
>> From: Alexandre Oliva <oliva@adacore.com>
>
>> Some acats-4 tests that check the precision of Float elementary
>> functions fail with vxworks 7.2's implementations of single-precision
>> math functions.
>
>> This patch arranges for us to bypass the single-precision functions,
>> and use the Aux_Long_Float implementation, based on the double-typed
>> calls from the C library, for Float and Short_Float.
>
>
>On platforms in which Aux_[Real_Type] involves non-NOP conversions
>(e.g., between single- and double-precision, or between short float
>and float), the conversions before the calls are CSEd too late for
>sincos to combine calls.
>
>This patch moves sincos after PRE, where the conversions are unified
>at -O2.
>
>I'm regstrapping this on x86_64-linux-gnu and powerpc64-linux-gnu, and
>also testing it on affected platforms.  Another way to go, that would
>take a little more effort, would be to extend sincos to take equivalent
>conversions as the same operand, but I doubt I'll be able to undertake
>that any time soon, so...  Is this one ok to install?

Can you move it one pass further after sink please? Also I don't remember exactly but does pass_sincos only handle sin/cos unifying? 

Thanks, 
Richard. 

>
>for  gcc/ChangeLog
>
>	* passes.def: Move sincos after pre.
>
>for  gcc/testsuite/ChangeLog
>
>	* gnat.dg/sin_cos.ads: New.
>	* gnat.dg/sin_cos.adb: New.
>	* gcc.dg/sin_cos.c: New.
>---
> gcc/passes.def                    |    2 +-
>gcc/testsuite/gcc.dg/sin_cos.c    |   41
>+++++++++++++++++++++++++++++++++++++
> gcc/testsuite/gnat.dg/sin_cos.adb |   14 +++++++++++++
> gcc/testsuite/gnat.dg/sin_cos.ads |    4 ++++
> 4 files changed, 60 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.dg/sin_cos.c
> create mode 100644 gcc/testsuite/gnat.dg/sin_cos.adb
> create mode 100644 gcc/testsuite/gnat.dg/sin_cos.ads
>
>diff --git a/gcc/passes.def b/gcc/passes.def
>index cf15d8e..2743506 100644
>--- a/gcc/passes.def
>+++ b/gcc/passes.def
>@@ -242,12 +242,12 @@ along with GCC; see the file COPYING3.  If not
>see
>       NEXT_PASS (pass_ccp, true /* nonzero_p */);
>       /* After CCP we rewrite no longer addressed locals into SSA
> 	 form if possible.  */
>-      NEXT_PASS (pass_cse_sincos);
>       NEXT_PASS (pass_optimize_bswap);
>       NEXT_PASS (pass_laddress);
>       NEXT_PASS (pass_lim);
>       NEXT_PASS (pass_walloca, false);
>       NEXT_PASS (pass_pre);
>+      NEXT_PASS (pass_cse_sincos);
>       NEXT_PASS (pass_sink_code);
>       NEXT_PASS (pass_sancov);
>       NEXT_PASS (pass_asan);
>diff --git a/gcc/testsuite/gcc.dg/sin_cos.c
>b/gcc/testsuite/gcc.dg/sin_cos.c
>new file mode 100644
>index 00000000..a4a7727
>--- /dev/null
>+++ b/gcc/testsuite/gcc.dg/sin_cos.c
>@@ -0,0 +1,41 @@
>+/* { dg-do compile } */
>+/* { dg-options "-O2" } */
>+
>+/* This maps to essentially the same gimple that is generated for
>+   gnat.dg/sin_cos.adb, on platforms that use the wraplf variant of
>+   Ada.Numerics.Aux_Float.  The value of EPSILON is not relevant to
>+   the test, but the test must be there to keep the conversions in
>+   different BBs long enough to trigger the problem that prevented the
>+   sincos optimization, because the arguments passed to sin and cos
>+   didn't get unified into a single SSA_NAME in time for sincos.  */
>+
>+#include <math.h>
>+
>+#define EPSILON 3.4526697709225118160247802734375e-4
>+
>+static float my_sinf(float x) {
>+  return (float) sin ((double) x);
>+}
>+
>+static float wrap_sinf(float x) {
>+  if (fabs (x) < EPSILON)
>+    return 0;
>+  return my_sinf (x);
>+}
>+
>+static float my_cosf(float x) {
>+  return (float) cos ((double) x);
>+}
>+
>+static float wrap_cosf(float x) {
>+  if (fabs (x) < EPSILON)
>+    return 1;
>+  return my_cosf (x);
>+}
>+
>+float my_sin_cos(float x, float *s, float *c) {
>+  *s = wrap_sinf (x);
>+  *c = wrap_cosf (x);
>+}
>+
>+/* { dg-final { scan-assembler "sincos\|cexp" { target *-linux-gnu*
>*-w64-mingw* powerpc*-*-* } } } */
>diff --git a/gcc/testsuite/gnat.dg/sin_cos.adb
>b/gcc/testsuite/gnat.dg/sin_cos.adb
>new file mode 100644
>index 00000000..e72f521
>--- /dev/null
>+++ b/gcc/testsuite/gnat.dg/sin_cos.adb
>@@ -0,0 +1,14 @@
>+--  { dg-do compile }
>+--  { dg-options "-O2 -gnatn" }
>+
>+with Ada.Numerics.Elementary_Functions;
>+use Ada.Numerics.Elementary_Functions;
>+package body Sin_Cos is
>+   procedure Sin_Cos (Angle : T; SinA, CosA : out T) is
>+   begin
>+      SinA := Sin (Angle);
>+      CosA := Cos (Angle);
>+   end;
>+end Sin_Cos;
>+
>+--  { dg-final { scan-assembler "sincos\|cexp" { target *-linux-gnu*
>*-w64-mingw* powerpc*-*-* } } }
>diff --git a/gcc/testsuite/gnat.dg/sin_cos.ads
>b/gcc/testsuite/gnat.dg/sin_cos.ads
>new file mode 100644
>index 00000000..a0eff3d
>--- /dev/null
>+++ b/gcc/testsuite/gnat.dg/sin_cos.ads
>@@ -0,0 +1,4 @@
>+package Sin_Cos is
>+   subtype T is Float;
>+   procedure Sin_Cos (Angle : T; SinA, CosA : out T);
>+end Sin_Cos;