Patchwork fix cross build

login
register
mail settings
Submitter Nathan Sidwell
Date May 23, 2012, 5:58 p.m.
Message ID <4FBD2560.3010704@acm.org>
Download mbox | patch
Permalink /patch/160981/
State New
Headers show

Comments

Nathan Sidwell - May 23, 2012, 5:58 p.m.
On 05/22/12 15:12, Richard Guenther wrote:

> But I wonder why CONSTRUCTORs do not inherit TREE_SIDE_EFFECTS
> properly ...

the attached patch fixes the ICE and causes no regressions on i686-pc-linux-gnu.

ok?

nathan
Richard Guenther - May 24, 2012, 8:03 a.m.
On Wed, May 23, 2012 at 7:58 PM, Nathan Sidwell <nathan@acm.org> wrote:
> On 05/22/12 15:12, Richard Guenther wrote:
>
>> But I wonder why CONSTRUCTORs do not inherit TREE_SIDE_EFFECTS
>> properly ...
>
>
> the attached patch fixes the ICE and causes no regressions on
> i686-pc-linux-gnu.
>
> ok?

Looks ok to me.  Though I wonder how we got away with that for so long time ...

What do others prefer?  Keep CONSTRUCTORs "broken" and paper over in
gimplify.c instead?

If you don't hear from somebody else in 24h the patch is ok as-is
(can you do some grepping whether there are callers of build_constructor
that set TREE_SIDE_EFFECTS on it afterwards?)

Thanks,
Richard.

> nathan
Nathan Sidwell - May 27, 2012, 4:28 p.m.
On 05/24/12 09:03, Richard Guenther wrote:

> If you don't hear from somebody else in 24h the patch is ok as-is
> (can you do some grepping whether there are callers of build_constructor
> that set TREE_SIDE_EFFECTS on it afterwards?)

I committed the patch.  grepping C & C++ sources didn't show callers of 
build_constructor poking T_S_E themselves.  some of them did clear TREE_CONSTANT 
though.  Rather than chase the rationale for that I decided to let them lie.

nathan

Patch

2012-05-23  Nathan Sidwell  <nathan@acm.org>

	* tree.c (build_constructor): Propagate TREE_SIDE_EFFECTS.

	* gcc.dg/stmt-expr-4.c: New.

Index: tree.c
===================================================================
--- tree.c	(revision 187628)
+++ tree.c	(working copy)
@@ -1416,17 +1416,24 @@  build_constructor (tree type, VEC(constr
   unsigned int i;
   constructor_elt *elt;
   bool constant_p = true;
+  bool side_effects_p = false;
 
   TREE_TYPE (c) = type;
   CONSTRUCTOR_ELTS (c) = vals;
 
   FOR_EACH_VEC_ELT (constructor_elt, vals, i, elt)
-    if (!TREE_CONSTANT (elt->value))
-      {
+    {
+      /* Mostly ctors will have elts that don't have side-effects, so
+	 the usual case is to scan all the elements.  Hence a single
+	 loop for both const and side effects, rather than one loop
+	 each (with early outs).  */
+      if (!TREE_CONSTANT (elt->value))
 	constant_p = false;
-	break;
-      }
+      if (TREE_SIDE_EFFECTS (elt->value))
+	side_effects_p = true;
+    }
 
+  TREE_SIDE_EFFECTS (c) = side_effects_p;
   TREE_CONSTANT (c) = constant_p;
 
   return c;
Index: testsuite/gcc.dg/stmt-expr-4.c
===================================================================
--- testsuite/gcc.dg/stmt-expr-4.c	(revision 0)
+++ testsuite/gcc.dg/stmt-expr-4.c	(revision 0)
@@ -0,0 +1,22 @@ 
+
+/* { dg-options "-O2 -std=gnu99" } */
+/* Internal compiler error in iterative_hash_expr */
+
+struct tree_string
+{
+  char str[1];
+};
+
+union tree_node
+{
+  struct tree_string string;
+};
+
+char *Foo (union tree_node * num_string)
+{
+  char *str = ((union {const char * _q; char * _nq;})
+	       ((const char *)(({ __typeof (num_string) const __t
+				     = num_string;  __t; })
+			       ->string.str)))._nq;
+  return str;
+}