Re: [V4 01/18] vla: introduce allocated/associated flags

[Doug Evans <dje at google dot com>]

On Sun, Feb 8, 2015 at 10:52 PM, Joel Brobecker <brobecker@adacore.com> wrote:
> [Copying Doug, as I think Doug has experience in this area, and is
> also dealing with the kind of giant programs where size of types
> really make a big difference]

Thanks!

>> Fortran 90 provide types whose values may be dynamically
>> allocated or associated with a variable under explicit
>> program control. The purpose of this commit is to read
>> allocated/associated DWARF tags and store them to the
>> main_type.
>>
>> 2014-05-28  Keven Boell  <keven.boell@intel.com>
>>             Sanimir Agovic  <sanimir.agovic@intel.com>
>>
>>       * dwarf2read.c (set_die_type): Add reading of
>>       allocated/associated flags.
>>       * gdbtypes.h (struct main_type): Add allocated/
>>       associated dwarf2_prop attributes.
>>       (TYPE_ALLOCATED_PROP): New macro.
>>       (TYPE_ASSOCIATED_PROP): New macro.
>>       (TYPE_NOT_ALLOCATED): New macro.
>>       (TYPE_NOT_ASSOCIATED): New macro.
>
> struct main_type is size-critical, so we simply cannot add
> fields to it that only a very small minority of instances
> will actually be using..

Yeah.
Plus it can be really hard to get rid of them, especially as
years go by and stuff gets built on top, so best be especially
conservative when adding them.

> To avoid the size increase, I propose that we turn...
>
>   struct dynamic_prop *data_location;
>
> ... into a chained list of dynamic properties. To determine
> which type of property each element in the list is, we'll need
> to introduce an enumerated type to be used as discriminant.

I don't know the details enough at the moment to help much here,
but *some* extension of data_location does sound reasonable, and
certainly ok from the standpoint of not growing main_type. :-)

I can even imagine removing data_location from the main_type "base class"
[see below].  But I'm not advocating that that needs to be done
now or even soon.

> So, I propose something like that:
>
> /* FIXME: Add comment.  */
>
> enum dynamic_prop_kind
> {
>   /* FIXME: Document.  */
>   DYNAMIC_PROP_DATA_LOCATION,
>   /* FIXME: Document.  */
>   DYNAMIC_PROP_ALLOCATED,
>   /* FIXME: Document.  */
>   DYNAMIC_PROP_ASSOCIATED,
> };
>
> /* FIXME: Document.  */
>
> struct dynamic_prop_list
> {
>   enum dynamic_prop_kind kind;
>   struct dynamic_prop *prop;
>   struct dynamic_prop_list *next;
> };
>
> ... then replace...
>
>   struct dynamic_prop *data_location;
>
> ... into ...
>
>   struct dynamic_prop_list *dyn_prop_list;
>
> ... and finally adjust the macros to go through the list instead of
> accessing the data through the dedicated fields. Using a function
> which does the search for a given kind will probably be useful.
>
> I think you might find it easier to do it in 2 stages (and therefore
> two patches):
>
>   1. Convert the current "data_location" field to the list scheme;
>   2. Then add the two new kinds of properties, which should then
>      be fairly straightforward.

Now that vptr_fieldno is gone we've got more room in the bitfields
section of struct main_type. I can imagine having a bit there that
says that data_location lives just after the struct.
We do similar things with other space-important types.
And if data_location has iterators for read access then it'd be trivial
to first check that bit. But again I'm not suggesting that needs to
be done now (or even soon).
I like the list-with-accessor-fns approach being proposed though because
I suspect it would make removing data_location from struct main_type
straightforward if/when there's a need for it.

One can imagine, of course, that if it were easier to subclass
in the language then such things would fall out naturally instead of
needing such attention and hacks.
There's another 8 bytes in main_type, type_specific, that could go away
if the data being pointed to followed the "base class".