Re: [PATCH] Inline even functions containing static variables (was Re: Removing space waste for g++ static inlined objects)

[Jason Merrill <jason at redhat dot com>]

>>>>> "Jakub" == Jakub Jelinek <jakub@redhat.com> writes:

> Here is what I hacked up yesterday and today.

Thanks!

> There are 2 things unclear to me in
> http://reality.sgi.com/dehnert_engr/cxx/abi.html#mangling

>> structure. The discriminator is used only for the second and later
>> occurrences of the same name within a single function.  In this case
>> <number> is n - 2, if this is the nth occurrence, in lexical order, of
>> the given name.

> though:

> 1) gcc uses no discriminator for 1st occurrence, _ for 2nd occurrence, _0
> for 3rd occurrence and onwards, while the above paragraph (at least in my
> reading suggests that no discriminator for 1st occurrence, _0 for 2nd
> occurrence, _1 for 3rd occurrence (at least my understanding is that n in
> above text is 1 based, not 0 based; speaking in native language about
> ``second'' occurrence I'd really think it is 1 based, Alex?).

I agree, this is a discrepancy.  Please fix the compiler to reflect the
spec.  Unless Alex feels differently?

I think we can assume that 'nth' is 1 based.

> 2) it is also not really clear to me whether local static objects and
> local classes are discriminated together or whether they are
> discriminated separately.

I agree, this should be clarified in the spec.  But since it's fine for a
global object and class to have the same name, I would think local objects
and classes with the same name should be discriminated separately.

> I had to tweak maybe_commonize_var, because if we are creating a guard
> variable before maybe_commonize_var is called (which we do in reference
> initialization code), the guard needs to get the same linkage as the
> variable it is guarding.

No, it doesn't.  The guard variable should always be common.  Your change
seems unnecessary.

> The patch also cleans up DECL_INLINED_FNS handling to use a TREE_VEC
> instead of custom data structure. It is done in the same patch, since
> this patch is moving DECL_INLINED_FNS anyway (so that it does not eat any
> more space in lang_decl structure).

I don't think DECL_LOCAL_NAMES belongs in the decl anyway; it should go in
struct cp_language_function, with other things that only matter during
compilation.  Ditto for local_classes; the current handling of
local_classes is very wrong, as it is never cleared.

> +	  /* Rather than try to get this right with inlining, we suppress
> +	     inlining of such functions.  */
> +	  current_function_cannot_inline
> +	    = "function with static variable cannot be inline";
> +	  DECL_UNINLINABLE (current_function_decl) = 1;

I don't think this is necessary once we're mangling things properly.  Let's
take it out and see if anything breaks.

> @@ -8427,6 +8511,8 @@ expand_static_init (decl, init)
>  	 threads should not be able to initialize the variable more
>  	 than once.  We don't yet attempt to ensure thread-safety.  */
 
> +      push_local_name (decl);

Why do you push both here and in maybe_commonize_var?  The latter seems the
appropriate place.

Jason