Re: [PATCH] Replace a SRA FIXME with an assert

[Richard Guenther <rguenther at suse dot de>]

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.

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?

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));