Tail calls and floating-point types (was Re: remove unneeded fold-converts)

[Richard Sandiford]

Nathan Sidwell <nathan@codesourcery.com> writes:
> Hi,
> I've installed this patch which replaces things like
> 	fold_convert (some_integral_type, integer_zero_node);
> with
> 	build_int_cst (some_integral_type, 0);

This bit...

> 	* tree-tailcall.c (tree_optimize_tail_calls_1): Likewise.

...breaks newlib builds of __exp10.c.  The problem is that the tailcall
pass is trying to apply the accumulator transform to all types, not just
integral types.  Thus if the function returns a floating-point type,
the effect of:

> Index: tree-tailcall.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/tree-tailcall.c,v
> retrieving revision 2.23
> diff -c -3 -p -r2.23 tree-tailcall.c
> *** tree-tailcall.c	6 Sep 2004 10:08:13 -0000	2.23
> --- tree-tailcall.c	6 Sep 2004 13:27:28 -0000
> *************** tree_optimize_tail_calls_1 (bool opt_tai
> *** 875,882 ****
>   	  add_referenced_tmp_var (tmp);
>   
>   	  phi = create_phi_node (tmp, first);
> ! 	  add_phi_arg (&phi, fold_convert (ret_type, integer_zero_node),
> ! 		       first->pred);
>   	  a_acc = PHI_RESULT (phi);
>   	}
>   
> --- 875,881 ----
>   	  add_referenced_tmp_var (tmp);
>   
>   	  phi = create_phi_node (tmp, first);
> ! 	  add_phi_arg (&phi, build_int_cst (ret_type, 0), first->pred);
>   	  a_acc = PHI_RESULT (phi);
>   	}
>   
> *************** tree_optimize_tail_calls_1 (bool opt_tai
> *** 888,895 ****
>   	  add_referenced_tmp_var (tmp);
>   
>   	  phi = create_phi_node (tmp, first);
> ! 	  add_phi_arg (&phi, fold_convert (ret_type, integer_one_node),
> ! 		       first->pred);
>   	  m_acc = PHI_RESULT (phi);
>   	}
>       }
> --- 887,893 ----
>   	  add_referenced_tmp_var (tmp);
>   
>   	  phi = create_phi_node (tmp, first);
> ! 	  add_phi_arg (&phi, build_int_cst (ret_type, 1), first->pred);
>   	  m_acc = PHI_RESULT (phi);
>   	}
>       }

is to build an INTEGER_CST with a floating-point type.  This causes an
assertion failure further down the line.

I'm not really sure whether the accumulator transforms are supposed to
handle floating-point types or not.  Compilation of __exp10.c succeeded
before the patch, with the multiplicative transformation applied,
but the additive transform clearly isn't safe for IEEE.

The attached testcase shows both problems.  At the moment, it fails
to compile because of the assertion failure, but if you revert the
patch above, it fails at execution time at -O1, -O2 and -Os.  Both
failures can be seen on i686-pc-linux-gnu.

FWIW, the attached patch makes the test pass by restricting the accumulator
transforms to integral return types.  It was bootstrapped & regression
tested on i686-pc-linux-gnu, but it's more for discussion than anything.
I suspect we'll want to add a flag_unsafe_math_optimizations or HONOR_*
check somewhere...

Richard


	* tree-tailcall.c (process_assignment): Only do accumulator
	optimisations if the function has an integral return type.

testsuite/
	* gcc.c-torture/execute/ieee/mzero6.c: New test.

Index: tree-tailcall.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-tailcall.c,v
retrieving revision 2.24
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r2.24 tree-tailcall.c
*** tree-tailcall.c	7 Sep 2004 10:22:44 -0000	2.24
--- tree-tailcall.c	9 Sep 2004 15:02:50 -0000
*************** process_assignment (tree ass, tree stmt,
*** 283,288 ****
--- 283,291 ----
    if (TREE_CODE_CLASS (code) != '2')
      return false;
  
+   if (!INTEGRAL_TYPE_P (TREE_TYPE (DECL_RESULT (current_function_decl))))
+     return false;
+ 
    /* We only handle the code like
  
       x = call ();
*** /dev/null	Fri Apr 23 00:21:55 2004
--- testsuite/gcc.c-torture/execute/ieee/mzero6.c	Thu Sep  9 16:01:51 2004
***************
*** 0 ****
--- 1,24 ----
+ /* Tail call optimizations would convert func() into the moral equivalent of:
+ 
+        double acc = 0.0;
+        for (int i = 0; i < n; i++)
+ 	 acc += d;
+        return acc;
+ 
+    which mishandles the case where 'd' is -0.  They also initialised 'acc'
+    to a zero int rather than a zero double.  */
+ 
+ double func (double d, int n)
+ {
+   if (n == 0)
+     return d;
+   else
+     return d + func (d, n - 1);
+ }
+ 
+ int main ()
+ {
+   if (__builtin_copysign (1.0, func (0.0 / -5.0, 10)) != -1.0)
+     abort ();
+   exit (0);
+ }