On 10/5/12, Richard Guenther <rguent...@suse.de> wrote:
> 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 intended my statement to include local static within static.
> I mean, isn't it possible to have an initializer like
>
> foo ()
> {
> static X x = X();
Without a default constructor definition, you would get zero
initialization by writing simply
static X x;
> that behaves in the way that it is "inlined", that is, implemented
> as putting x into .data, with proper initial contents?
The static initialization is permitted, but not required.
My general thinking is that if the dynamic initialization is a
problem, then we should change the compiler to do the permitted
inlining to static initialization.
BTW, C++11 addresses this issue head on, buy allowing you to declare
a constructor as 'constexpr', in which case static initialization
is guaranteed.
> 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?).
Providing an operator new to do this task would be stretching the
limits of what programmers expect here. It's not clear it would
matter either, because the constructor is going to be the same cost
inside operator new and outside operator new.
The real issue is non-static local variables. Programmers using
htab_t had to remember to assign null to it.
void func (...)
{
htab_t foo = NULL;
If you forgot, bad things could happen.
The C++ equivalent would be, as you suggest above, but with
a longer name.
hash_table <pointer_hash <whatever> > x
= hash_table <pointer_hash <whatever> > x ();
I'm guessing there would be some objection to typing that.
The alternative is an init member function.
hash_table <pointer_hash <whatever> > x;
x.init ();
With the constructor, you don't have to remember and you don't
have to type more. If you have a variable, you know that it is
properly initialized.
>> 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.
--
Lawrence Crowl
