On Wed, Mar 21, 2012 at 08:46:49AM +0100, Richard Guenther wrote:
> On Tue, 20 Mar 2012, Martin Jambor wrote:
>
> > 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).
>
> Yeah, it does that.
>
> > >
> > > 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.
>
> I'd just remove the comment then.
>
OK, I committed the following (after including it in a bootstrap of
another patch)
Thanks,
Martin
2012-03-23 Martin Jambor <mjam...@suse.cz>
* tree-sra.c (build_accesses_from_assign): Remove FIXME comment.
Index: src/gcc/tree-sra.c
===================================================================
--- src.orig/gcc/tree-sra.c
+++ src/gcc/tree-sra.c
@@ -1175,8 +1175,6 @@ 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))
{
