diff mbox

fix cross build

Message ID 4FBD2560.3010704@acm.org
State New
Headers show

Commit Message

Nathan Sidwell May 23, 2012, 5:58 p.m. UTC 
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

Comments

Richard Biener May 24, 2012, 8:03 a.m. UTC | #1 
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. UTC | #2 
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
diff mbox

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;
+}