On Thu, 4 Oct 2012, Lawrence Crowl wrote:
> On 10/4/12, Richard Guenther <rguent...@suse.de> wrote:
> > On Tue, 2 Oct 2012, Lawrence Crowl wrote:
> >> On 10/2/12, Richard Guenther <rguent...@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.
