http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57297
--- Comment #6 from rguenther at suse dot de <rguenther at suse dot de> --- On Thu, 16 May 2013, jamborm at gcc dot gnu.org wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57297 > > --- Comment #4 from Martin Jambor <jamborm at gcc dot gnu.org> --- > So far I have not attempted to reproduce this myself and so do not > quite follow all the previous comments but... > > (In reply to Richard Biener from comment #2) > > > > build_ref_for_offset ends up creating a MEM_REF with different alias pointer > > type than "the original" (note that there may be multiple originals and > > AFAIK we don't make any attempt to "merge" them conservatively). > > ...it looks like the second arguments in > generate_subtree_copies (racc->first_child, racc->base, 0, 0, 0, > gsi, false, false, loc); > and > generate_subtree_copies (lacc->first_child, lacc->base, 0, 0, 0, > gsi, true, true, loc); > > should be the rhs and lhs respectively, rather than [rl]acc->base. > Using rhs/lhs, the generated statements would have the same alias > pointer type as the respective sides of the original statement. Can > you try that? We should probably also change accordingly (almost all) > other calls to generate_subtree_copies (this code generally predates > generating MEM_REFs in build_ref_for_offset and this did not matter). I'm not sure - but yes, that would make sure the re-materialization uses the same alias set (does it? not sure exactly how generate_subtree_copies works) as the load that follows. Note that the Fortran frontend is where the bug lies here. I was thinking that the re-materialization for a whole-structure access could use a new temporary and indeed use the alias-set of the access we re-materialize it for for re-construction. Given that if we have if (a) a.b = 1; <use alias-set 3> else a.b = 2; <use alias-set 8> y = a; <aggregate use> we cannot re-materialize a before y = a in a way that restore original runtime behavior (the if or the else path may be never executed at runtime). But given that accessing 'a' in the aggregate copy with an alias-set that does not conflict with the actual dynamic type of the memory location invokes undefined behavior we can indeed make sure to re-materialize a before y = a using stores with the alias-set of that access. > > Re-materializing the original variable will always be hard. I believe that > > > > Index: gcc/tree-sra.c > > =================================================================== > > --- gcc/tree-sra.c (revision 198420) > > +++ gcc/tree-sra.c (working copy) > > @@ -3158,7 +3158,7 @@ sra_modify_assign (gimple *stmt, gimple_ > > > > if (modify_this_stmt > > || gimple_has_volatile_ops (*stmt) > > - || contains_vce_or_bfcref_p (rhs) > > + || contains_bitfld_comp_ref_p (rhs) > > || contains_vce_or_bfcref_p (lhs)) > > { > > if (access_has_children_p (racc)) > > > > should work. > > Even though relaxing this condition might be a good idea to try to > generate better code, I'd be against fixing bugs this way, if we can > avoid it. This should be the safe path capable of handling everything > that the latter more sophisticated approaches might choke on. It > would make the already complex code more difficult to maintain and > sooner or later we'd hit the same problem again (I think it should be > possible use structures with a single field to create a testcase with > a similar problem and modify_this_stmt set to true, for example). Yeah, it wasn't meant as a fix for the issue at hand. I remember we have the VCE special-casing here because, contrary to specification, VIEW_CONVERT_EXPRs may change the size of the access. Richard.
