Re: [PATCH, pretty-ipa] Fix wave ICE in SRA

[Richard Guenther <richard dot guenther at gmail dot com>]

2009/5/6 Martin Jambor <mjambor@suse.cz>:
> Hi,
>
> sorry for a late reply, I was on vacation and in wilderness last two
> days.
>
> On Sat, May 02, 2009 at 11:18:56AM +0200, Richard Guenther wrote:
>> On Fri, May 1, 2009 at 11:59 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> >> I would think that if DECL_SIZE is existent you should use it.
>> >
>> > Yes, I agree.
>> >
>> >> I think the problem here simply is, that gen_inner_reference is supposed
>> >> to be called only with COMPONENT_REF, ARRAY_{RANGE,}REF or BIT_FIELD_REF.
>> >
>> > Yes, get_inner_reference should only be called on handled_component_p trees,
>> > that may indeed be the bug.
>>
>> Ok, that shows up a suitable fix here. ?Note that I don't agree that
>> get_inner_reference
>> (or in this case get_ref_base_and_extent) should be not called on
>> decls, that only
>> puts extra burdens on the caller.
>>
>> Martin, try extending get_ref_base_and_extent like Micha suggested - does that
>> fix your problem?
>
> If you look at the tree dumps ?in my email from April 30th, you'll see
> that the RHS VAR_DECL DECL_SIZE is consistent with the TYPE_SIZE (both
> are ?128 ?bits) and ?so ?still different ?from ?the ?DECL_SIZE of ?the
> FIELD_DECL on the RHS (72 bits). ?Therefore using DECL_SIZE instead of
> TYPE_SIZE for DECL_P arguments does not help in this particular case.
>
> But for ?the sake of it, I ?have tried putting the ?following lines to
> the beginning of get_ref_base_and_extent but it did not help:
>
> ?if (DECL_P (exp))
> ? ?size_tree = DECL_SIZE (exp);
> ?else if (TREE_CODE (exp) == COMPONENT_REF)
> ?...and so on...
>
> On the other hand, when I changed the second line like this:
>
> ?if (TREE_CODE (exp) == COMPONENT_REF)
> ? ?size_tree = TYPE_SIZE (TREE_TYPE (TREE_OPERAND (exp, 1)));
> ?else if ...and so on...
>
> I ?did ?not hit ?the ?assert ?and the ?problem ?went ?away (i.e. ?this
> particular problem, I did not bootstrap or test it).
>
> Apparently, at ?least this particular VAR_DECL has ?its alingment also
> included in its DECL_SIZE.
>
> Given the above, what is the ?correct thing to do? ?Is calculating the
> size only from TYPE_SIZEs correct? ?Would this negatively affect other
> users ? of ?get_ref_base_and_extent()? ? ?Would ?it ? mix ? well ?with
> GET_MODE_BITSIZE? ? Or ?should ?I ?round ?up ?the ?DECL_SIZEs ?to ?the
> alignment ?requirement in ?some ?other ?way? ?Or ?should ?we take ?the
> opposite ?approach ?and ?somehow ? strip ?off ?the ?padding ?from ?the
> individual DECL_SIZEs? ?If so, how do I determine them?

I can certainly imagine that for example with

 T *p = &decl.field;
 T x = *p;

we can create such inconsistency if T is a sub-structure in decl and
it's TYPE_SIZE is padded at the end while the field-decls DECL_SIZE
does not include it.

It looks like that if the theory is that the extra size can only be padding
we can simply ignore this difference - and chose either size that fits
our previous analysis more.  Would that be a solution for the SRA problem?

In any case the get_ref_base_and_extent patch is still correct and fixing
a bug.

Richard.

> Thanks for looking into this,
>
> Martin
>