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