PARM_DECL of DECL_SIZE 0, but TYPE_SIZE of 96 bits

PARM_DECL of DECL_SIZE 0, but TYPE_SIZE of 96 bits

[Daniel Berlin]

Why is that C++ can't create normal DECL's like everyone else?

Case in point:

(gdb)
 <parm_decl 0x40125000 it
    type <record_type 0x4010b2f4 node_iterator needs-constructing type_1
type_5 type_6 BLK
        size <integer_cst 0x4002aa80 constant invariant 96>
        unit size <integer_cst 0x4002aa98 constant invariant 12>
...

    addressable used BLK file minimal.c line 194 size <integer_cst
0x4002a960 0> unit size <integer_cst 0x4002a198 0>
$11 = void
(gdb) y



So we've got a parm decl that if you ask it for the DECL_SIZE, says 0,
but has a TYPE_SIZE of 12 bytes, and we access fields in it, etc.


This is causing a bug in detecting what portions of structures are used
in tree-ssa-alias, because we use DECL_SIZE, and this causes us to think
we use no part of the structure, since it gets min/max of 0!


I'm going to work around this by using TYPE_SIZE, but it would be nice
if somebody could explain the purpose for this behavior (if it's a bug,
i'll file a bug report). I would imagine we don't have truly empty
things in C++, so you could simply assert that TREE_INT_CST_LOW of
whatever you are setting DECL_SIZE to is not 0 and find these that way.


--Dan

Re: PARM_DECL of DECL_SIZE 0, but TYPE_SIZE of 96 bits

[Richard Henderson]

On Wed, Jun 29, 2005 at 06:57:12PM -0400, Daniel Berlin wrote:
> I'm going to work around this by using TYPE_SIZE, but it would be nice
> if somebody could explain the purpose for this behavior (if it's a bug,
> i'll file a bug report). I would imagine we don't have truly empty
> things in C++, so you could simply assert that TREE_INT_CST_LOW of
> whatever you are setting DECL_SIZE to is not 0 and find these that way.

It is most definitely a bug.  I'm surprised about the 0 instead
of a NULL there.  The later would easily be explicable by 
forgetting to call layout_decl.

My only guess is that this decl had an incomplete type at some
point.  Is the function in question a template?  I could see as
how maybe we need to call relayout_decl after instantiation, or
simply re-order how the parm_decls are created.


r~

Re: PARM_DECL of DECL_SIZE 0, but TYPE_SIZE of 96 bits

[Daniel Berlin]

On Wed, 2005-06-29 at 16:55 -0700, Richard Henderson wrote:
> On Wed, Jun 29, 2005 at 06:57:12PM -0400, Daniel Berlin wrote:
> > I'm going to work around this by using TYPE_SIZE, but it would be nice
> > if somebody could explain the purpose for this behavior (if it's a bug,
> > i'll file a bug report). I would imagine we don't have truly empty
> > things in C++, so you could simply assert that TREE_INT_CST_LOW of
> > whatever you are setting DECL_SIZE to is not 0 and find these that way.
> 
> It is most definitely a bug.  I'm surprised about the 0 instead
> of a NULL there.  The later would easily be explicable by 
> forgetting to call layout_decl.


Okay, well, here's what happens:

1. layout_decl gets called on the original PARM_DECL, when it's still a
template parameter, which sets the size to type_size, which is still 0
at that point, so we get DECL_SIZE of INTEGER_CST (0).

2. We copy the parm_decl when instantiating the template.

3. layout_decl gets called later on the copy of the PARM_DECL in the
instantiated function, which has a DECL_SIZE of INTEGER_CST (0) still.

layout_decl does nothing to DECL_SIZE and DECL_SIZE_UNIT of this
PARM_DECL in this case, even though TYPE_SIZE has changed from 0 to 96
bits.


I imagine the correct thing to do here is to 
1. In require_complete_types_for_parms, in the C++ FE, reset DECL_SIZE
to NULL before we call layout_decl on the parm and let layout_decl
figure out what to do.

or

2. Add code in layout_decl to copy TYPE_SIZE/TYPE_SIZE_UNIT if the
DECL_SIZE is integer_cst (0)
or

3. Not call layout_decl on the template types until they are completed.


Doing 1 fixes the bug I'm seeing and seems to do the correct thing, but
I'm not a C++ person, so there may be dragons this way.

--Dan

[C++] Re: PARM_DECL of DECL_SIZE 0, but TYPE_SIZE of 96 bits

[Richard Henderson]

On Wed, Jun 29, 2005 at 09:17:07PM -0400, Daniel Berlin wrote:
> 1. In require_complete_types_for_parms, in the C++ FE, reset DECL_SIZE
> to NULL before we call layout_decl on the parm and let layout_decl
> figure out what to do.

This is what relayout_decl does.

> 2. Add code in layout_decl to copy TYPE_SIZE/TYPE_SIZE_UNIT if the
> DECL_SIZE is integer_cst (0)

Bad.

> 3. Not call layout_decl on the template types until they are completed.

Certainly an option; not doing extra work is good.

4. Make sure that template types are incomplete.  That is, with
   TYPE_SIZE/TYPE_SIZE_UNIT unset.

A C++ front end maintainer should help choose.


r~

Re: PARM_DECL of DECL_SIZE 0, but TYPE_SIZE of 96 bits

[Giovanni Bajo]

Daniel Berlin <dberlin@dberlin.org> wrote:

> So we've got a parm decl that if you ask it for the DECL_SIZE, says 0,
> but has a TYPE_SIZE of 12 bytes, and we access fields in it, etc.


I am not sure of what the exact relations between DECL_SIZE and TYPE_SIZE
are, but probably some verification could be done, eg, at gimplification
time, rather than waiting for latent bugs in optimizers to produce wrong
code?
-- 
Giovanni Bajo

Re: PARM_DECL of DECL_SIZE 0, but TYPE_SIZE of 96 bits

[Daniel Berlin]

On Thu, 2005-06-30 at 20:47 +0200, Giovanni Bajo wrote:
> Daniel Berlin <dberlin@dberlin.org> wrote:
> 
> > So we've got a parm decl that if you ask it for the DECL_SIZE, says 0,
> > but has a TYPE_SIZE of 12 bytes, and we access fields in it, etc.
> 
> 
> I am not sure of what the exact relations between DECL_SIZE and TYPE_SIZE
> are, but probably some verification could be done, eg, at gimplification
> time, rather than waiting for latent bugs in optimizers to produce wrong
> code?

I'll add a verify pass, but i'm sure it's going to be painful :).

--Dan

Re: [C++] Re: PARM_DECL of DECL_SIZE 0, but TYPE_SIZE of 96 bits

[Geoffrey Keating]

Richard Henderson <rth@redhat.com> writes:

> On Wed, Jun 29, 2005 at 09:17:07PM -0400, Daniel Berlin wrote:
> > 1. In require_complete_types_for_parms, in the C++ FE, reset DECL_SIZE
> > to NULL before we call layout_decl on the parm and let layout_decl
> > figure out what to do.
> 
> This is what relayout_decl does.
> 
> > 2. Add code in layout_decl to copy TYPE_SIZE/TYPE_SIZE_UNIT if the
> > DECL_SIZE is integer_cst (0)
> 
> Bad.
> 
> > 3. Not call layout_decl on the template types until they are completed.
> 
> Certainly an option; not doing extra work is good.
> 
> 4. Make sure that template types are incomplete.  That is, with
>    TYPE_SIZE/TYPE_SIZE_UNIT unset.

I think this makes a lot of sense considering the language semantics.
(An alternative would be to make TYPE_SIZE an expression based on the
template parameter, but that seems like a lot of work for very little
gain.)

Re: [C++] Re: PARM_DECL of DECL_SIZE 0, but TYPE_SIZE of 96 bits

[Daniel Berlin]

On Thu, 2005-06-30 at 14:11 -0700, Geoffrey Keating wrote:
> Richard Henderson <rth@redhat.com> writes:
> 
> > On Wed, Jun 29, 2005 at 09:17:07PM -0400, Daniel Berlin wrote:
> > > 1. In require_complete_types_for_parms, in the C++ FE, reset DECL_SIZE
> > > to NULL before we call layout_decl on the parm and let layout_decl
> > > figure out what to do.
> > 
> > This is what relayout_decl does.
> > 
> > > 2. Add code in layout_decl to copy TYPE_SIZE/TYPE_SIZE_UNIT if the
> > > DECL_SIZE is integer_cst (0)
> > 
> > Bad.
> > 
> > > 3. Not call layout_decl on the template types until they are completed.
> > 
> > Certainly an option; not doing extra work is good.
> > 
> > 4. Make sure that template types are incomplete.  That is, with
> >    TYPE_SIZE/TYPE_SIZE_UNIT unset.
> 
> I think this makes a lot of sense considering the language semantics.

This i'm not sure how to do.
:)
I've submitted a patch for option 1 for the moment, and i'll go explore
option 4 while someone considers that (since it's blocking 22071, which
has been a regression for a few weeks now :( )

Re: [C++] Re: PARM_DECL of DECL_SIZE 0, but TYPE_SIZE of 96 bits

[Mark Mitchell]

Daniel Berlin wrote:
> On Thu, 2005-06-30 at 14:11 -0700, Geoffrey Keating wrote:
> 

>>>Certainly an option; not doing extra work is good.
>>>
>>>4. Make sure that template types are incomplete.  That is, with
>>>   TYPE_SIZE/TYPE_SIZE_UNIT unset.
>>
>>I think this makes a lot of sense considering the language semantics.

It does not make sense to make template types incomplete, as they are in 
fact complete, and there are situations where the compiler needs to look 
inside them.  (For example, we need to look to see if there's a 
declaration of "void A<T>::f()" when we see a declaration of such a 
function.)

I'm not sure why any declaration with dependent type is ever reaching 
the middle end.  That sounds like a problem to me, unless its purely in 
the context of debugging information.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PARM_DECL of DECL_SIZE 0, but TYPE_SIZE of 96 bits

[Mark Mitchell]

Daniel Berlin wrote:

> 3. Not call layout_decl on the template types until they are completed.

In the abstract, this is the best choice.  Although we need to know that 
types are complete (which means (in the current implementation) that 
TYPE_SIZE != NULL_TREE), but the C++ front end should not be caring 
about DECL_SIZE on a PARM_DECL from a template.  If it is, I'd like to 
know where.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PARM_DECL of DECL_SIZE 0, but TYPE_SIZE of 96 bits

[Daniel Berlin]

On Tue, 2005-07-05 at 08:35 -0700, Mark Mitchell wrote:
> Daniel Berlin wrote:
> 
> > 3. Not call layout_decl on the template types until they are completed.
> 
> In the abstract, this is the best choice.  Although we need to know that 
> types are complete (which means (in the current implementation) that 
> TYPE_SIZE != NULL_TREE), but the C++ front end should not be caring 
> about DECL_SIZE on a PARM_DECL from a template.  If it is, I'd like to 
> know where.

the layout_decl that sets it to size 0 gets called originally from

#0  layout_decl (decl=0x40088b64, known_align=0) at stor-layout.c:294
#1  0x085f4d51 in build_decl_stat (code=PARM_DECL, name=0x400cc208,
type=0x40088870) at tree.c:2874
#2  0x080f88c3 in cp_build_parm_decl (name=0x400cc208, type=0x40088870)
at decl2.c:141
#3  0x0807bc94 in grokdeclarator (declarator=0x898b244,
declspecs=0x898b268, decl_context=PARM, initialized=0,
attrlist=0xbfffe918)
    at decl.c:7768



IE it's getting called from common code.

Because i wasn't sure how to get around this easily, i simply went for
the easy solution :)