Updated patch (was Re: [PATCH 11/11] Make opt_pass and gcc::pipeline be GC-managed)

[David Malcolm <dmalcolm at redhat dot com>]

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.

Thanks.

Here's an updated version of the patch which uses your template idea,
removing the handcoded functions.

I put the templates in ggc.h, with a big comment.  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. 

I stepped through a collection, and observed gcc::context::gt_ggc_mx
calling gt_ggc_mx<gcc::pass_manager>, which then called
gt_ggc_mx<opt_pass>.

I've successfully bootstrapped the *end result* of the revised patch
series 3-11 (plus the patch "3.1" from [1], which upsets the numbering)
on x86_64-unknown-linux-gnu, and ran the testsuite: all testcases show
the same results as an unpatched build (relative to r201397).

OK for trunk?

[1] http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00118.html

>From 2e3002e597eb7cfdbfb274ede7c02c0510e35e70 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Fri, 2 Aug 2013 15:58:46 -0400
Subject: [PATCH 12/15] Make opt_pass and gcc::pass_manager be GC-managed

This patch makes gcc::pass_manager 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.

It also adds templates to ggc.h, to create boilerplate for GTY((user))
types that gengtype can't handle.

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

	* Makefile.in (GTFILES): Add pass_manager.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.
	* ggc.h (gt_ggc_mx <T>): New.
	(gt_pch_nx_with_op <T>): New.
	(gt_pch_nx <T>): New.
	* passes.c (opt_pass::gt_ggc_mx): New.
	(opt_pass::gt_pch_nx): New.
	(opt_pass::gt_pch_nx_with_op): New.
	(pass_manager::gt_ggc_mx): New.
	(pass_manager::gt_pch_nx): New.
	(pass_manager::gt_pch_nx_with_op): New.
	(pass_manager::operator new): Use
	ggc_internal_cleared_alloc_stat rather than xcalloc.
	* pass_manager.h (class pass_manager): Add GTY((user)) marking.
	(pass_manager::gt_ggc_mx): New.
	(pass_manager::gt_pch_nx): New.
	(pass_manager::gt_pch_nx_with_op): 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.
---
 gcc/Makefile.in    |   2 +
 gcc/context.c      |   6 +--
 gcc/ggc.h          |  46 +++++++++++++++++++++
 gcc/pass_manager.h |   9 ++++-
 gcc/passes.c       | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 gcc/tree-pass.h    |  13 +++++-
 6 files changed, 184 insertions(+), 8 deletions(-)

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 61a4d7c..df24bdc 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -3820,6 +3820,8 @@ GTFILES = $(CPP_ID_DATA_H) $(srcdir)/input.h $(srcdir)/coretypes.h \
   $(srcdir)/asan.c \
   $(srcdir)/tsan.c \
   $(srcdir)/context.h \
+  $(srcdir)/pass_manager.h \
+  $(srcdir)/tree-pass.h \
   @all_gtfiles@
 
 # Compute the list of GT header files from the corresponding C sources,
diff --git a/gcc/context.c b/gcc/context.c
index ba6f335..698cc57 100644
--- a/gcc/context.c
+++ b/gcc/context.c
@@ -42,18 +42,18 @@ gcc::context::context()
 void
 gcc::context::gt_ggc_mx ()
 {
-  /* Currently a no-op.  */
+  ::gt_ggc_mx (passes_);
 }
 
 void
 gcc::context::gt_pch_nx ()
 {
-  /* Currently a no-op.  */
+  ::gt_pch_nx (passes_);
 }
 
 void
 gcc::context::gt_pch_nx (gt_pointer_operator op ATTRIBUTE_UNUSED,
 			 void *cookie ATTRIBUTE_UNUSED)
 {
-  /* Currently a no-op.  */
+  op (&passes_, cookie);
 }
diff --git a/gcc/ggc.h b/gcc/ggc.h
index b31bc80..e2a1aaf 100644
--- a/gcc/ggc.h
+++ b/gcc/ggc.h
@@ -276,4 +276,50 @@ ggc_alloc_cleared_gimple_statement_d_stat (size_t s MEM_STAT_DECL)
     ggc_internal_cleared_alloc_stat (s PASS_MEM_STAT);
 }
 
+/* gengtype will autogenerate traversal functions (in gtype-desc.c) for
+   all GTY-marked types that it sees are referenced by a GTY marker.
+
+   Unfortunately, it will not generate traveral functions for types that
+   are only referenced by GTY((user)) types.
+
+   The following templates are a substitute, providing equivalent
+   traversal functions for such types.  They are instantiated for
+   types whose objects that are traversed during GC/PCH, and are
+   called *every time* that an instance of type T is traversed during
+   GC/PCH.
+
+   They require the presence of the following member functions
+
+     void gt_ggc_mx ();
+     void gt_pch_nx ();
+     void gt_pch_nx_with_op (gt_pointer_operator op, void *cookie);
+
+   within class T, which are called *once* per object - the first
+   time the object is visited during the traversal.  */
+
+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_op (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_op (op, cookie);
+    }
+}
+
+template<class T>
+inline void gt_pch_nx (T *p)
+{
+  if (gt_pch_note_object (p, p, gt_pch_nx_with_op<T>))
+    p->gt_pch_nx ();
+}
+
 #endif
diff --git a/gcc/pass_manager.h b/gcc/pass_manager.h
index 00f0b1c..c861893 100644
--- a/gcc/pass_manager.h
+++ b/gcc/pass_manager.h
@@ -44,13 +44,19 @@ namespace gcc {
 
 class context;
 
-class pass_manager
+class GTY((user)) pass_manager
 {
 public:
+  /* Ensure that instances are allocated in the GC-managed heap.  */
   void *operator new (size_t sz);
 
   pass_manager(context *ctxt);
 
+  /* GTY((user)) methods.  */
+  void gt_ggc_mx ();
+  void gt_pch_nx ();
+  void gt_pch_nx_with_op (gt_pointer_operator op, void *cookie);
+
   void register_pass (struct register_pass_info *pass_info);
   void register_one_dump_file (struct opt_pass *pass);
 
@@ -125,4 +131,3 @@ private:
 } // namespace gcc
 
 #endif /* ! GCC_PASS_MANAGER_H */
-
diff --git a/gcc/passes.c b/gcc/passes.c
index aa273fb..db6fc3b 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -82,6 +82,33 @@ struct opt_pass *current_pass;
 
 static void register_pass_name (struct opt_pass *, const char *);
 
+void*
+opt_pass::operator new (size_t sz)
+{
+  return ggc_internal_cleared_alloc_stat (sz MEM_STAT_INFO);
+}
+
+void opt_pass::gt_ggc_mx ()
+{
+  ::gt_ggc_mx (ctxt_);
+  ::gt_ggc_mx (sub);
+  ::gt_ggc_mx (next);
+}
+
+void opt_pass::gt_pch_nx ()
+{
+  ::gt_pch_nx (ctxt_);
+  ::gt_pch_nx (sub);
+  ::gt_pch_nx (next);
+}
+
+void opt_pass::gt_pch_nx_with_op (gt_pointer_operator op, void *cookie)
+{
+  op (&(ctxt_), cookie);
+  op (&(sub), cookie);
+  op (&(next), cookie);
+}
+
 /* Most passes are single-instance (within their context) and thus don't
    need to implement cloning, but passes that support multiple instances
    *must* provide their own implementation of the clone method.
@@ -116,6 +143,92 @@ opt_pass::opt_pass(const pass_data &data, context *ctxt)
 {
 }
 
+void
+pass_manager::gt_ggc_mx ()
+{
+  ::gt_ggc_mx (all_passes);
+  ::gt_ggc_mx (all_small_ipa_passes);
+  ::gt_ggc_mx (all_lowering_passes);
+  ::gt_ggc_mx (all_regular_ipa_passes);
+  ::gt_ggc_mx (all_lto_gen_passes);
+  ::gt_ggc_mx (all_late_ipa_passes);
+
+  for (int i = 0; i < passes_by_id_size; i++)
+    ::gt_ggc_mx (passes_by_id[i]);
+
+#define INSERT_PASSES_AFTER(PASS)
+#define PUSH_INSERT_PASSES_WITHIN(PASS)
+#define POP_INSERT_PASSES()
+#define NEXT_PASS(PASS, NUM) ::gt_ggc_mx (PASS ## _ ## NUM);
+#define TERMINATE_PASS_LIST()
+
+#include "pass-instances.def"
+
+#undef INSERT_PASSES_AFTER
+#undef PUSH_INSERT_PASSES_WITHIN
+#undef POP_INSERT_PASSES
+#undef NEXT_PASS
+#undef TERMINATE_PASS_LIST
+
+}
+
+void
+pass_manager::gt_pch_nx ()
+{
+  ::gt_pch_nx (all_passes);
+  ::gt_pch_nx (all_small_ipa_passes);
+  ::gt_pch_nx (all_lowering_passes);
+  ::gt_pch_nx (all_regular_ipa_passes);
+  ::gt_pch_nx (all_lto_gen_passes);
+  ::gt_pch_nx (all_late_ipa_passes);
+
+  for (int i = 0; i < passes_by_id_size; i++)
+    ::gt_pch_nx (passes_by_id[i]);
+
+#define INSERT_PASSES_AFTER(PASS)
+#define PUSH_INSERT_PASSES_WITHIN(PASS)
+#define POP_INSERT_PASSES()
+#define NEXT_PASS(PASS, NUM) ::gt_pch_nx (PASS ## _ ## NUM);
+#define TERMINATE_PASS_LIST()
+
+#include "pass-instances.def"
+
+#undef INSERT_PASSES_AFTER
+#undef PUSH_INSERT_PASSES_WITHIN
+#undef POP_INSERT_PASSES
+#undef NEXT_PASS
+#undef TERMINATE_PASS_LIST
+
+}
+
+void
+pass_manager::gt_pch_nx_with_op (gt_pointer_operator op, void *cookie)
+{
+  op (&(all_passes), cookie);
+  op (&(all_small_ipa_passes), cookie);
+  op (&(all_lowering_passes), cookie);
+  op (&(all_regular_ipa_passes), cookie);
+  op (&(all_lto_gen_passes), cookie);
+  op (&(all_late_ipa_passes), cookie);
+
+  for (int i = 0; i < passes_by_id_size; i++)
+    op (&(passes_by_id[i]), cookie);
+
+#define INSERT_PASSES_AFTER(PASS)
+#define PUSH_INSERT_PASSES_WITHIN(PASS)
+#define POP_INSERT_PASSES()
+#define NEXT_PASS(PASS, NUM) op (&(PASS ## _ ## NUM), cookie);
+#define TERMINATE_PASS_LIST()
+
+#include "pass-instances.def"
+
+#undef INSERT_PASSES_AFTER
+#undef PUSH_INSERT_PASSES_WITHIN
+#undef POP_INSERT_PASSES
+#undef NEXT_PASS
+#undef TERMINATE_PASS_LIST
+
+}
 
 void
 pass_manager::execute_early_local_passes ()
@@ -129,7 +242,6 @@ pass_manager::execute_pass_mode_switching ()
   return pass_mode_switching_1->execute ();
 }
 
-
 /* Call from anywhere to find out what pass this is.  Useful for
    printing out debugging information deep inside an service
    routine.  */
@@ -1464,7 +1576,7 @@ void *
 pass_manager::operator new (size_t sz)
 {
   /* Ensure that all fields of the pass manager are zero-initialized.  */
-  return xcalloc (1, sz);
+  return ggc_internal_cleared_alloc_stat (sz MEM_STAT_INFO);
 }
 
 pass_manager::pass_manager (context *ctxt)
diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
index 41d5d92..063b166 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
+     these, chain up to the base class implementation, then walk their
+     extra fields.  */
+  virtual void gt_ggc_mx ();
+  virtual void gt_pch_nx ();
+  virtual void gt_pch_nx_with_op (gt_pointer_operator op, void *cookie);
+
   /* Create a copy of this pass.
 
      Passes that can have multiple instances must provide their own
-- 
1.7.11.7