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

  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.



Signed-off-by: Keven Boell <keven.boell@intel.com>
---
 gdb/dwarf2read.c |   28 ++++++++++++++++++++++++++++
 gdb/gdbtypes.h   |   26 ++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)
  

[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]

> 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..

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.

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.
  

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".
  

> 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.

I thought of that, also, and in the end, decided not to suggest it
because it is slightly more complex to implement. In particular,
you'd need to know whether you'll neex some extra room at the end
before you allocate the type (or else, you'll have to realloc it
later on, and this might not be easy to do when it is allocated on
an obstack). There were also minor difficulties associated to
debugging and pretty-printing, but nothing necessarily unsurmountable.

Probably, as you suggest also, sub-classing would help more.
  

On Wed, Feb 11, 2015 at 12:43 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>> 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.
>
> I thought of that, also, and in the end, decided not to suggest it
> because it is slightly more complex to implement. In particular,
> you'd need to know whether you'll neex some extra room at the end
> before you allocate the type (or else, you'll have to realloc it
> later on, and this might not be easy to do when it is allocated on
> an obstack).

Yep. 'twas on my mind as well, it's something I'd like to see
looked into.
  

On 09.02.2015 07:52, Joel Brobecker 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]
> 
>> 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..
> 
> 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.
> 
> 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.
> 

Hi Joel,

Thanks for your reply. Appreciate the time you invested for reviewing the patches.
I will re-work this part to have the new attributes as a linked list as you proposed and will come up with an updated patch series.

Thanks,
Keven