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

[Jakub Jelinek <jakub at redhat dot com>]

On Fri, Mar 16, 2001 at 02:16:17PM +0000, Jason Merrill wrote:
> I agree, this is a discrepancy.  Please fix the compiler to reflect the
> spec.  Unless Alex feels differently?

Ok, will do in a separate patch.

> 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.

Without that change it was always
        .local  _ZGVZ4mainE1a
        .comm   _ZGVZ4mainE1a,8,8
and now if the function is inline, it will be only .comm _ZGVZ4mainE1a,8,8.
get_guard has:
      /* The guard should have the same linkage as what it guards. */
      TREE_PUBLIC (guard) = TREE_PUBLIC (decl);
      TREE_STATIC (guard) = TREE_STATIC (decl);
      DECL_COMMON (guard) = DECL_COMMON (decl);
      DECL_ONE_ONLY (guard) = DECL_ONE_ONLY (decl);
      if (TREE_PUBLIC (decl))
        DECL_WEAK (guard) = DECL_WEAK (decl);
Should this be changed to unconditionally force it to TREE_PUBLIC +
TREE_STATIC + DECL_COMMON? What about static functions? Then the guard
variable surely cannot be common, right?

(This is what I was playing with for guard variables:
class foo { char buffer[1024]; public: foo(); };
int main()
{
  static foo& a = *(new foo);
  static foo& b = *(new foo);
  {
    static foo& a = *(new foo);
  }
}
).

> > 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.

The problem with cp_language_function is that it goes away too soon.
With inlining on trees, a function can be inlined even after it has gone
through genrtl_start_function, which kills cp_language_function data.
If genrtl_start_function sets DECL_UNINLINEABLE, then yes, DECL_LOCAL_NAMES
can go into cp_language_function and DECL_INLINED_FNS can go away completely
(inlining will be limited by disallowing inlining if rtl has been emitted).
I can certainly kill local_classes altogether and either record it in the
same varray/TREE_VEC as DECL_LOCAL_NAMES or in a separate varray/TREE_VEC,
it will make things faster since only local classes of the current function
will have to be compared.

> 
> > +	  /* 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.

I think you're right, will take it out.

> 
> > @@ -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.

Because get_guard when mangling the guard variable needs to discriminate
among local static variables of the same name. maybe_commonize_var calls
push_local_name only in inline functions (to save memory), while guard
variables can be created in other routines as well.

	Jakub