Hi, On Tue, Mar 20, 2012 at 04:08:31PM +0100, Richard Guenther wrote: > On Tue, 20 Mar 2012, Martin Jambor wrote: > > > Hi, > > > > this patch which removes one of only two FIXMEs in tree-sra.c has been > > sitting in my patch queue for over a year. Yesterday I noticed it > > there, bootstrapped and tested it on x86_64-linux and it passed. > > > > I'd like to either commit it or just remove the comment, if there > > likely still are size inconsistencies in assignments but we are not > > planning to do anything with them in foreseeable future (and perhaps > > add a note to the bug). > > > > So, which should it be? > > Well. Aggregate assignments can still be off I think, especially > because of the disconnect between TYPE_SIZE and DECL_SIZE in > some cases, considering *p = x; with typeof (x) == typeof (*p) > (tail-padding re-use). > > The comments in PR40058 hint at that that issue might be fixed, > but I also remember issues with Ada.
The other FIXME in tree-sra.c suggests that Ada can produce VIEW_CONVERT_EXPRs with a different size than its argument, perhaps that is it (I'll try removing that one too). > > GIMPLE verification ensures compatible types (but not a match > of type_size / decl_size which will be exposed by get_ref_base_and_extent) > > But the real question is what do you want to guard against here? > The assert at least looks like it is going to triggert at some point, > but, would it be a problem if the sizes to not match? > I really can't remember what exactly happened but I do remember it did lead to a bug (it's been already part of the chck-in of new SRA so svn history does not help). We copy access tree children accross assignments and also change the type of the LHS access to a scalar if the RHS access is a scalar (assignments into a structure containing just one scalar) and both could lead to some access tree children covering larger part of the aggregate than the parent, making the children un-findable or even creating overlaps which are prohibited for SRA candidates. But as I wrote before, I'll be happy to just remove the FIXME comment. Martin > Richard. > > > > 2011-01-06 Martin Jambor <mjam...@suse.cz> > > > > * tree-sra.c (build_accesses_from_assign): Make size equality test > > an assert. > > > > Index: src/gcc/tree-sra.c > > =================================================================== > > --- src.orig/gcc/tree-sra.c > > +++ src/gcc/tree-sra.c > > @@ -1175,13 +1175,11 @@ build_accesses_from_assign (gimple stmt) > > && !lacc->grp_unscalarizable_region > > && !racc->grp_unscalarizable_region > > && AGGREGATE_TYPE_P (TREE_TYPE (lhs)) > > - /* FIXME: Turn the following line into an assert after PR 40058 is > > - fixed. */ > > - && lacc->size == racc->size > > && useless_type_conversion_p (lacc->type, racc->type)) > > { > > struct assign_link *link; > > > > + gcc_assert (lacc->size == racc->size); > > link = (struct assign_link *) pool_alloc (link_pool); > > memset (link, 0, sizeof (struct assign_link));