This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, pretty-ipa] Fix wave ICE in SRA
On Thu, May 7, 2009 at 12:51 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Thu, May 7, 2009 at 12:06 PM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Thu, May 7, 2009 at 12:50 AM, Michael Matz <matz@suse.de> wrote:
>>> Hi,
>>>
>>> On Thu, 7 May 2009, Richard Guenther wrote:
>>>
>>>> > ?if (TREE_CODE (exp) == COMPONENT_REF)
>>>> > ? ?size_tree = TYPE_SIZE (TREE_TYPE (TREE_OPERAND (exp, 1)));
>>>> > ?else if ...and so on...
>>>> >
>>>> > is wrong, it's DECL_SIZE (TREE_OPERAND (exp, 1)).
>>>>
>>>> They should be exchangable in the context of an access.
>>>
>>> As pointed out, you always should use DECL_SIZE if available. ?IMO.
>>>
>>> ...
>>>
>>> May I say that this whole thing looks rather disturbing, mismatching
>>> random mixes of different TYPE_ or DECL_SIZEs on the same assignment can't
>>> be a good sign of a proper intermediate representation. ?Except for very
>>> good reasons, maybe bitfields, but a simple copy from a base variable ...
>>>
>>> IMO that variable shouldn't have had a DECL_SIZE that includes alignment ...
>>> I would understand that TYPE_SIZE includes it (and only for the reason
>>> that constructing arrays of such elements then implicitely includes the
>>> alignment when going from one to the next member).
>>>
>>>
>>>> ?At least
>>>> ? ... = x.a;
>>>> and
>>>>
>>>> ? p = &x.a;
>>>> ? ... = *p;
>>>>
>>>> already have different sizes and that better should not matter(?).
>>>
>>> Well, if typeof(*p) has larger alignment (and hence TYPE_SIZE) than
>>> typeof(x.a) you have a problem when the end of x.a lies exactly on a page
>>> border for instance, and you want to copy too many bytes. ?Weren't there
>>> even bugreports about exactly such situation?
>>
>> I think they would be valid only if the type is packed in which case
>> TYPE_SIZE would always match the decl or field size.
>>
>>> (Of course *p shouldn't have larger alignment from the start to not
>>> introduce unaligned accesses, but in that case they also will have the
>>> same TYPE_SIZEs again, so I stay by my nervousness about mismatching
>>> TYPE/DECL_SIZEs)
>>
>> Hm, at least I cannot reproduce with a simple C testcase:
>>
>> struct A {
>> ?int i;
>> ?char c;
>> };
>> struct B {
>> ?struct A a;
>> ?int b;
>> };
>> void foo(void)
>> {
>> ?struct A a;
>> ?struct B b;
>> ?struct A *p = &b.a;
>> ?*p = a;
>> }
>>
>> the FIELD_DECL b.a has size 8, just like the struct A type has.
>> I suppose with C++ the situation may be different because of
>> tail packing. ?For example with
>>
>> struct A {
>> ?A() {}
>> ?int i;
>> ?char c;
>> };
>> struct B : A {
>> ?B() {}
>> ?char c;
>> };
>> void foo(void)
>> {
>> ?A a;
>> ?B b;
>> ?A *p = &b;
>> ?*p = a;
>> ?static_cast<A&>(b) = a;
>> }
>>
>> the anonymous field-decl for the base A has a DECL_SIZE not
>> including the tail padding. ?But the C++ FE is very careful to
>> use memcpy for both assignments _not_ including the tail
>> padding.
>>
>> So I'm curious where we end up generating this possibly
>> bogus assignment statement ...
>
> It seems to be early inlining producing this stmt from inlining
>
> phoenix::actor<BaseT>::actor(const BaseT&) [with BaseT =
> phoenix::logical_or_composite<phoenix::actor<phoenix::closure_member<0,
> phoenix::closure<boost::wave::grammars::closures::closure_value> > >,
> phoenix::actor<phoenix::argument<0> > >] D.105075 (struct
> actorD.104944 * const thisD.105076, const struct
> logical_or_compositeD.104943 & baseD.105077)
> {
> <bb 2>:
> ?thisD.105076_1(D)->D.105085 = *baseD.105077_2(D);
> ?return;
>
> }
>
> (which already looks bogus!)
>
> into
>
> phoenix::actor<phoenix::logical_or_composite<phoenix::actor<BaseT>,
> phoenix::actor<BaseT> > > phoenix::operator||(const
> phoenix::actor<BaseT>&, const phoenix::actor<BaseT>&) [with BaseT0 =
> phoenix::closure_member<0,
> phoenix::closure<boost::wave::grammars::closures::closure_value> >,
> BaseT1 = phoenix::argument<0>] D.104951 (const struct actorD.84106 &
> _0D.104952, const struct actorD.70282 & _1D.104953)
> {
> ?struct logical_or_compositeD.104943 D.139113;
> ?struct actorD.104944 D.139123;
> ?struct actorD.104944 D.139124;
>
> <bb 2>:
> ?__comp_ctor D.104972 (&D.139113, _0D.104952_1(D), _1D.104953_2(D));
> ?__comp_ctor D.105075 (&D.139123, &D.139113);
> ?D.139124 = D.139123;
> ?return D.139124;
>
> }
>
> Thus I think this is a frontend bug.
>
> Note there is no .original dump for exactly the called function, but
> it only appears after .007.useless (thus likely it is a clone).
>
> Testcase to reproduce the issue:
>
> struct Base {
> ?Base();
> ?int i;
> ?char c;
> };
>
> template <typename BaseT>
> struct actor : public BaseT {
> ?actor(BaseT const& base);
> ?char c;
> };
>
> template <typename BaseT>
> actor<BaseT>::actor(BaseT const& base)
> ? ? : BaseT(base) {}
>
> actor<Base>
> foo(const Base c)
> {
> ?return actor<Base>(c);
> }
>
> with -O2 we inline the constructor and get
>
> actor<Base> foo(Base) D.2099 (const struct Base c)
> {
> ?struct actor D.2133;
> ?struct actor D.2134;
>
> <bb 2>:
> ?D.2133.D.2111 = c;
> ?D.2134 = D.2133;
> ?return D.2134;
>
> }
>
> The assignment
>
> ?D.2133.D.2111 = c;
>
> has on the LHS a size of 40 (from the FIELD_DECL) and on the
> RHS a size of 64, from the decl. ?In the un-inlined constructor we had
> the bogus
>
> actor<BaseT>::actor(const BaseT&) [with BaseT = Base] D.2105 (struct
> actor * const this, const struct Base & base)
> {
> <bb 2>:
> ?this_1(D)->D.2111 = *base_2(D);
> ?return;
>
> }
>
> with the same issue (size 40 on the lhs, size 64 on the rhs).
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40058
Richard.
> Richard.
>