[11/11] Make opt_pass and gcc::pipeline be GC-managed

This patch makes gcc::pipeline and opt_pass instances be allocated
within the GC-heap, and adds traversal hooks for GC/PCH, so that passes
can own refs to other GC-allocated objects.

gcc/
	Make opt_pass and gcc::pipeline be GC-managed, so that pass
	instances can own GC refs.

	* Makefile.in (GTFILES): Add pipeline.h and tree-pass.h.
	* context.c (gcc::context::gt_ggc_mx): Traverse passes_.
	(gcc::context::gt_pch_nx): Likewise.
	(gcc::context::gt_pch_nx):  Likewise.
	* passes.c (opt_pass::operator new): New.
	(opt_pass::gt_ggc_mx): New.
	(opt_pass::gt_pch_nx): New.
	(opt_pass::gt_pch_nx_with_op): New.
	(gt_ggc_mx (opt_pass *)): New.
	(gt_pch_nx (opt_pass *)): New.
	(gt_pch_nx_opt_pass): New.
	(pipeline::operator new): New.
	(pipeline::gt_ggc_mx): New.
	(pipeline::gt_pch_nx): New.
	(pipeline::gt_pch_nx_with_op): New.
	(gt_ggc_mx (pipeline *)): New.
	(gt_pch_nx (pipeline *)): New.
	(gt_pch_nx_pipeline): New.
	* pipeline.h (class pipeline): Add GTY((user)) marking.
	(pipeline::operator new): New.
	(pipeline::gt_ggc_mx): New.
	(pipeline::gt_pch_nx): New.
	(pipeline::gt_pch_nx_with_op): New.
	(gt_ggc_mx (pipeline *)): New.
	(gt_pch_nx (pipeline *)): New.
	(gt_pch_nx_pipeline): New.
	* tree-pass.h (class opt_pass): Add GTY((user)) marking.
	(opt_pass::operator new): New.
	(opt_pass::gt_ggc_mx): New.
	(opt_pass::gt_pch_nx): New.
	(opt_pass::gt_pch_nx_with_op): New.
	(gt_ggc_mx (opt_pass *)): New.
	(gt_pch_nx (opt_pass *)): New.
	(gt_pch_nx_opt_pass): New.
---
 gcc/Makefile.in |   2 +
 gcc/context.c   |   9 ++--
 gcc/passes.c    | 160 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gcc/pipeline.h  |  15 +++++-
 gcc/tree-pass.h |  19 ++++++-
 5 files changed, 198 insertions(+), 7 deletions(-)

On 26 July 2013 17:04:41 David Malcolm <dmalcolm@redhat.com> wrote:


> diff --git a/gcc/passes.c b/gcc/passes.c
> index ce5cdeb..dd1b0ba 100644
> --- a/gcc/passes.c
> +++ b/gcc/passes.c

> +
> +void gt_pch_nx_pipeline (void *this_obj, void *p,
> +			 gt_pointer_operator op, void *cookie)
> +{
> +  pipeline *passes = (pipeline *)p;
> +  if (p == this_obj)
> +    passes->gt_pch_nx_with_op (op, cookie);
> +}
> +
>
>  /* Call from anywhere to find out what pass this is.  Useful for
>     printing out debugging information deep inside an service

s/an service/a service/;# separate patch

>
> diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
> index 41d5d92..c3e89d4 100644
> --- a/gcc/tree-pass.h
> +++ b/gcc/tree-pass.h
> @@ -76,11 +76,22 @@ namespace gcc
>
>  /* An instance of a pass.  This is also "pass_data" to minimize the
>     changes in existing code.  */
> -class opt_pass : public pass_data
> +class GTY((user)) opt_pass : public pass_data
>  {
>  public:
> +  /* Ensure that instances are allocated in the GC-managed heap.  */
> +  void *operator new (size_t sz);
> +
>    virtual ~opt_pass () { }
>
> +  /* GTY((user)) methods, to be called once per traversal.
> +     opt_pass subclasses with additional GC-managed data should overide

s/ overide/ override/

Not a review but sounds goodish.


Sent with AquaMail for Android
http://www.aqua-mail.com

On 07/26/2013 05:04 AM, David Malcolm wrote:
> 	(opt_pass::gt_ggc_mx): New.
> 	(opt_pass::gt_pch_nx): New.
> 	(opt_pass::gt_pch_nx_with_op): New.
> 	(gt_ggc_mx (opt_pass *)): New.
> 	(gt_pch_nx (opt_pass *)): New.
> 	(gt_pch_nx_opt_pass): New.
> 	(pipeline::operator new): New.
> 	(pipeline::gt_ggc_mx): New.
> 	(pipeline::gt_pch_nx): New.
> 	(pipeline::gt_pch_nx_with_op): New.
> 	(gt_ggc_mx (pipeline *)): New.
> 	(gt_pch_nx (pipeline *)): New.
> 	(gt_pch_nx_pipeline): New.

I guess my previous comments about ::gt_ggc_mx vs class::gt_ggc_mx wrt
the context structure apply as well to this patch.


r~

On Thu, 2013-08-01 at 11:45 -1000, Richard Henderson wrote:
> On 07/26/2013 05:04 AM, David Malcolm wrote:
> > 	(opt_pass::gt_ggc_mx): New.
> > 	(opt_pass::gt_pch_nx): New.
> > 	(opt_pass::gt_pch_nx_with_op): New.
> > 	(gt_ggc_mx (opt_pass *)): New.
> > 	(gt_pch_nx (opt_pass *)): New.
> > 	(gt_pch_nx_opt_pass): New.
> > 	(pipeline::operator new): New.
> > 	(pipeline::gt_ggc_mx): New.
> > 	(pipeline::gt_pch_nx): New.
> > 	(pipeline::gt_pch_nx_with_op): New.
> > 	(gt_ggc_mx (pipeline *)): New.
> > 	(gt_pch_nx (pipeline *)): New.
> > 	(gt_pch_nx_pipeline): New.
> 
> I guess my previous comments about ::gt_ggc_mx vs class::gt_ggc_mx wrt
> the context structure apply as well to this patch.

For context.h, the global functions are used by autogenerated code in
gtype-desc.c, which calls them the first time the context is visited in
a gc or pch traversal; they merely call into the class, to avoid needing
friend decls.

For opt_pass and pass_manager, something different is going on.

For some reason gengtype doesn't generate the triad of gt_ggc_mx_FOO,
gt_pch_nx_FOO, gt_pch_p_NFOO functions in gtype-desc.c, for types
FOO=opt_pass and pass_manager.  Presumably this is because the types are
only visited by code in context.c

So the global functions for opt_pass and pass_manager are a hand-written
implementation of what gengtype would write; they are called *each time*
the entity is reached during a traversal.  The member functions are
called only the *first time* the entity is visited.

Does this need a descriptive comment in the source code?

Thanks
Dave

On 08/02/2013 09:08 AM, David Malcolm wrote:
> For opt_pass and pass_manager, something different is going on.

I'd wondered about that.

> For some reason gengtype doesn't generate the triad of gt_ggc_mx_FOO,
> gt_pch_nx_FOO, gt_pch_p_NFOO functions in gtype-desc.c, for types
> FOO=opt_pass and pass_manager.  Presumably this is because the types are
> only visited by code in context.c
> 
> So the global functions for opt_pass and pass_manager are a hand-written
> implementation of what gengtype would write; they are called *each time*
> the entity is reached during a traversal.  The member functions are
> called only the *first time* the entity is visited.

I wonder if we can reduce the amount of boiler-plate for this.  Perhaps,

-------------------------------------------------------------------
//
// These templates assume the presence of several member functions.
//

template<class T>
inline void gt_ggc_mx (T *p)
{
  if (ggc_test_and_set_mark (p))
    p->gt_ggc_mx ();
}

template<class T>
void gt_pch_nx_with_obj(void *this_obj, void *p,
                        gt_pointer_operator op, void *cookie)
{
  if (p == this_obj)
    {
      T *t = static_cast<T *>(p);
      t->gt_pch_nx_with_obj (op, cookie);
    }
}

template<class T>
inline void gt_pch_nx (T *p)
{
  if (gt_pch_note_object (p, p, gt_pch_nx_with_obj<T>))
    p->gt_pch_nx ();
}

---------------------------------------------------------------------

I had thought about an abstract base class instead of the templates, but
that would unnecessarily force the use of vtables in places that don't
need them.  In some cases this would be relatively harmless (e.g. the
very few pass_manager objects), but in others it might be a no-go.

The use of the template obviates that.  It ought only be instantiated when
necessary for gty user objects that don't have their own specialization.

Thoughts?


r~

On Fri, 2013-08-02 at 10:01 -1000, Richard Henderson wrote:
> On 08/02/2013 09:08 AM, David Malcolm wrote:
> > For opt_pass and pass_manager, something different is going on.
> 
> I'd wondered about that.
> 
> > For some reason gengtype doesn't generate the triad of gt_ggc_mx_FOO,
> > gt_pch_nx_FOO, gt_pch_p_NFOO functions in gtype-desc.c, for types
> > FOO=opt_pass and pass_manager.  Presumably this is because the types are
> > only visited by code in context.c
> > 
> > So the global functions for opt_pass and pass_manager are a hand-written
> > implementation of what gengtype would write; they are called *each time*
> > the entity is reached during a traversal.  The member functions are
> > called only the *first time* the entity is visited.
> 
> I wonder if we can reduce the amount of boiler-plate for this.  Perhaps,
> 
> -------------------------------------------------------------------
> //
> // These templates assume the presence of several member functions.
> //
> 
> template<class T>
> inline void gt_ggc_mx (T *p)
> {
>   if (ggc_test_and_set_mark (p))
>     p->gt_ggc_mx ();
> }
> 
> template<class T>
> void gt_pch_nx_with_obj(void *this_obj, void *p,
>                         gt_pointer_operator op, void *cookie)
> {
>   if (p == this_obj)
>     {
>       T *t = static_cast<T *>(p);
>       t->gt_pch_nx_with_obj (op, cookie);
>     }
> }
> 
> template<class T>
> inline void gt_pch_nx (T *p)
> {
>   if (gt_pch_note_object (p, p, gt_pch_nx_with_obj<T>))
>     p->gt_pch_nx ();
> }
> 
> ---------------------------------------------------------------------
> 
> I had thought about an abstract base class instead of the templates, but
> that would unnecessarily force the use of vtables in places that don't
> need them.  In some cases this would be relatively harmless (e.g. the
> very few pass_manager objects), but in others it might be a no-go.
> 
> The use of the template obviates that.  It ought only be instantiated when
> necessary for gty user objects that don't have their own specialization.

I like this approach, and I'm trying a bootstrap now with the templates.
I changed the "with_obj" to "with_op", on the grounds that you always
have an obj, but you don't always have an op.

FWIW I had a go at avoiding templates by attempting to tell gengtype to
write out functions for all GTY((user)) types, regardless of whether it
thinks they're referenced, with this:
@@ -3697,7 +3697,8 @@ write_types (outf_p output_header, type_p structures, type_p param_structs,
   /* At last we emit the functions code.  */
   oprintf (output_header, "\n/* functions code */\n");
   for (s = structures; s; s = s->next)
-    if (s->gc_used == GC_POINTED_TO || s->gc_used == GC_MAYBE_POINTED_TO)
+    if (s->gc_used == GC_POINTED_TO || s->gc_used == GC_MAYBE_POINTED_TO
+        || s->kind == TYPE_USER_STRUCT)
       {
        options_p opt;
 
but I ran into various "incomplete structure" errors due to gengtype's
lack of a full C/C++ parser.

Hence templates seem the sanest option.

On 08/02/2013 11:53 AM, David Malcolm wrote:
> FWIW I had a go at avoiding templates by attempting to tell gengtype to
> write out functions for all GTY((user)) types, regardless of whether it
> thinks they're referenced, with this:
> @@ -3697,7 +3697,8 @@ write_types (outf_p output_header, type_p structures, type_p param_structs,
>    /* At last we emit the functions code.  */
>    oprintf (output_header, "\n/* functions code */\n");
>    for (s = structures; s; s = s->next)
> -    if (s->gc_used == GC_POINTED_TO || s->gc_used == GC_MAYBE_POINTED_TO)
> +    if (s->gc_used == GC_POINTED_TO || s->gc_used == GC_MAYBE_POINTED_TO
> +        || s->kind == TYPE_USER_STRUCT)
>        {
>         options_p opt;
>  
> but I ran into various "incomplete structure" errors due to gengtype's
> lack of a full C/C++ parser.
> 
> Hence templates seem the sanest option.

I was actually contemplating going the other direction -- have gengtype
write out _less_, relying on the templates more.  I hadn't gone very far
down that road yet...


r~

On Sat, Aug 3, 2013 at 2:21 AM, Richard Henderson <rth@redhat.com> wrote:
> On 08/02/2013 11:53 AM, David Malcolm wrote:
>> FWIW I had a go at avoiding templates by attempting to tell gengtype to
>> write out functions for all GTY((user)) types, regardless of whether it
>> thinks they're referenced, with this:
>> @@ -3697,7 +3697,8 @@ write_types (outf_p output_header, type_p structures, type_p param_structs,
>>    /* At last we emit the functions code.  */
>>    oprintf (output_header, "\n/* functions code */\n");
>>    for (s = structures; s; s = s->next)
>> -    if (s->gc_used == GC_POINTED_TO || s->gc_used == GC_MAYBE_POINTED_TO)
>> +    if (s->gc_used == GC_POINTED_TO || s->gc_used == GC_MAYBE_POINTED_TO
>> +        || s->kind == TYPE_USER_STRUCT)
>>        {
>>         options_p opt;
>>
>> but I ran into various "incomplete structure" errors due to gengtype's
>> lack of a full C/C++ parser.
>>
>> Hence templates seem the sanest option.
>
> I was actually contemplating going the other direction -- have gengtype
> write out _less_, relying on the templates more.  I hadn't gone very far
> down that road yet...

That was the original idea, btw (well, the original idea was to get rid of
gengtype completely of course).

Btw, I don't like moving the pass pipeline and pass objects into GC space.
It doesn't make much sense as pass execution is quite constrained so
passes should not allocate objects that extend the passes lifetime.

Richard.