Patchwork Fix PR57980

login
register
mail settings
Submitter Marek Polacek
Date Aug. 9, 2013, 2:35 p.m.
Message ID <20130809143518.GH17022@redhat.com>
Download mbox | patch
Permalink /patch/266046/
State New
Headers show

Comments

Marek Polacek - Aug. 9, 2013, 2:35 p.m.
In this PR the problem was that when dealing with the gimple assign in
the tailcall optimization, we, when the rhs operand is of a vector
type, need to create -1 also of a vector type, but build_int_cst
doesn't create vectors (ICEs).  Instead, we should use build_minus_one_cst
because that can create even the VECTOR_TYPE constant (and, it can
create even REAL_TYPE/COMPLEX_TYPE), as suggested by Marc.

Regtested/bootstrapped on x86_64-linux, ok for trunk and 4.8?

2013-08-09  Marek Polacek  <polacek@redhat.com>
	    Marc Glisse  <marc.glisse@inria.fr>

	PR tree-optimization/57980
	* tree-tailcall.c (process_assignment): Call build_minus_one_cst
	when creating -1 constant.

	* gcc.dg/pr57980.c: New test.


	Marek
Richard Guenther - Aug. 9, 2013, 6:40 p.m.
Marek Polacek <polacek@redhat.com> wrote:
>In this PR the problem was that when dealing with the gimple assign in
>the tailcall optimization, we, when the rhs operand is of a vector
>type, need to create -1 also of a vector type, but build_int_cst
>doesn't create vectors (ICEs).  Instead, we should use
>build_minus_one_cst
>because that can create even the VECTOR_TYPE constant (and, it can
>create even REAL_TYPE/COMPLEX_TYPE), as suggested by Marc.
>
>Regtested/bootstrapped on x86_64-linux, ok for trunk and 4.8?

Ok. Double-check that this function exists on the branch please.

Thanks,
Richard.

>2013-08-09  Marek Polacek  <polacek@redhat.com>
>	    Marc Glisse  <marc.glisse@inria.fr>
>
>	PR tree-optimization/57980
>	* tree-tailcall.c (process_assignment): Call build_minus_one_cst
>	when creating -1 constant.
>
>	* gcc.dg/pr57980.c: New test.
>
>--- gcc/tree-tailcall.c.mp	2013-08-09 16:07:55.778616996 +0200
>+++ gcc/tree-tailcall.c	2013-08-09 16:08:00.068635823 +0200
>@@ -326,11 +326,7 @@ process_assignment (gimple stmt, gimple_
>       return true;
> 
>     case NEGATE_EXPR:
>-      if (FLOAT_TYPE_P (TREE_TYPE (op0)))
>-        *m = build_real (TREE_TYPE (op0), dconstm1);
>-      else
>-        *m = build_int_cst (TREE_TYPE (op0), -1);
>-
>+      *m = build_minus_one_cst (TREE_TYPE (op0));
>       *ass_var = dest;
>       return true;
> 
>@@ -339,11 +335,7 @@ process_assignment (gimple stmt, gimple_
>  *a = fold_build1 (NEGATE_EXPR, TREE_TYPE (non_ass_var), non_ass_var);
>       else
>         {
>-          if (FLOAT_TYPE_P (TREE_TYPE (non_ass_var)))
>-            *m = build_real (TREE_TYPE (non_ass_var), dconstm1);
>-          else
>-            *m = build_int_cst (TREE_TYPE (non_ass_var), -1);
>-
>+	  *m = build_minus_one_cst (TREE_TYPE (non_ass_var));
>  *a = fold_build1 (NEGATE_EXPR, TREE_TYPE (non_ass_var), non_ass_var);
>         }
> 
>--- gcc/testsuite/gcc.dg/pr57980.c.mp	2013-08-09 16:07:25.967485356
>+0200
>+++ gcc/testsuite/gcc.dg/pr57980.c	2013-08-09 16:07:17.967450526 +0200
>@@ -0,0 +1,19 @@
>+/* PR tree-optimization/57980 */
>+/* { dg-do compile } */
>+/* { dg-options "-O -foptimize-sibling-calls" } */
>+
>+typedef int V __attribute__ ((vector_size (sizeof (int))));
>+extern V f (void);
>+
>+V
>+bar (void)
>+{
>+  return -f ();
>+}
>+
>+V
>+foo (void)
>+{
>+  V v = { };
>+  return v - f ();
>+}
>
>	Marek
Marek Polacek - Aug. 12, 2013, 8:56 a.m.
On Fri, Aug 09, 2013 at 08:40:00PM +0200, Richard Biener wrote:
> Marek Polacek <polacek@redhat.com> wrote:
> >In this PR the problem was that when dealing with the gimple assign in
> >the tailcall optimization, we, when the rhs operand is of a vector
> >type, need to create -1 also of a vector type, but build_int_cst
> >doesn't create vectors (ICEs).  Instead, we should use
> >build_minus_one_cst
> >because that can create even the VECTOR_TYPE constant (and, it can
> >create even REAL_TYPE/COMPLEX_TYPE), as suggested by Marc.
> >
> >Regtested/bootstrapped on x86_64-linux, ok for trunk and 4.8?
> 
> Ok. Double-check that this function exists on the branch please.

It does not :(.  So not backporting to 4.8...

	Marek
Jakub Jelinek - Aug. 13, 2013, 10:24 a.m.
On Mon, Aug 12, 2013 at 10:56:44AM +0200, Marek Polacek wrote:
> On Fri, Aug 09, 2013 at 08:40:00PM +0200, Richard Biener wrote:
> > Marek Polacek <polacek@redhat.com> wrote:
> > >In this PR the problem was that when dealing with the gimple assign in
> > >the tailcall optimization, we, when the rhs operand is of a vector
> > >type, need to create -1 also of a vector type, but build_int_cst
> > >doesn't create vectors (ICEs).  Instead, we should use
> > >build_minus_one_cst
> > >because that can create even the VECTOR_TYPE constant (and, it can
> > >create even REAL_TYPE/COMPLEX_TYPE), as suggested by Marc.
> > >
> > >Regtested/bootstrapped on x86_64-linux, ok for trunk and 4.8?
> > 
> > Ok. Double-check that this function exists on the branch please.
> 
> It does not :(.  So not backporting to 4.8...

For 4.8/4.7, I'd say just change those
  else
    *m = build_int_cst (TREE_TYPE (...), -1);
into
  else if (INTEGRAL_TYPE_P (TREE_TYPE (...)))
    *m = build_int_cst (TREE_TYPE (...), -1);
  else
    return false;

	Jakub
Jakub Jelinek - Aug. 13, 2013, 10:27 a.m.
On Tue, Aug 13, 2013 at 12:24:59PM +0200, Jakub Jelinek wrote:
> On Mon, Aug 12, 2013 at 10:56:44AM +0200, Marek Polacek wrote:
> > On Fri, Aug 09, 2013 at 08:40:00PM +0200, Richard Biener wrote:
> > > Marek Polacek <polacek@redhat.com> wrote:
> > > >In this PR the problem was that when dealing with the gimple assign in
> > > >the tailcall optimization, we, when the rhs operand is of a vector
> > > >type, need to create -1 also of a vector type, but build_int_cst
> > > >doesn't create vectors (ICEs).  Instead, we should use
> > > >build_minus_one_cst
> > > >because that can create even the VECTOR_TYPE constant (and, it can
> > > >create even REAL_TYPE/COMPLEX_TYPE), as suggested by Marc.
> > > >
> > > >Regtested/bootstrapped on x86_64-linux, ok for trunk and 4.8?
> > > 
> > > Ok. Double-check that this function exists on the branch please.
> > 
> > It does not :(.  So not backporting to 4.8...
> 
> For 4.8/4.7, I'd say just change those
>   else
>     *m = build_int_cst (TREE_TYPE (...), -1);
> into
>   else if (INTEGRAL_TYPE_P (TREE_TYPE (...)))
>     *m = build_int_cst (TREE_TYPE (...), -1);
>   else
>     return false;

Also, for the testcase,
typedef int V __attribute__ ((vector_size (sizeof (int))));
looks weird, that is one element vector, can you use 2 * sizeof (int)
instead and add -w to dg-options (for the various -Wpsabi warnings)?

	Jakub

Patch

--- gcc/tree-tailcall.c.mp	2013-08-09 16:07:55.778616996 +0200
+++ gcc/tree-tailcall.c	2013-08-09 16:08:00.068635823 +0200
@@ -326,11 +326,7 @@  process_assignment (gimple stmt, gimple_
       return true;
 
     case NEGATE_EXPR:
-      if (FLOAT_TYPE_P (TREE_TYPE (op0)))
-        *m = build_real (TREE_TYPE (op0), dconstm1);
-      else
-        *m = build_int_cst (TREE_TYPE (op0), -1);
-
+      *m = build_minus_one_cst (TREE_TYPE (op0));
       *ass_var = dest;
       return true;
 
@@ -339,11 +335,7 @@  process_assignment (gimple stmt, gimple_
         *a = fold_build1 (NEGATE_EXPR, TREE_TYPE (non_ass_var), non_ass_var);
       else
         {
-          if (FLOAT_TYPE_P (TREE_TYPE (non_ass_var)))
-            *m = build_real (TREE_TYPE (non_ass_var), dconstm1);
-          else
-            *m = build_int_cst (TREE_TYPE (non_ass_var), -1);
-
+	  *m = build_minus_one_cst (TREE_TYPE (non_ass_var));
           *a = fold_build1 (NEGATE_EXPR, TREE_TYPE (non_ass_var), non_ass_var);
         }
 
--- gcc/testsuite/gcc.dg/pr57980.c.mp	2013-08-09 16:07:25.967485356 +0200
+++ gcc/testsuite/gcc.dg/pr57980.c	2013-08-09 16:07:17.967450526 +0200
@@ -0,0 +1,19 @@ 
+/* PR tree-optimization/57980 */
+/* { dg-do compile } */
+/* { dg-options "-O -foptimize-sibling-calls" } */
+
+typedef int V __attribute__ ((vector_size (sizeof (int))));
+extern V f (void);
+
+V
+bar (void)
+{
+  return -f ();
+}
+
+V
+foo (void)
+{
+  V v = { };
+  return v - f ();
+}