This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Replace a SRA FIXME with an assert
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 <mjambor@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));