This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFA: shrink main_type


> This patch shrinks struct main_type 32- and 64-bit machines by
> shrinking the type_code and flags fields and moving around a couple
> other fields.
> 
> On my x86 box, this saves about 1% of memory on "gdb -readnow cc1" --
> a few hundred K -- at the cost of slightly increasing gdb's text size.

I'm not opposed to this sort of packing, particularly since this is
something that we already do for other fields of this type, but I'm
interested in the performance impact of such a change. A few hundred
K of memory for cc1 doesn't sound like a lot nowadays. I wonder what
it would cost and save for a program of the size of Mozilla...

> I noticed that the Ada code does not use TYPE_FLAG_* consistently with
> the rest of gdb (it tests the flags directly rather than using the
> macros), but I didn't change this.

Another good observation. I will try to find some time to fix...
I only saw a couple of instances (with TYPE_FIXED_INSTANCE). Did you
spot any other?

:REVIEWMAIL:

> 2008-08-17  Tom Tromey  <tromey@redhat.com>
> 
> 	* gdbtypes.h (enum type_flag_value): New enum.
> 	(TYPE_FLAG_UNSIGNED, TYPE_FLAG_NOSIGN, TYPE_FLAG_STUB,
> 	TYPE_FLAG_TARGET_STUB, TYPE_FLAG_STATIC, TYPE_FLAG_CONST,
> 	TYPE_FLAG_VOLATILE, TYPE_FLAG_PROTOTYPED, TYPE_FLAG_INCOMPLETE,
> 	TYPE_FLAG_CODE_SPACE, TYPE_FLAG_DATA_SPACE, TYPE_FLAG_VARARGS,
> 	TYPE_FLAG_VECTOR, TYPE_FLAG_ADDRESS_CLASS_1,
> 	TYPE_FLAG_ADDRESS_CLASS_2, TYPE_FLAG_FIXED_INSTANCE,
> 	TYPE_FLAG_STUB_SUPPORTED, TYPE_FLAG_NOTTEXT): Now enum constants.

I think this change is really great, and can be checked in separately
from the rest.

> 	(struct main_type) <code>: Now 5 bits.
> 	<flags>: Move earlier.  Now a bit field.
> 	<nfields, vptr_fieldno>: Move earlier.

I'm having trouble understanding the new layout that you propose.
It looks like this:

  ENUM_BITFIELD(type_code) code : 5;

  /* Array bounds.  These fields appear at this location because
     they pack nicely here.  */

  ENUM_BITFIELD(array_bound_type) upper_bound_type : 4;
  ENUM_BITFIELD(array_bound_type) lower_bound_type : 4;

  /* Flags about this type.  This field appears at this location
     because it packs nicely here.  */

  ENUM_BITFIELD(type_flag_value) flags : 18;

Do the upper/lower_bound_type fields still "pack nicely"?
This is where my knowledge of C (or lack thereof) shows up, but
would the following declaration instead help the compiler?

  ENUM_BITFIELD(type_code) code : 6;
  ENUM_BITFIELD(type_flag_value) flags : 18;
  ENUM_BITFIELD(array_bound_type) upper_bound_type : 4;
  ENUM_BITFIELD(array_bound_type) lower_bound_type : 4;

Then, I don't understand why the two "short" fields have been
moved up just behind the "flags" field (sorry, like I said, there
are holes in my C knowledge).
  
-- 
Joel


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]