PATCH: PR middle-end/33007: builtin lround doesn't work

PATCH: PR middle-end/33007: builtin lround doesn't work

[H.J. Lu]

fold_fixed_mathfn may turn builtin lround into FIX_TRUNC_EXPR. But
expand_builtin_int_roundingfn_2 isn't prepared to deal with it. This
patch checks return from expand_expr and handle FIX_TRUNC_EXPR.


H.J.
-----
gcc/

2007-05-13  H.J. Lu  <hongjiu.lu@intel.com>

	PR middle-end/33007
	* builtins.c (expand_builtin_int_roundingfn_2): Handle NOP_EXPR
	and FIX_TRUNC_EXPR return from expand_expr.

gcc/testsuite/

2007-05-13  H.J. Lu  <hongjiu.lu@intel.com>

	PR middle-end/33007
	* gcc.dg/torture/builtin-rounding-2.c: New.

--- gcc/builtins.c.round	2007-07-29 13:18:40.000000000 -0700
+++ gcc/builtins.c	2007-08-07 09:58:46.000000000 -0700
@@ -2689,6 +2689,16 @@ expand_builtin_int_roundingfn_2 (tree ex
 
   start_sequence ();
 
+  if (TREE_CODE (exp) == NOP_EXPR)
+    switch (TREE_CODE (TREE_OPERAND (exp, 0)))
+      {
+      case FIX_TRUNC_EXPR:
+	builtin_optab = sfixtrunc_optab;
+	break;
+      default:
+	gcc_unreachable ();
+      }
+
   if (expand_sfix_optab (target, op0, builtin_optab))
     {
       /* Output the entire sequence.  */
@@ -2698,6 +2708,8 @@ expand_builtin_int_roundingfn_2 (tree ex
       return target;
     }
 
+  gcc_assert (TREE_CODE (exp) == CALL_EXPR);
+
   /* If we were unable to expand via the builtin, stop the sequence
      (without outputting the insns) and call to the library function
      with the stabilized argument list.  */
--- gcc/testsuite/gcc.dg/torture/builtin-rounding-2.c.round	2007-08-07 10:07:05.000000000 -0700
+++ gcc/testsuite/gcc.dg/torture/builtin-rounding-2.c	2007-08-07 10:35:40.000000000 -0700
@@ -0,0 +1,169 @@
+/* Copyright (C) 2007 Free Software Foundation.
+
+   Check that integer folding of the rounding math functions doesn't
+   break anything and produces the expected results.  */
+
+/* { dg-do run } */
+/* { dg-options "-ffast-math -ftrapping-math" } */
+
+#define TEST(i)		\
+  if (test1 (i) != i)	\
+    abort ();		\
+			\
+  if (test2 (i) != i)	\
+    abort ();		\
+			\
+  if (test3 (i) != i)	\
+    abort ();		\
+			\
+  if (test4 (i) != i)	\
+    abort ();		\
+			\
+  if (test5 (i) != i)	\
+    abort ();		\
+			\
+  if (test6 (i) != i)	\
+    abort ();		\
+			\
+  if (test7 (i) != i)	\
+    abort ();		\
+			\
+  if (test8 (i) != i)	\
+    abort ();		\
+			\
+  if (test9 (i) != i)	\
+    abort ();		\
+			\
+  if (test10 (i) != i)	\
+    abort ();		\
+			\
+  if (test11 (i) != i)	\
+    abort ();		\
+			\
+  if (test12 (i) != i)	\
+    abort ();
+  
+extern void abort();
+
+long
+__attribute__ ((__noinline__))
+test1 (short i)
+{
+  float x;
+  x = i;
+  return __builtin_lround (x);
+}
+
+long
+__attribute__ ((__noinline__))
+test2 (int i)
+{
+  float x;
+  x = i;
+  return __builtin_lround (x);
+}
+
+long
+__attribute__ ((__noinline__))
+test3 (long long i)
+{
+  float x;
+  x = i;
+  return __builtin_lround (x);
+}
+
+long
+__attribute__ ((__noinline__))
+test4 (short i)
+{
+  double x;
+  x = i;
+  return __builtin_lround (x);
+}
+
+long
+__attribute__ ((__noinline__))
+test5 (int i)
+{
+  double x;
+  x = i;
+  return __builtin_lround (x);
+}
+
+long
+__attribute__ ((__noinline__))
+test6 (long long i)
+{
+  double x;
+  x = i;
+  return __builtin_lround (x);
+}
+
+long long
+__attribute__ ((__noinline__))
+test7 (short i)
+{
+  float x;
+  x = i;
+  return __builtin_llround (x);
+}
+
+long long
+__attribute__ ((__noinline__))
+test8 (int i)
+{
+  float x;
+  x = i;
+  return __builtin_llround (x);
+}
+
+long long
+__attribute__ ((__noinline__))
+test9 (long long i)
+{
+  float x;
+  x = i;
+  return __builtin_llround (x);
+}
+
+long long
+__attribute__ ((__noinline__))
+test10 (short i)
+{
+  double x;
+  x = i;
+  return __builtin_llround (x);
+}
+
+long long
+__attribute__ ((__noinline__))
+test11 (int i)
+{
+  double x;
+  x = i;
+  return __builtin_llround (x);
+}
+
+long long
+__attribute__ ((__noinline__))
+test12 (long long i)
+{
+  double x;
+  x = i;
+  return __builtin_llround (x);
+}
+
+int
+main (void)
+{
+  TEST (0);
+  TEST (1);
+  TEST (2);
+  TEST (3);
+  TEST (6);
+  TEST (-1);
+  TEST (-2);
+  TEST (-3);
+  TEST (-6);
+  return 0;
+}

Re: PATCH: PR middle-end/33007: builtin lround doesn't work

[Richard Guenther]

On 8/7/07, H.J. Lu <hjl@lucon.org> wrote:
> fold_fixed_mathfn may turn builtin lround into FIX_TRUNC_EXPR. But
> expand_builtin_int_roundingfn_2 isn't prepared to deal with it. This
> patch checks return from expand_expr and handle FIX_TRUNC_EXPR.
>

This is not the right way to fix this really.  Instead this is the
common problem
that build_call_expr folds the built expression.  Instead for

  /* Wrap the computation of the argument in a SAVE_EXPR, as we may
     need to expand the argument again.  This way, we will not perform
     side-effects more the once.  */
  narg = builtin_save_expr (arg);
  if (narg != arg)
    {
      arg = narg;
      exp = build_call_expr (fndecl, 1, arg);
    }

we should re-build the call expression _without_ folding it.  (Or
rather we can fix
the place that created the call without folding it - that is, fold
during TER).  See
for example expand_builtin_cexpi or others which make sure not to re-fold the
call expression.

Richard.

Re: PATCH: PR middle-end/33007: builtin lround doesn't work

[H.J. Lu]

On Mon, Aug 20, 2007 at 10:32:24AM +0200, Richard Guenther wrote:
> On 8/7/07, H.J. Lu <hjl@lucon.org> wrote:
> > fold_fixed_mathfn may turn builtin lround into FIX_TRUNC_EXPR. But
> > expand_builtin_int_roundingfn_2 isn't prepared to deal with it. This
> > patch checks return from expand_expr and handle FIX_TRUNC_EXPR.
> >
> 
> This is not the right way to fix this really.  Instead this is the
> common problem
> that build_call_expr folds the built expression.  Instead for
> 
>   /* Wrap the computation of the argument in a SAVE_EXPR, as we may
>      need to expand the argument again.  This way, we will not perform
>      side-effects more the once.  */
>   narg = builtin_save_expr (arg);
>   if (narg != arg)
>     {
>       arg = narg;
>       exp = build_call_expr (fndecl, 1, arg);
>     }
> 
> we should re-build the call expression _without_ folding it.  (Or
> rather we can fix
> the place that created the call without folding it - that is, fold
> during TER).  See
> for example expand_builtin_cexpi or others which make sure not to re-fold the
> call expression.
> 

The testcase is

long
foo (int i)
{
  float x;
  x = i;
  return __builtin_lroundf (x);
}

When fold_fixed_mathfn is called the first time, integer_valued_real_p
returns false since it doesn't enough info. When fold_fixed_mathfn is
called after expand_builtin_int_roundingfn_2, integer_valued_real_p
returns true, fold_fixed_mathfn can perform optimization. Will your
suggestion lead to missed optimization?


H.J.

Re: PATCH: PR middle-end/33007: builtin lround doesn't work

[Richard Guenther]

On 8/20/07, H.J. Lu <hjl@lucon.org> wrote:
> On Mon, Aug 20, 2007 at 10:32:24AM +0200, Richard Guenther wrote:
> > On 8/7/07, H.J. Lu <hjl@lucon.org> wrote:
> > > fold_fixed_mathfn may turn builtin lround into FIX_TRUNC_EXPR. But
> > > expand_builtin_int_roundingfn_2 isn't prepared to deal with it. This
> > > patch checks return from expand_expr and handle FIX_TRUNC_EXPR.
> > >
> >
> > This is not the right way to fix this really.  Instead this is the
> > common problem
> > that build_call_expr folds the built expression.  Instead for
> >
> >   /* Wrap the computation of the argument in a SAVE_EXPR, as we may
> >      need to expand the argument again.  This way, we will not perform
> >      side-effects more the once.  */
> >   narg = builtin_save_expr (arg);
> >   if (narg != arg)
> >     {
> >       arg = narg;
> >       exp = build_call_expr (fndecl, 1, arg);
> >     }
> >
> > we should re-build the call expression _without_ folding it.  (Or
> > rather we can fix
> > the place that created the call without folding it - that is, fold
> > during TER).  See
> > for example expand_builtin_cexpi or others which make sure not to re-fold the
> > call expression.
> >
>
> The testcase is
>
> long
> foo (int i)
> {
>   float x;
>   x = i;
>   return __builtin_lroundf (x);
> }
>
> When fold_fixed_mathfn is called the first time, integer_valued_real_p
> returns false since it doesn't enough info. When fold_fixed_mathfn is
> called after expand_builtin_int_roundingfn_2, integer_valued_real_p
> returns true, fold_fixed_mathfn can perform optimization. Will your
> suggestion lead to missed optimization?

Yes.  But only because we expand unfolded expressions.

Richard.

Re: PATCH: PR middle-end/33007: builtin lround doesn't work

[H.J. Lu]

On Tue, Aug 21, 2007 at 10:21:26AM +0200, Richard Guenther wrote:
> On 8/20/07, H.J. Lu <hjl@lucon.org> wrote:
> > On Mon, Aug 20, 2007 at 10:32:24AM +0200, Richard Guenther wrote:
> > > On 8/7/07, H.J. Lu <hjl@lucon.org> wrote:
> > > > fold_fixed_mathfn may turn builtin lround into FIX_TRUNC_EXPR. But
> > > > expand_builtin_int_roundingfn_2 isn't prepared to deal with it. This
> > > > patch checks return from expand_expr and handle FIX_TRUNC_EXPR.
> > > >
> > >
> > > This is not the right way to fix this really.  Instead this is the
> > > common problem
> > > that build_call_expr folds the built expression.  Instead for
> > >
> > >   /* Wrap the computation of the argument in a SAVE_EXPR, as we may
> > >      need to expand the argument again.  This way, we will not perform
> > >      side-effects more the once.  */
> > >   narg = builtin_save_expr (arg);
> > >   if (narg != arg)
> > >     {
> > >       arg = narg;
> > >       exp = build_call_expr (fndecl, 1, arg);
> > >     }
> > >
> > > we should re-build the call expression _without_ folding it.  (Or
> > > rather we can fix
> > > the place that created the call without folding it - that is, fold
> > > during TER).  See
> > > for example expand_builtin_cexpi or others which make sure not to re-fold the
> > > call expression.
> > >
> >
> > The testcase is
> >
> > long
> > foo (int i)
> > {
> >   float x;
> >   x = i;
> >   return __builtin_lroundf (x);
> > }
> >
> > When fold_fixed_mathfn is called the first time, integer_valued_real_p
> > returns false since it doesn't enough info. When fold_fixed_mathfn is
> > called after expand_builtin_int_roundingfn_2, integer_valued_real_p
> > returns true, fold_fixed_mathfn can perform optimization. Will your
> > suggestion lead to missed optimization?
> 
> Yes.  But only because we expand unfolded expressions.
> 

Do you have a suggestion to geneate the same assembly code for

long
foo (int i)
{
  float x;
  x = i;
  return __builtin_lroundf (x);
}

and

long
foo (int i)
{
  return __builtin_lroundf (i);
}


H.J.

Re: PATCH: PR middle-end/33007: builtin lround doesn't work

[Richard Guenther]

On 8/21/07, H.J. Lu <hjl@lucon.org> wrote:
> On Tue, Aug 21, 2007 at 10:21:26AM +0200, Richard Guenther wrote:
> > On 8/20/07, H.J. Lu <hjl@lucon.org> wrote:
> > > On Mon, Aug 20, 2007 at 10:32:24AM +0200, Richard Guenther wrote:
> > > > On 8/7/07, H.J. Lu <hjl@lucon.org> wrote:
> > > > > fold_fixed_mathfn may turn builtin lround into FIX_TRUNC_EXPR. But
> > > > > expand_builtin_int_roundingfn_2 isn't prepared to deal with it. This
> > > > > patch checks return from expand_expr and handle FIX_TRUNC_EXPR.
> > > > >
> > > >
> > > > This is not the right way to fix this really.  Instead this is the
> > > > common problem
> > > > that build_call_expr folds the built expression.  Instead for
> > > >
> > > >   /* Wrap the computation of the argument in a SAVE_EXPR, as we may
> > > >      need to expand the argument again.  This way, we will not perform
> > > >      side-effects more the once.  */
> > > >   narg = builtin_save_expr (arg);
> > > >   if (narg != arg)
> > > >     {
> > > >       arg = narg;
> > > >       exp = build_call_expr (fndecl, 1, arg);
> > > >     }
> > > >
> > > > we should re-build the call expression _without_ folding it.  (Or
> > > > rather we can fix
> > > > the place that created the call without folding it - that is, fold
> > > > during TER).  See
> > > > for example expand_builtin_cexpi or others which make sure not to re-fold the
> > > > call expression.
> > > >
> > >
> > > The testcase is
> > >
> > > long
> > > foo (int i)
> > > {
> > >   float x;
> > >   x = i;
> > >   return __builtin_lroundf (x);
> > > }
> > >
> > > When fold_fixed_mathfn is called the first time, integer_valued_real_p
> > > returns false since it doesn't enough info. When fold_fixed_mathfn is
> > > called after expand_builtin_int_roundingfn_2, integer_valued_real_p
> > > returns true, fold_fixed_mathfn can perform optimization. Will your
> > > suggestion lead to missed optimization?
> >
> > Yes.  But only because we expand unfolded expressions.
> >
>
> Do you have a suggestion to geneate the same assembly code for
>
> long
> foo (int i)
> {
>   float x;
>   x = i;
>   return __builtin_lroundf (x);
> }
>
> and
>
> long
> foo (int i)
> {
>   return __builtin_lroundf (i);
> }

A tree combiner.  That is, extend value-numbering to do this.

Richard.

Re: PATCH: PR middle-end/33007: builtin lround doesn't work

[H.J. Lu]

On Mon, Aug 20, 2007 at 10:32:24AM +0200, Richard Guenther wrote:
> On 8/7/07, H.J. Lu <hjl@lucon.org> wrote:
> > fold_fixed_mathfn may turn builtin lround into FIX_TRUNC_EXPR. But
> > expand_builtin_int_roundingfn_2 isn't prepared to deal with it. This
> > patch checks return from expand_expr and handle FIX_TRUNC_EXPR.
> >
> 
> This is not the right way to fix this really.  Instead this is the
> common problem
> that build_call_expr folds the built expression.  Instead for
> 
>   /* Wrap the computation of the argument in a SAVE_EXPR, as we may
>      need to expand the argument again.  This way, we will not perform
>      side-effects more the once.  */
>   narg = builtin_save_expr (arg);
>   if (narg != arg)
>     {
>       arg = narg;
>       exp = build_call_expr (fndecl, 1, arg);
>     }
> 
> we should re-build the call expression _without_ folding it.  (Or
> rather we can fix
> the place that created the call without folding it - that is, fold
> during TER).  See
> for example expand_builtin_cexpi or others which make sure not to re-fold the
> call expression.
> 

I am not familiar with middle end. Can some middle-end people look
into it?

Thanks.


H.J.

Re: PATCH: PR middle-end/33007: builtin lround doesn't work

[Richard Guenther]

On 8/21/07, H.J. Lu <hjl@lucon.org> wrote:
> On Mon, Aug 20, 2007 at 10:32:24AM +0200, Richard Guenther wrote:
> > On 8/7/07, H.J. Lu <hjl@lucon.org> wrote:
> > > fold_fixed_mathfn may turn builtin lround into FIX_TRUNC_EXPR. But
> > > expand_builtin_int_roundingfn_2 isn't prepared to deal with it. This
> > > patch checks return from expand_expr and handle FIX_TRUNC_EXPR.
> > >
> >
> > This is not the right way to fix this really.  Instead this is the
> > common problem
> > that build_call_expr folds the built expression.  Instead for
> >
> >   /* Wrap the computation of the argument in a SAVE_EXPR, as we may
> >      need to expand the argument again.  This way, we will not perform
> >      side-effects more the once.  */
> >   narg = builtin_save_expr (arg);
> >   if (narg != arg)
> >     {
> >       arg = narg;
> >       exp = build_call_expr (fndecl, 1, arg);
> >     }
> >
> > we should re-build the call expression _without_ folding it.  (Or
> > rather we can fix
> > the place that created the call without folding it - that is, fold
> > during TER).  See
> > for example expand_builtin_cexpi or others which make sure not to re-fold the
> > call expression.
> >
>
> I am not familiar with middle end. Can some middle-end people look
> into it?

Sure.

Richard.