On Wed, 22 Mar 2017, Jakub Jelinek wrote: > On Wed, Mar 22, 2017 at 09:20:32AM +0100, Richard Biener wrote: > > > --- gcc/gimplify.c.jj 2017-03-21 07:56:55.000000000 +0100 > > > +++ gcc/gimplify.c 2017-03-21 13:37:45.555612652 +0100 > > > @@ -5098,6 +5098,13 @@ gimplify_modify_expr_rhs (tree *expr_p, > > > if (ret != GS_ERROR) > > > ret = GS_OK; > > > > > > + /* If we are going to write RESULT more than once, clear > > > + TREE_READONLY flag, otherwise we might gimplify it > > > + incorrectly. */ > > > + if (DECL_P (result) > > > > Can it ever be that result is sth like decl.component? At least I > > don't see GENERIC restricted in a way that it cannot. (and generally > > assigning to sth TREE_READONLY is fishy at best...) > > I guess it can, but it can't trigger > if (valid_const_initializer > && num_nonzero_elements > 1 > && TREE_READONLY (object) > && VAR_P (object) > && (flag_merge_constants >= 2 || !TREE_ADDRESSABLE (object))) > which is the problem here. The problem isn't multiple assignments to > TREE_READONLY object if it isn't promoted to static.
Ok. > So, if you want, I can replace the DECL_P (result) check with VAR_P > (result), and perhaps change the comment somehow. Yeah, changing it to VAR_P and expanding the comment so it says it avoids incorrect promotion to readonly-static. (so it _is_ fishy that we have multiple assignments to TREE_READONLY objects given that code looks at a single assignment only) > > I see how this fixes the problem at hand (and in a safe way), so the > > patch is probably ok, just looking how to avoid this situation in > > a more general (and better) way. > > Jakub > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
