Don't create SSA names until in SSA form

Don't create SSA names until in SSA form

[Richard Sandiford]

An upcoming patch adds a fold from hypot(x,x) to fabs(x)*sqrt(2).
This is unusual in that it could trigger in the gimplifier but would
require new SSA names to be created.  This patch makes sure that we
don't try to create new SSA names when we're not yet in SSA form.

Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi.
OK to install?

Thanks,
Richard


gcc/
	* gimple-match-head.c (maybe_push_res_to_seq): Don't attempt
	to create new SSA names if not in SSA form.

diff --git a/gcc/gimple-match-head.c b/gcc/gimple-match-head.c
index 8f72919..1345cf9 100644
--- a/gcc/gimple-match-head.c
+++ b/gcc/gimple-match-head.c
@@ -331,7 +331,11 @@ maybe_push_res_to_seq (code_helper rcode, tree type, tree *ops,
 	      && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (ops[2])))
 	return NULL_TREE;
       if (!res)
-	res = make_ssa_name (type);
+	{
+	  if (!gimple_in_ssa_p (cfun))
+	    return NULL_TREE;
+	  res = make_ssa_name (type);
+	}
       maybe_build_generic_op (rcode, type, &ops[0], ops[1], ops[2]);
       gimple *new_stmt = gimple_build_assign (res, rcode,
 					     ops[0], ops[1], ops[2]);
@@ -361,7 +365,11 @@ maybe_push_res_to_seq (code_helper rcode, tree type, tree *ops,
 	}
       gcc_assert (nargs != 0);
       if (!res)
-	res = make_ssa_name (type);
+	{
+	  if (!gimple_in_ssa_p (cfun))
+	    return NULL_TREE;
+	  res = make_ssa_name (type);
+	}
       gimple *new_stmt = gimple_build_call (decl, nargs, ops[0], ops[1], ops[2]);
       gimple_call_set_lhs (new_stmt, res);
       gimple_seq_add_stmt_without_update (seq, new_stmt);

Re: Don't create SSA names until in SSA form

[Richard Biener]

On Mon, Oct 26, 2015 at 10:36 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> An upcoming patch adds a fold from hypot(x,x) to fabs(x)*sqrt(2).
> This is unusual in that it could trigger in the gimplifier but would
> require new SSA names to be created.  This patch makes sure that we
> don't try to create new SSA names when we're not yet in SSA form.
>
> Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi.
> OK to install?

Please use

 if (gimple_in_ssa_p (cfun))
  res = make_ssa_name (type);
 else
  res = create_tmp_reg (type);

Ok with that change.

Thanks,
Richard.

> Thanks,
> Richard
>
>
> gcc/
>         * gimple-match-head.c (maybe_push_res_to_seq): Don't attempt
>         to create new SSA names if not in SSA form.
>
> diff --git a/gcc/gimple-match-head.c b/gcc/gimple-match-head.c
> index 8f72919..1345cf9 100644
> --- a/gcc/gimple-match-head.c
> +++ b/gcc/gimple-match-head.c
> @@ -331,7 +331,11 @@ maybe_push_res_to_seq (code_helper rcode, tree type, tree *ops,
>               && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (ops[2])))
>         return NULL_TREE;
>        if (!res)
> -       res = make_ssa_name (type);
> +       {
> +         if (!gimple_in_ssa_p (cfun))
> +           return NULL_TREE;
> +         res = make_ssa_name (type);
> +       }
>        maybe_build_generic_op (rcode, type, &ops[0], ops[1], ops[2]);
>        gimple *new_stmt = gimple_build_assign (res, rcode,
>                                              ops[0], ops[1], ops[2]);
> @@ -361,7 +365,11 @@ maybe_push_res_to_seq (code_helper rcode, tree type, tree *ops,
>         }
>        gcc_assert (nargs != 0);
>        if (!res)
> -       res = make_ssa_name (type);
> +       {
> +         if (!gimple_in_ssa_p (cfun))
> +           return NULL_TREE;
> +         res = make_ssa_name (type);
> +       }
>        gimple *new_stmt = gimple_build_call (decl, nargs, ops[0], ops[1], ops[2]);
>        gimple_call_set_lhs (new_stmt, res);
>        gimple_seq_add_stmt_without_update (seq, new_stmt);
>

Fold comparisons between sqrt and zero

[Richard Sandiford]

Richard Biener <richard.guenther@gmail.com> writes:
> On Mon, Oct 26, 2015 at 10:36 AM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> An upcoming patch adds a fold from hypot(x,x) to fabs(x)*sqrt(2).
>> This is unusual in that it could trigger in the gimplifier but would
>> require new SSA names to be created.  This patch makes sure that we
>> don't try to create new SSA names when we're not yet in SSA form.
>>
>> Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi.
>> OK to install?
>
> Please use
>
>  if (gimple_in_ssa_p (cfun))
>   res = make_ssa_name (type);
>  else
>   res = create_tmp_reg (type);
>
> Ok with that change.

Thanks, committed in that form.  I retested the series on top of it and
the create_tmp_reg causes the later signbit patch (not yet committed)
to regress on:

  signbit(sqrt(x))

which is always 0 for -ffast-math.  The signbit fold first converts it to:

  sqrt(x) < 0

and whether we realise that this is false depends on a race between two
folders: the sqrt comparison folder, which wants to convert it to:

  x < 0*0

and the generic tree_expr_nonnegative_p rule for ... < 0, which would
give the hoped-for 0.

The sqrt code already handles comparisons with negative values specially,
so this patch simply extends that idea to comparisons with zero.

I don't really like patches like this since they'll probably help no
real code, but still...

Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi.
OK to install?

Thanks,
Richard


gcc/
	* match.pd: Handle sqrt(x) cmp 0 specially.

gcc/testsuite/
	* gcc.dg/torture/builtin-sqrt-cmp-1.c: New test.

diff --git a/gcc/match.pd b/gcc/match.pd
index 26491d2..b8e6b46 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1973,6 +1973,25 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 	{ constant_boolean_node (true, type); })
        /* sqrt(x) > y is the same as x >= 0, if y is negative.  */
        (ge @0 { build_real (TREE_TYPE (@0), dconst0); })))
+     (if (real_equal (TREE_REAL_CST_PTR (@1), &dconst0))
+      (switch
+       /* sqrt(x) < 0 is always false.  */
+       (if (cmp == LT_EXPR)
+	{ constant_boolean_node (false, type); })
+       /* sqrt(x) >= 0 is always true if we don't care about NaNs.  */
+       (if (cmp == GE_EXPR && !HONOR_NANS (@0))
+	{ constant_boolean_node (true, type); })
+       /* sqrt(x) <= 0 -> x == 0.  */
+       (if (cmp == LE_EXPR)
+	(eq @0 @1))
+       /* Otherwise sqrt(x) cmp 0 -> x cmp 0.  Here cmp can be >=, >,
+          == or !=.  In the last case:
+
+	    (sqrt(x) != 0) == (NaN != 0) == true == (x != 0)
+
+	  if x is negative or NaN.  Due to -funsafe-math-optimizations,
+	  the results for other x follow from natural arithmetic.  */
+       (cmp @0 @1)))
      (if (cmp == GT_EXPR || cmp == GE_EXPR)
       (with
        {
diff --git a/gcc/testsuite/gcc.dg/torture/builtin-sqrt-cmp-1.c b/gcc/testsuite/gcc.dg/torture/builtin-sqrt-cmp-1.c
new file mode 100644
index 0000000..3f4a708
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/builtin-sqrt-cmp-1.c
@@ -0,0 +1,53 @@
+/* { dg-do link } */
+/* { dg-options "-ffast-math" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
+
+extern double sqrt (double);
+extern float sqrtf (float);
+extern long double sqrtl (long double);
+
+/* All references to link_error should go away at compile-time.  */
+extern void link_error (void);
+
+#define TEST_ONE(SUFFIX, TYPE)			\
+  void __attribute__ ((noinline, noclone))	\
+  test##SUFFIX (TYPE f, int *res)		\
+  {						\
+    TYPE sqrt_res = sqrt##SUFFIX (f);		\
+    res[0] = sqrt_res < 0;			\
+    if (res[0])					\
+      link_error ();				\
+    res[1] = sqrt_res <= 0;			\
+    if (res[1] != (f == 0))			\
+      link_error ();				\
+    res[2] = (sqrt_res == 0);			\
+    if (res[2] != (f == 0))			\
+      link_error ();				\
+    res[3] = (sqrt_res != 0);			\
+    if (res[3] != (f != 0))			\
+      link_error ();				\
+    res[4] = (sqrt_res > 0);			\
+    if (res[4] != (f > 0))			\
+      link_error ();				\
+    res[5] = (sqrt_res >= 0);			\
+    if (!res[5])				\
+      link_error ();				\
+  }
+
+volatile float f;
+volatile double d;
+volatile long double ld;
+
+TEST_ONE (f, float)
+TEST_ONE (, double)
+TEST_ONE (l, long double)
+
+int
+main ()
+{
+  int res[6];
+  testf (f, res);
+  test (d, res);
+  testl (ld, res);
+  return 0;
+}

Re: Fold comparisons between sqrt and zero

[Richard Biener]

On Tue, Oct 27, 2015 at 10:21 AM, Richard Sandiford
<richard.sandiford@arm.com> wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>> On Mon, Oct 26, 2015 at 10:36 AM, Richard Sandiford
>> <rdsandiford@googlemail.com> wrote:
>>> An upcoming patch adds a fold from hypot(x,x) to fabs(x)*sqrt(2).
>>> This is unusual in that it could trigger in the gimplifier but would
>>> require new SSA names to be created.  This patch makes sure that we
>>> don't try to create new SSA names when we're not yet in SSA form.
>>>
>>> Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi.
>>> OK to install?
>>
>> Please use
>>
>>  if (gimple_in_ssa_p (cfun))
>>   res = make_ssa_name (type);
>>  else
>>   res = create_tmp_reg (type);
>>
>> Ok with that change.
>
> Thanks, committed in that form.  I retested the series on top of it and
> the create_tmp_reg causes the later signbit patch (not yet committed)
> to regress on:
>
>   signbit(sqrt(x))
>
> which is always 0 for -ffast-math.  The signbit fold first converts it to:
>
>   sqrt(x) < 0
>
> and whether we realise that this is false depends on a race between two
> folders: the sqrt comparison folder, which wants to convert it to:
>
>   x < 0*0
>
> and the generic tree_expr_nonnegative_p rule for ... < 0, which would
> give the hoped-for 0.
>
> The sqrt code already handles comparisons with negative values specially,
> so this patch simply extends that idea to comparisons with zero.
>
> I don't really like patches like this since they'll probably help no
> real code, but still...
>
> Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi.
> OK to install?

Ok.

Thanks,
Richard.

> Thanks,
> Richard
>
>
> gcc/
>         * match.pd: Handle sqrt(x) cmp 0 specially.
>
> gcc/testsuite/
>         * gcc.dg/torture/builtin-sqrt-cmp-1.c: New test.
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 26491d2..b8e6b46 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -1973,6 +1973,25 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>         { constant_boolean_node (true, type); })
>         /* sqrt(x) > y is the same as x >= 0, if y is negative.  */
>         (ge @0 { build_real (TREE_TYPE (@0), dconst0); })))
> +     (if (real_equal (TREE_REAL_CST_PTR (@1), &dconst0))
> +      (switch
> +       /* sqrt(x) < 0 is always false.  */
> +       (if (cmp == LT_EXPR)
> +       { constant_boolean_node (false, type); })
> +       /* sqrt(x) >= 0 is always true if we don't care about NaNs.  */
> +       (if (cmp == GE_EXPR && !HONOR_NANS (@0))
> +       { constant_boolean_node (true, type); })
> +       /* sqrt(x) <= 0 -> x == 0.  */
> +       (if (cmp == LE_EXPR)
> +       (eq @0 @1))
> +       /* Otherwise sqrt(x) cmp 0 -> x cmp 0.  Here cmp can be >=, >,
> +          == or !=.  In the last case:
> +
> +           (sqrt(x) != 0) == (NaN != 0) == true == (x != 0)
> +
> +         if x is negative or NaN.  Due to -funsafe-math-optimizations,
> +         the results for other x follow from natural arithmetic.  */
> +       (cmp @0 @1)))
>       (if (cmp == GT_EXPR || cmp == GE_EXPR)
>        (with
>         {
> diff --git a/gcc/testsuite/gcc.dg/torture/builtin-sqrt-cmp-1.c b/gcc/testsuite/gcc.dg/torture/builtin-sqrt-cmp-1.c
> new file mode 100644
> index 0000000..3f4a708
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/builtin-sqrt-cmp-1.c
> @@ -0,0 +1,53 @@
> +/* { dg-do link } */
> +/* { dg-options "-ffast-math" } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
> +
> +extern double sqrt (double);
> +extern float sqrtf (float);
> +extern long double sqrtl (long double);
> +
> +/* All references to link_error should go away at compile-time.  */
> +extern void link_error (void);
> +
> +#define TEST_ONE(SUFFIX, TYPE)                 \
> +  void __attribute__ ((noinline, noclone))     \
> +  test##SUFFIX (TYPE f, int *res)              \
> +  {                                            \
> +    TYPE sqrt_res = sqrt##SUFFIX (f);          \
> +    res[0] = sqrt_res < 0;                     \
> +    if (res[0])                                        \
> +      link_error ();                           \
> +    res[1] = sqrt_res <= 0;                    \
> +    if (res[1] != (f == 0))                    \
> +      link_error ();                           \
> +    res[2] = (sqrt_res == 0);                  \
> +    if (res[2] != (f == 0))                    \
> +      link_error ();                           \
> +    res[3] = (sqrt_res != 0);                  \
> +    if (res[3] != (f != 0))                    \
> +      link_error ();                           \
> +    res[4] = (sqrt_res > 0);                   \
> +    if (res[4] != (f > 0))                     \
> +      link_error ();                           \
> +    res[5] = (sqrt_res >= 0);                  \
> +    if (!res[5])                               \
> +      link_error ();                           \
> +  }
> +
> +volatile float f;
> +volatile double d;
> +volatile long double ld;
> +
> +TEST_ONE (f, float)
> +TEST_ONE (, double)
> +TEST_ONE (l, long double)
> +
> +int
> +main ()
> +{
> +  int res[6];
> +  testf (f, res);
> +  test (d, res);
> +  testl (ld, res);
> +  return 0;
> +}
>