This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC 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: Convert more non-GTY htab_t to hash_table.


On Thu, 4 Oct 2012, Lawrence Crowl wrote:

> On 10/4/12, Richard Guenther <rguenther@suse.de> wrote:
> > On Tue, 2 Oct 2012, Lawrence Crowl wrote:
> >> On 10/2/12, Richard Guenther <rguenther@suse.de> wrote:
> >> > On Mon, 1 Oct 2012, Lawrence Crowl wrote:
> >> > > Change more non-GTY hash tables to use the new type-safe
> >> > > template hash table.  Constify member function parameters that
> >> > > can be const.  Correct a couple of expressions in formerly
> >> > > uninstantiated templates.
> >> > >
> >> > > The new code is 0.362% faster in bootstrap, with a 99.5%
> >> > > confidence of being faster.
> >> > >
> >> > > Tested on x86-64.
> >> > >
> >> > > Okay for trunk?
> >> >
> >> > You are changing a hashtable used by fold checking, did you test
> >> > with fold checking enabled?
> >>
> >> I didn't know I had to do anything beyond the normal make check.
> >> What do I do?
> >>
> >> > +/* Data structures used to maintain mapping between basic blocks and
> >> > +   copies.  */
> >> > +static hash_table <bb_copy_hasher> bb_original;
> >> > +static hash_table <bb_copy_hasher> bb_copy;
> >> >
> >> > note that because hash_table has a constructor we now get global
> >> > CTORs for all statics :( (and mx-protected local inits ...)
> >>
> >> The overhead for the global constructors isn't significant.
> >> Only the function-local statics have mx-protection, and that can
> >> be eliminated by making them global static.
> >>
> >> > Can you please try to remove the constructor from hash_table to
> >> > avoid this overhead?  (as a followup - that is, don't initialize
> >> > htab)
> >>
> >> The initialization avoids potential errors in calling dispose.
> >> I can do it, but I don't think the overhead (after moving the
> >> function-local statics to global) will matter, and so I prefer to
> >> keep the safety.  So is the move of the statics sufficient or do
> >> you still want to remove constructors?
> >
> > Hm, having them in-scope where they are used is good style.
> > Why can't they be statically initialized and put in .data?
> > Please make it so - you know C++ enough (ISTR value-initialization
> > is default - which means NULL for the pointer?)
> 
> Zero initialization is default for static variables, but not for
> local or heap variables.  We can live with the uninitialized memory

Not for local static variables?  I mean, isn't it possible
to have an initializer like

foo ()
{
  static X x = X();

that behaves in the way that it is "inlined", that is, implemented
as putting x into .data, with proper initial contents?  Then,
why isn't there a way that X is default (aka zero) initialized
when there is no initializer?  I simply want C behavior back here ...
and I would have expected that not providing the
hash_table::hash_table() constructor would just do that (with
the complication that new-style allocation will end up with an
uninitialized object, which we could avoid with providing a operator new
implementation?).

> in some cases, and add another function to explicitly null the
> member in the rest of the cases.  I am not convinced that extra
> coding is worth the performance difference, particularly as I do
> not expect that difference to be measureable.
> 
> However we decide here, I think that work should be a separate patch,
> as it will certainly touch more files than the current patch.  So,
> can we separate the issue?

Sure, see my initial request.

Thanks,
Richard.


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