diff mbox

Fix PR79460

Message ID alpine.LSU.2.20.1702141547390.6076@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Feb. 14, 2017, 2:48 p.m. UTC
The following enables final value replacement for floating point
expressions if -funsafe-math-optimizations is set (that's the
flag the reassoc pass controls similar transforms on).

Bootstrapped / tested on x86_64-unknown-linux-gnu, queued for GCC 8.

Richard.

2017-02-14  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/79460
	* tree-scalar-evolution.c (final_value_replacement_loop): Also
	allow final value replacement of floating point expressions.

	* gcc.dg/tree-ssa/sccp-3.c: New testcase.

Comments

Jakub Jelinek Feb. 14, 2017, 2:55 p.m. UTC | #1
On Tue, Feb 14, 2017 at 03:48:38PM +0100, Richard Biener wrote:
> 2017-02-14  Richard Biener  <rguenther@suse.de>
> 
> 	PR tree-optimization/79460
> 	* tree-scalar-evolution.c (final_value_replacement_loop): Also
> 	allow final value replacement of floating point expressions.
> 
> 	* gcc.dg/tree-ssa/sccp-3.c: New testcase.
> 
> Index: gcc/tree-scalar-evolution.c
> ===================================================================
> --- gcc/tree-scalar-evolution.c	(revision 245417)
> +++ gcc/tree-scalar-evolution.c	(working copy)
> @@ -3718,8 +3718,10 @@ final_value_replacement_loop (struct loo
>  	  continue;
>  	}
>  
> -      if (!POINTER_TYPE_P (TREE_TYPE (def))
> -	  && !INTEGRAL_TYPE_P (TREE_TYPE (def)))
> +      if (! (POINTER_TYPE_P (TREE_TYPE (def))
> +	     || INTEGRAL_TYPE_P (TREE_TYPE (def))
> +	     || (FLOAT_TYPE_P (TREE_TYPE (def))
> +		 && flag_unsafe_math_optimizations)))

I think Segher mentioned in the PR that this should be better
flag_associative_math.  Also, FLOAT_TYPE_P stands not just for
SCALAR_FLOAT_TYPE_P, but for COMPLEX_TYPE and VECTOR_TYPE thereof as well.
Does SCEV handle complex and vector types well (it would be really nice
if it could of course, but then we should use ANY_INTEGRAL_TYPE_P as
well to also handle complex and vector integers)?

	Jakub
Richard Biener Feb. 14, 2017, 3:07 p.m. UTC | #2
On Tue, 14 Feb 2017, Jakub Jelinek wrote:

> On Tue, Feb 14, 2017 at 03:48:38PM +0100, Richard Biener wrote:
> > 2017-02-14  Richard Biener  <rguenther@suse.de>
> > 
> > 	PR tree-optimization/79460
> > 	* tree-scalar-evolution.c (final_value_replacement_loop): Also
> > 	allow final value replacement of floating point expressions.
> > 
> > 	* gcc.dg/tree-ssa/sccp-3.c: New testcase.
> > 
> > Index: gcc/tree-scalar-evolution.c
> > ===================================================================
> > --- gcc/tree-scalar-evolution.c	(revision 245417)
> > +++ gcc/tree-scalar-evolution.c	(working copy)
> > @@ -3718,8 +3718,10 @@ final_value_replacement_loop (struct loo
> >  	  continue;
> >  	}
> >  
> > -      if (!POINTER_TYPE_P (TREE_TYPE (def))
> > -	  && !INTEGRAL_TYPE_P (TREE_TYPE (def)))
> > +      if (! (POINTER_TYPE_P (TREE_TYPE (def))
> > +	     || INTEGRAL_TYPE_P (TREE_TYPE (def))
> > +	     || (FLOAT_TYPE_P (TREE_TYPE (def))
> > +		 && flag_unsafe_math_optimizations)))
> 
> I think Segher mentioned in the PR that this should be better
> flag_associative_math.

We don't associate anything though.

Technically we could, for constant niter, fully simulate
the rounding effects (flag_rounding_math would need checking then).

Given that reassoc transforms x + x + x + x -> 4 * x with just
-funsafe-math-optimizations using that flag is at least consistent.

I think flag_associative_math wasn't meant to change the number of
rounding steps.  flag_fp_contract_mode controls this IMHO, but we
have the reassoc precedent...

>  Also, FLOAT_TYPE_P stands not just for
> SCALAR_FLOAT_TYPE_P, but for COMPLEX_TYPE and VECTOR_TYPE thereof as well.
> Does SCEV handle complex and vector types well (it would be really nice
> if it could of course, but then we should use ANY_INTEGRAL_TYPE_P as
> well to also handle complex and vector integers)?

SCEV should gate out types it doesn't handle itself -- it already
gates out VECTOR_TYPE:

static tree
analyze_scalar_evolution_1 (struct loop *loop, tree var, tree res)
{
  tree type = TREE_TYPE (var);
  gimple *def;
  basic_block bb;
  struct loop *def_loop;

  if (loop == NULL || TREE_CODE (type) == VECTOR_TYPE)
    return chrec_dont_know;

and given that it special-cases only REAL_CST, FIXED_CST and INTEGER_CST
in get_scalar_evolution I doubt it handles anything else reasonably.

But as I said, it's SCEVs job to reject them, not users of the API.
I see no reason to not allow vectors for example (it's just the
code might be not ready and uses wrong tree building interfaces).

Richard.


> 	Jakub
> 
>
Bin.Cheng Feb. 14, 2017, 3:19 p.m. UTC | #3
On Tue, Feb 14, 2017 at 2:48 PM, Richard Biener <rguenther@suse.de> wrote:
>
> The following enables final value replacement for floating point
> expressions if -funsafe-math-optimizations is set (that's the
> flag the reassoc pass controls similar transforms on).
Looks to me it's kind of abusing of current implementation of SCEV for
floating point values.  I believe it's designed only with integral
type in mind, for example, we may need to reject float time when
tracking scev chain via type conversion.

Thanks,
bin
>
> Bootstrapped / tested on x86_64-unknown-linux-gnu, queued for GCC 8.
>
> Richard.
>
> 2017-02-14  Richard Biener  <rguenther@suse.de>
>
>         PR tree-optimization/79460
>         * tree-scalar-evolution.c (final_value_replacement_loop): Also
>         allow final value replacement of floating point expressions.
>
>         * gcc.dg/tree-ssa/sccp-3.c: New testcase.
>
> Index: gcc/tree-scalar-evolution.c
> ===================================================================
> --- gcc/tree-scalar-evolution.c (revision 245417)
> +++ gcc/tree-scalar-evolution.c (working copy)
> @@ -3718,8 +3718,10 @@ final_value_replacement_loop (struct loo
>           continue;
>         }
>
> -      if (!POINTER_TYPE_P (TREE_TYPE (def))
> -         && !INTEGRAL_TYPE_P (TREE_TYPE (def)))
> +      if (! (POINTER_TYPE_P (TREE_TYPE (def))
> +            || INTEGRAL_TYPE_P (TREE_TYPE (def))
> +            || (FLOAT_TYPE_P (TREE_TYPE (def))
> +                && flag_unsafe_math_optimizations)))
>         {
>           gsi_next (&psi);
>           continue;
> Index: gcc/testsuite/gcc.dg/tree-ssa/sccp-3.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/sccp-3.c      (nonexistent)
> +++ gcc/testsuite/gcc.dg/tree-ssa/sccp-3.c      (working copy)
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -funsafe-math-optimizations -fdump-tree-sccp" } */
> +
> +float f(float x[])
> +{
> +  float p = 1.0;
> +  for (int i = 0; i < 200; i++)
> +    p += 1;
> +  return p;
> +}
> +
> +/* { dg-final { scan-tree-dump "final value replacement.* = 2.01e\\+2;" "sccp" } } */
Richard Biener Feb. 14, 2017, 3:57 p.m. UTC | #4
On February 14, 2017 4:19:05 PM GMT+01:00, "Bin.Cheng" <amker.cheng@gmail.com> wrote:
>On Tue, Feb 14, 2017 at 2:48 PM, Richard Biener <rguenther@suse.de>
>wrote:
>>
>> The following enables final value replacement for floating point
>> expressions if -funsafe-math-optimizations is set (that's the
>> flag the reassoc pass controls similar transforms on).
>Looks to me it's kind of abusing of current implementation of SCEV for
>floating point values.  I believe it's designed only with integral
>type in mind, for example, we may need to reject float time when
>tracking scev chain via type conversion.

Note the vectorizer relies on SCEV itself here.

Richard.

>Thanks,
>bin
>>
>> Bootstrapped / tested on x86_64-unknown-linux-gnu, queued for GCC 8.
>>
>> Richard.
>>
>> 2017-02-14  Richard Biener  <rguenther@suse.de>
>>
>>         PR tree-optimization/79460
>>         * tree-scalar-evolution.c (final_value_replacement_loop):
>Also
>>         allow final value replacement of floating point expressions.
>>
>>         * gcc.dg/tree-ssa/sccp-3.c: New testcase.
>>
>> Index: gcc/tree-scalar-evolution.c
>> ===================================================================
>> --- gcc/tree-scalar-evolution.c (revision 245417)
>> +++ gcc/tree-scalar-evolution.c (working copy)
>> @@ -3718,8 +3718,10 @@ final_value_replacement_loop (struct loo
>>           continue;
>>         }
>>
>> -      if (!POINTER_TYPE_P (TREE_TYPE (def))
>> -         && !INTEGRAL_TYPE_P (TREE_TYPE (def)))
>> +      if (! (POINTER_TYPE_P (TREE_TYPE (def))
>> +            || INTEGRAL_TYPE_P (TREE_TYPE (def))
>> +            || (FLOAT_TYPE_P (TREE_TYPE (def))
>> +                && flag_unsafe_math_optimizations)))
>>         {
>>           gsi_next (&psi);
>>           continue;
>> Index: gcc/testsuite/gcc.dg/tree-ssa/sccp-3.c
>> ===================================================================
>> --- gcc/testsuite/gcc.dg/tree-ssa/sccp-3.c      (nonexistent)
>> +++ gcc/testsuite/gcc.dg/tree-ssa/sccp-3.c      (working copy)
>> @@ -0,0 +1,12 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -funsafe-math-optimizations -fdump-tree-sccp" }
>*/
>> +
>> +float f(float x[])
>> +{
>> +  float p = 1.0;
>> +  for (int i = 0; i < 200; i++)
>> +    p += 1;
>> +  return p;
>> +}
>> +
>> +/* { dg-final { scan-tree-dump "final value replacement.* =
>2.01e\\+2;" "sccp" } } */
diff mbox

Patch

Index: gcc/tree-scalar-evolution.c
===================================================================
--- gcc/tree-scalar-evolution.c	(revision 245417)
+++ gcc/tree-scalar-evolution.c	(working copy)
@@ -3718,8 +3718,10 @@  final_value_replacement_loop (struct loo
 	  continue;
 	}
 
-      if (!POINTER_TYPE_P (TREE_TYPE (def))
-	  && !INTEGRAL_TYPE_P (TREE_TYPE (def)))
+      if (! (POINTER_TYPE_P (TREE_TYPE (def))
+	     || INTEGRAL_TYPE_P (TREE_TYPE (def))
+	     || (FLOAT_TYPE_P (TREE_TYPE (def))
+		 && flag_unsafe_math_optimizations)))
 	{
 	  gsi_next (&psi);
 	  continue;
Index: gcc/testsuite/gcc.dg/tree-ssa/sccp-3.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/sccp-3.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/sccp-3.c	(working copy)
@@ -0,0 +1,12 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -funsafe-math-optimizations -fdump-tree-sccp" } */
+
+float f(float x[])
+{
+  float p = 1.0;
+  for (int i = 0; i < 200; i++)
+    p += 1;
+  return p;
+}
+
+/* { dg-final { scan-tree-dump "final value replacement.* = 2.01e\\+2;" "sccp" } } */