On Wed, Feb 27, 2019 at 9:01 AM Martin Liška <mli...@suse.cz> wrote:
>
> On 2/26/19 7:48 PM, Richard Biener wrote:
> > On February 26, 2019 6:50:13 PM GMT+01:00, "Martin Liška" <mli...@suse.cz> 
> > wrote:
> >> On 2/26/19 4:02 PM, Richard Biener wrote:
> >>> On Tue, Feb 26, 2019 at 3:30 PM Martin Liška <mli...@suse.cz> wrote:
> >>>>
> >>>> Hi.
> >>>>
> >>>> I've rewritten bitmap memory statistics which abused unsigned
> >>>> type via register_overhead (map, -((int)sizeof (bitmap_head))).
> >>>> I come up with a concept that each bitmap has assigned a unique ID
> >>>> which is used for stats tracking. It's caused by fact that e.g. DF
> >> is
> >>>> heavily reallocating bitmaps that then have a different address.
> >>>>
> >>>> Survives bootstrap with --enable-gather-detailed-mem-stats.
> >>>>
> >>>> Ready for next stage1?
> >>>
> >>> +  /* Get bitmap descriptor UID casted to an unsigned integer
> >> pointer.  */
> >>> +  unsigned *get_descriptor ()
> >>> +  {
> >>> +    return (unsigned *)(ptrdiff_t)alloc_descriptor;
> >>> +  }
> >>>
> >>> this one is a bit ugly and together with
> >>
> >> I know it's not perfect.
> >>
> >>>
> >>> template <typename Type>
> >>> inline hashval_t
> >>> pointer_hash <Type>::hash (const value_type &candidate)
> >>> {
> >>>   /* This is a really poor hash function, but it is what the current
> >> code uses,
> >>>      so I am reusing it to avoid an additional axis in testing.  */
> >>>   return (hashval_t) ((intptr_t)candidate >> 3);
> >>>
> >>> will give quite some hash collisions.  So I guess you should shift
> >>> the descriptor << 3 at least (and then make it at most 29 bits in
> >>> size?).
> >>
> >> That's easily doable.
> >>
> >>> Not sure what to do about the descriptor wrapping btw.
> >>
> >> Question is whether we want to invest in our internal scaffolding more
> >> time?
> >> Or can we live with the ugly casting?
> >
> > I guess we can live with it if we can avoid the hash collisions.
>
> Great.
>
> Then there's updated version of the patch for next stage1.

LGTM.  The << 3 in get_descriptor deserves a comment though.

Also leaving two bits in the bitfield uninitialized may generate
awkward code (you might want to check), explicitely having
a pad : 2 and initializing that to zero might allow better code
generation here (guarding the member and init with
#if might also be possible).

Richard.

> Martin
>
> >
> > Richard.
> >
> >> Martin
> >>
> >>>
> >>> Richard.
> >>>
> >>>> Thanks,
> >>>> Martin
> >>>>
> >>>> gcc/ChangeLog:
> >>>>
> >>>> 2019-02-26  Martin Liska  <mli...@suse.cz>
> >>>>
> >>>>         * bitmap.c (bitmap_register): Come up with
> >>>>         alloc_descriptor_max_uid and assign it for
> >>>>         a new bitmap.
> >>>>         (register_overhead): Use get_descriptor as
> >>>>         a descriptor.
> >>>>         (release_overhead): New.
> >>>>         (bitmap_elem_to_freelist): Call it.
> >>>>         (bitmap_elt_clear_from): Likewise.
> >>>>         (bitmap_obstack_free): Likewise.
> >>>>         (bitmap_move): Sensitively release memory.
> >>>>         * bitmap.h (struct GTY): Add alloc_descriptor.
> >>>>         (bitmap_initialize): Initialize alloc_descriptor to zero.
> >>>>         * tree-ssa-pre.c (do_hoist_insertion): Use bitmap_move.
> >>>> ---
> >>>>  gcc/bitmap.c       | 39 ++++++++++++++++++++++++++++-----------
> >>>>  gcc/bitmap.h       | 17 ++++++++++++++---
> >>>>  gcc/tree-ssa-pre.c |  2 +-
> >>>>  3 files changed, 43 insertions(+), 15 deletions(-)
> >>>>
> >>>>
> >
>

Reply via email to