public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 1/6] Convert gimple types from a union to a C++ class hierarchy
  2013-08-29 16:20 [PATCH 0/6] Convert gimple to a C++ class hierarchy David Malcolm
                   ` (2 preceding siblings ...)
  2013-08-29 16:20 ` [PATCH 5/6] Port various places from union access to subclass access David Malcolm
@ 2013-08-29 16:20 ` David Malcolm
  2013-08-29 16:20 ` [PATCH 3/6] Autogenerated conversion of gimple to C++ David Malcolm
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 44+ messages in thread
From: David Malcolm @ 2013-08-29 16:20 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

	* coretypes.h (union gimple_statement_d): Remove declaration.
	(gimple): Convert from being a "union gimple_statement_d *"
	to a "struct gimple_statement_base *".
	(const_gimple): Likewise (with "const").
	* ggc.h (ggc_alloc_cleared_gimple_statement_d_stat): Replace
	with...
	(ggc_alloc_cleared_gimple_statement_stat): ...this.
	* gimple-pretty-print.c (debug): Change parameter from a
	"gimple_statement_d &" to a "gimple_statement_base &".
	(debug): Change parameter from a "gimple_statement_d *" to
	a "gimple_statement_base *".
	* gimple-pretty-print.h (debug): Update declarations as above.
	* gimple.c (gimple_alloc_stat): Update for renaming of
	ggc_alloc_cleared_gimple_statement_d_stat to
	ggc_alloc_cleared_gimple_statement_stat.
	* Makefile.in (GIMPLE_H): Add dep on is-a.h.
	* gimple.h: Include "is-a.h"
	(struct gimple statement_base): Convert to GTY((user)).
	(gimple_statement_with_ops_base): Convert to a subclass of
	gimple_statement_base, dropping initial "gsbase" field.
	(gimple_statement_with_ops): Convert to a subclass of
	gimple_statement_with_ops_base, dropping initial "opbase" field.
	(gimple_statement_with_memory_ops_base): Likewise.
	(gimple_statement_with_memory_ops): Convert to a subclass of
	public gimple_statement_with_memory_ops_base, dropping initial
	"membase" field.
	(gimple_statement_call): Likewise.
	(gimple_statement_omp): Convert to a subclass of
	gimple_statement_base, dropping initial "gsbase" field.
	(gimple_statement_bind): Likewise.
	(gimple_statement_catch): Likewise.
	(gimple_statement_eh_filter): Likewise.
	(gimple_statement_eh_else): Likewise.
	(gimple_statement_eh_mnt): Likewise.
	(gimple_statement_phi): Likewise.
	(gimple_statement_eh_ctrl): Likewise.
	(gimple_statement_try): Likewise.
	(gimple_statement_wce): Likewise.
	(gimple_statement_asm): Convert to a subclass of
	gimple_statement_with_memory_ops_base, dropping initial
	"membase" field.
	(gimple_statement_omp_critical): Convert to a subclass of
	gimple_statement_omp, dropping initial "omp" field.
	(gimple_statement_omp_for): Likewise.
	(gimple_statement_omp_parallel): Likewise.
	(gimple_statement_omp_task): Convert to a subclass of
	gimple_statement_omp_parallel, dropping initial "par" field.
	(gimple_statement_omp_sections): Convert to a subclass of
	gimple_statement_omp, dropping initial "omp" field.
	(gimple_statement_omp_continue): Convert to a subclass of
	gimple_statement_base, dropping initial "gsbase" field.
	(gimple_statement_omp_single): Convert to a subclass of
	gimple_statement_omp, dropping initial "omp" field.
	(gimple_statement_omp_atomic_load): Convert to a subclass of
	gimple_statement_base, dropping initial "gsbase" field.
	(gimple_statement_omp_atomic_store): Convert to a subclass of
	gimple_statement_base, dropping initial "gsbase" field.
	(gimple_statement_transaction): Convert to a subclass of
	gimple_statement_with_memory_ops_base, dropping initial "gsbase"
	field.
	(union gimple_statement_d): Remove.
	* system.h (CONST_CAST_GIMPLE): Update to use
	"struct gimple_statement_base *" rather than
	"union gimple_statement_d *".
	* tree-ssa-ccp.c (gimple_htab): Convert underlying type from
	gimple_statement_d to gimple_statement_base.
---
 gcc/Makefile.in           |   2 +-
 gcc/coretypes.h           |   5 +-
 gcc/ggc.h                 |   6 +-
 gcc/gimple-pretty-print.c |   4 +-
 gcc/gimple-pretty-print.h |   4 +-
 gcc/gimple.c              |   2 +-
 gcc/gimple.h              | 183 ++++++++++++++++++----------------------------
 gcc/system.h              |   2 +-
 gcc/tree-ssa-ccp.c        |   2 +-
 9 files changed, 84 insertions(+), 126 deletions(-)

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 387b60f..321c591 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -882,7 +882,7 @@ BASIC_BLOCK_H = basic-block.h $(PREDICT_H) $(VEC_H) $(FUNCTION_H) \
 	cfg-flags.def cfghooks.h
 GIMPLE_H = gimple.h gimple.def gsstruct.def pointer-set.h $(VEC_H) \
 	$(GGC_H) $(BASIC_BLOCK_H) $(TREE_H) tree-ssa-operands.h \
-	tree-ssa-alias.h $(INTERNAL_FN_H) $(HASH_TABLE_H)
+	tree-ssa-alias.h $(INTERNAL_FN_H) $(HASH_TABLE_H) is-a.h
 TRANS_MEM_H = trans-mem.h
 GCOV_IO_H = gcov-io.h gcov-iov.h auto-host.h
 COVERAGE_H = coverage.h $(GCOV_IO_H)
diff --git a/gcc/coretypes.h b/gcc/coretypes.h
index bff8f5c..5ced7cb 100644
--- a/gcc/coretypes.h
+++ b/gcc/coretypes.h
@@ -58,9 +58,8 @@ typedef const struct rtvec_def *const_rtvec;
 union tree_node;
 typedef union tree_node *tree;
 typedef const union tree_node *const_tree;
-union gimple_statement_d;
-typedef union gimple_statement_d *gimple;
-typedef const union gimple_statement_d *const_gimple;
+typedef struct gimple_statement_base *gimple;
+typedef const struct gimple_statement_base *const_gimple;
 typedef gimple gimple_seq;
 struct gimple_stmt_iterator_d;
 typedef struct gimple_stmt_iterator_d gimple_stmt_iterator;
diff --git a/gcc/ggc.h b/gcc/ggc.h
index b31bc80..bb8f939 100644
--- a/gcc/ggc.h
+++ b/gcc/ggc.h
@@ -269,10 +269,10 @@ ggc_alloc_cleared_tree_node_stat (size_t s MEM_STAT_DECL)
   return (union tree_node *) ggc_internal_cleared_alloc_stat (s PASS_MEM_STAT);
 }
 
-static inline union gimple_statement_d *
-ggc_alloc_cleared_gimple_statement_d_stat (size_t s MEM_STAT_DECL)
+static inline struct gimple_statement_base *
+ggc_alloc_cleared_gimple_statement_stat (size_t s MEM_STAT_DECL)
 {
-  return (union gimple_statement_d *)
+  return (struct gimple_statement_base *)
     ggc_internal_cleared_alloc_stat (s PASS_MEM_STAT);
 }
 
diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c
index 3ab558c..8fed925 100644
--- a/gcc/gimple-pretty-print.c
+++ b/gcc/gimple-pretty-print.c
@@ -82,13 +82,13 @@ print_gimple_stmt (FILE *file, gimple g, int spc, int flags)
 }
 
 DEBUG_FUNCTION void
-debug (gimple_statement_d &ref)
+debug (gimple_statement_base &ref)
 {
   print_gimple_stmt (stderr, &ref, 0, 0);
 }
 
 DEBUG_FUNCTION void
-debug (gimple_statement_d *ptr)
+debug (gimple_statement_base *ptr)
 {
   if (ptr)
     debug (*ptr);
diff --git a/gcc/gimple-pretty-print.h b/gcc/gimple-pretty-print.h
index 2b0285d..7e7edd9 100644
--- a/gcc/gimple-pretty-print.h
+++ b/gcc/gimple-pretty-print.h
@@ -29,8 +29,8 @@ extern void debug_gimple_stmt (gimple);
 extern void debug_gimple_seq (gimple_seq);
 extern void print_gimple_seq (FILE *, gimple_seq, int, int);
 extern void print_gimple_stmt (FILE *, gimple, int, int);
-extern void debug (gimple_statement_d &ref);
-extern void debug (gimple_statement_d *ptr);
+extern void debug (gimple_statement_base &ref);
+extern void debug (gimple_statement_base *ptr);
 extern void print_gimple_expr (FILE *, gimple, int, int);
 extern void pp_gimple_stmt_1 (pretty_printer *, gimple, int, int);
 extern void gimple_dump_bb_for_graph (pretty_printer *, basic_block);
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 4dbcdda..953057c 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -131,7 +131,7 @@ gimple_alloc_stat (enum gimple_code code, unsigned num_ops MEM_STAT_DECL)
       gimple_alloc_sizes[(int) kind] += size;
     }
 
-  stmt = ggc_alloc_cleared_gimple_statement_d_stat (size PASS_MEM_STAT);
+  stmt = ggc_alloc_cleared_gimple_statement_stat (size PASS_MEM_STAT);
   gimple_set_code (stmt, code);
   gimple_set_num_ops (stmt, num_ops);
 
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 9f29561..d7ea2e4 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-ssa-operands.h"
 #include "tree-ssa-alias.h"
 #include "internal-fn.h"
+#include "is-a.h"
 
 typedef gimple gimple_seq_node;
 
@@ -159,7 +160,8 @@ struct gimple_stmt_iterator_d
 /* Data structure definitions for GIMPLE tuples.  NOTE: word markers
    are for 64 bit hosts.  */
 
-struct GTY((chain_next ("%h.next"))) gimple_statement_base {
+/* tag GSS_BASE */
+struct GTY((user)) gimple_statement_base {
   /* [ WORD 1 ]
      Main identifying code for a tuple.  */
   ENUM_BITFIELD(gimple_code) code : 8;
@@ -223,10 +225,10 @@ struct GTY((chain_next ("%h.next"))) gimple_statement_base {
 
 /* Base structure for tuples with operands.  */
 
-struct GTY(()) gimple_statement_with_ops_base
+struct GTY((user)) gimple_statement_with_ops_base : public gimple_statement_base
+
 {
-  /* [ WORD 1-6 ]  */
-  struct gimple_statement_base gsbase;
+  /* [ WORD 1-6 ] : base class */
 
   /* [ WORD 7 ]
      SSA operand vectors.  NOTE: It should be possible to
@@ -239,10 +241,10 @@ struct GTY(()) gimple_statement_with_ops_base
 
 /* Statements that take register operands.  */
 
-struct GTY(()) gimple_statement_with_ops
+/* "GSS_WITH_OPS" */
+struct GTY((user)) gimple_statement_with_ops : public gimple_statement_with_ops_base
 {
-  /* [ WORD 1-7 ]  */
-  struct gimple_statement_with_ops_base opbase;
+  /* [ WORD 1-7 ] : base class */
 
   /* [ WORD 8 ]
      Operand vector.  NOTE!  This must always be the last field
@@ -254,10 +256,10 @@ struct GTY(()) gimple_statement_with_ops
 
 /* Base for statements that take both memory and register operands.  */
 
-struct GTY(()) gimple_statement_with_memory_ops_base
+/* "GSS_WITH_MEM_OPS_BASE" */
+struct GTY((user)) gimple_statement_with_memory_ops_base : public gimple_statement_with_ops_base
 {
-  /* [ WORD 1-7 ]  */
-  struct gimple_statement_with_ops_base opbase;
+  /* [ WORD 1-7 ] : base class */
 
   /* [ WORD 8-9 ]
      Virtual operands for this statement.  The GC will pick them
@@ -269,10 +271,10 @@ struct GTY(()) gimple_statement_with_memory_ops_base
 
 /* Statements that take both memory and register operands.  */
 
-struct GTY(()) gimple_statement_with_memory_ops
+/* "GSS_WITH_MEM_OPS" */
+struct GTY((user)) gimple_statement_with_memory_ops : public gimple_statement_with_memory_ops_base
 {
-  /* [ WORD 1-9 ]  */
-  struct gimple_statement_with_memory_ops_base membase;
+  /* [ WORD 1-9 ] : base class */
 
   /* [ WORD 10 ]
      Operand vector.  NOTE!  This must always be the last field
@@ -284,10 +286,9 @@ struct GTY(()) gimple_statement_with_memory_ops
 
 /* Call statements that take both memory and register operands.  */
 
-struct GTY(()) gimple_statement_call
+struct GTY((user)) gimple_statement_call : public gimple_statement_with_memory_ops_base
 {
-  /* [ WORD 1-9 ]  */
-  struct gimple_statement_with_memory_ops_base membase;
+  /* [ WORD 1-9 ] : base class */
 
   /* [ WORD 10-13 ]  */
   struct pt_solution call_used;
@@ -309,9 +310,8 @@ struct GTY(()) gimple_statement_call
 
 /* OpenMP statements (#pragma omp).  */
 
-struct GTY(()) gimple_statement_omp {
-  /* [ WORD 1-6 ]  */
-  struct gimple_statement_base gsbase;
+struct GTY((user)) gimple_statement_omp  : public gimple_statement_base {
+  /* [ WORD 1-6 ] : base class */
 
   /* [ WORD 7 ]  */
   gimple_seq body;
@@ -320,9 +320,8 @@ struct GTY(()) gimple_statement_omp {
 
 /* GIMPLE_BIND */
 
-struct GTY(()) gimple_statement_bind {
-  /* [ WORD 1-6 ]  */
-  struct gimple_statement_base gsbase;
+struct GTY((user)) gimple_statement_bind : public gimple_statement_base {
+  /* [ WORD 1-6 ] : base class */
 
   /* [ WORD 7 ]
      Variables declared in this scope.  */
@@ -343,9 +342,8 @@ struct GTY(()) gimple_statement_bind {
 
 /* GIMPLE_CATCH */
 
-struct GTY(()) gimple_statement_catch {
-  /* [ WORD 1-6 ]  */
-  struct gimple_statement_base gsbase;
+struct GTY((user)) gimple_statement_catch : public gimple_statement_base {
+  /* [ WORD 1-6 ] : base class */
 
   /* [ WORD 7 ]  */
   tree types;
@@ -357,9 +355,8 @@ struct GTY(()) gimple_statement_catch {
 
 /* GIMPLE_EH_FILTER */
 
-struct GTY(()) gimple_statement_eh_filter {
-  /* [ WORD 1-6 ]  */
-  struct gimple_statement_base gsbase;
+struct GTY((user)) gimple_statement_eh_filter : public gimple_statement_base {
+  /* [ WORD 1-6 ] : base class */
 
   /* [ WORD 7 ]
      Filter types.  */
@@ -372,9 +369,8 @@ struct GTY(()) gimple_statement_eh_filter {
 
 /* GIMPLE_EH_ELSE */
 
-struct GTY(()) gimple_statement_eh_else {
-  /* [ WORD 1-6 ]  */
-  struct gimple_statement_base gsbase;
+struct GTY((user)) gimple_statement_eh_else : public gimple_statement_base {
+  /* [ WORD 1-6 ] : base class */
 
   /* [ WORD 7,8 ] */
   gimple_seq n_body, e_body;
@@ -382,9 +378,8 @@ struct GTY(()) gimple_statement_eh_else {
 
 /* GIMPLE_EH_MUST_NOT_THROW */
 
-struct GTY(()) gimple_statement_eh_mnt {
-  /* [ WORD 1-6 ]  */
-  struct gimple_statement_base gsbase;
+struct GTY((user)) gimple_statement_eh_mnt : public gimple_statement_base {
+  /* [ WORD 1-6 ] : base class */
 
   /* [ WORD 7 ] Abort function decl.  */
   tree fndecl;
@@ -392,9 +387,8 @@ struct GTY(()) gimple_statement_eh_mnt {
 
 /* GIMPLE_PHI */
 
-struct GTY(()) gimple_statement_phi {
-  /* [ WORD 1-6 ]  */
-  struct gimple_statement_base gsbase;
+struct GTY((user)) gimple_statement_phi : public gimple_statement_base {
+  /* [ WORD 1-6 ] : base class */
 
   /* [ WORD 7 ]  */
   unsigned capacity;
@@ -410,10 +404,9 @@ struct GTY(()) gimple_statement_phi {
 
 /* GIMPLE_RESX, GIMPLE_EH_DISPATCH */
 
-struct GTY(()) gimple_statement_eh_ctrl
+struct GTY((user)) gimple_statement_eh_ctrl : public gimple_statement_base
 {
-  /* [ WORD 1-6 ]  */
-  struct gimple_statement_base gsbase;
+  /* [ WORD 1-6 ] : base class */
 
   /* [ WORD 7 ]
      Exception region number.  */
@@ -423,9 +416,8 @@ struct GTY(()) gimple_statement_eh_ctrl
 
 /* GIMPLE_TRY */
 
-struct GTY(()) gimple_statement_try {
-  /* [ WORD 1-6 ]  */
-  struct gimple_statement_base gsbase;
+struct GTY((user)) gimple_statement_try : public gimple_statement_base {
+  /* [ WORD 1-6 ] : base class */
 
   /* [ WORD 7 ]
      Expression to evaluate.  */
@@ -452,9 +444,8 @@ enum gimple_try_flags
 
 /* GIMPLE_WITH_CLEANUP_EXPR */
 
-struct GTY(()) gimple_statement_wce {
-  /* [ WORD 1-6 ]  */
-  struct gimple_statement_base gsbase;
+struct GTY((user)) gimple_statement_wce : public gimple_statement_base {
+  /* [ WORD 1-6 ] : base class */
 
   /* Subcode: CLEANUP_EH_ONLY.  True if the cleanup should only be
 	      executed if an exception is thrown, not on normal exit of its
@@ -469,10 +460,10 @@ struct GTY(()) gimple_statement_wce {
 
 /* GIMPLE_ASM  */
 
-struct GTY(()) gimple_statement_asm
+struct GTY((user)) gimple_statement_asm
+  : public gimple_statement_with_memory_ops_base
 {
-  /* [ WORD 1-9 ]  */
-  struct gimple_statement_with_memory_ops_base membase;
+  /* [ WORD 1-9 ] : base class */
 
   /* [ WORD 10 ]
      __asm__ statement.  */
@@ -494,9 +485,10 @@ struct GTY(()) gimple_statement_asm
 
 /* GIMPLE_OMP_CRITICAL */
 
-struct GTY(()) gimple_statement_omp_critical {
-  /* [ WORD 1-7 ]  */
-  struct gimple_statement_omp omp;
+struct GTY((user)) gimple_statement_omp_critical
+  : public gimple_statement_omp
+{
+  /* [ WORD 1-7 ] : base class */
 
   /* [ WORD 8 ]
      Critical section name.  */
@@ -523,9 +515,8 @@ struct GTY(()) gimple_omp_for_iter {
 
 /* GIMPLE_OMP_FOR */
 
-struct GTY(()) gimple_statement_omp_for {
-  /* [ WORD 1-7 ]  */
-  struct gimple_statement_omp omp;
+struct GTY((user)) gimple_statement_omp_for : public gimple_statement_omp {
+  /* [ WORD 1-7 ] : base class */
 
   /* [ WORD 8 ]  */
   tree clauses;
@@ -545,9 +536,8 @@ struct GTY(()) gimple_statement_omp_for {
 
 /* GIMPLE_OMP_PARALLEL */
 
-struct GTY(()) gimple_statement_omp_parallel {
-  /* [ WORD 1-7 ]  */
-  struct gimple_statement_omp omp;
+struct GTY((user)) gimple_statement_omp_parallel : public gimple_statement_omp {
+  /* [ WORD 1-7 ] : base class */
 
   /* [ WORD 8 ]
      Clauses.  */
@@ -565,9 +555,8 @@ struct GTY(()) gimple_statement_omp_parallel {
 
 /* GIMPLE_OMP_TASK */
 
-struct GTY(()) gimple_statement_omp_task {
-  /* [ WORD 1-10 ]  */
-  struct gimple_statement_omp_parallel par;
+struct GTY((user)) gimple_statement_omp_task : public gimple_statement_omp_parallel {
+  /* [ WORD 1-10 ] : base class */
 
   /* [ WORD 11 ]
      Child function holding firstprivate initialization if needed.  */
@@ -586,9 +575,10 @@ struct GTY(()) gimple_statement_omp_task {
 
 /* GIMPLE_OMP_SECTIONS */
 
-struct GTY(()) gimple_statement_omp_sections {
-  /* [ WORD 1-7 ]  */
-  struct gimple_statement_omp omp;
+struct GTY((user)) gimple_statement_omp_sections
+  : public gimple_statement_omp
+{
+  /* [ WORD 1-7 ] : base class */
 
   /* [ WORD 8 ]  */
   tree clauses;
@@ -604,9 +594,10 @@ struct GTY(()) gimple_statement_omp_sections {
    Note: This does not inherit from gimple_statement_omp, because we
          do not need the body field.  */
 
-struct GTY(()) gimple_statement_omp_continue {
-  /* [ WORD 1-6 ]  */
-  struct gimple_statement_base gsbase;
+struct GTY((user)) gimple_statement_omp_continue
+  : public gimple_statement_base
+{
+  /* [ WORD 1-6 ] : base class */
 
   /* [ WORD 7 ]  */
   tree control_def;
@@ -617,9 +608,8 @@ struct GTY(()) gimple_statement_omp_continue {
 
 /* GIMPLE_OMP_SINGLE */
 
-struct GTY(()) gimple_statement_omp_single {
-  /* [ WORD 1-7 ]  */
-  struct gimple_statement_omp omp;
+struct GTY((user)) gimple_statement_omp_single : public gimple_statement_omp {
+  /* [ WORD 1-7 ] : base class */
 
   /* [ WORD 7 ]  */
   tree clauses;
@@ -630,9 +620,10 @@ struct GTY(()) gimple_statement_omp_single {
    Note: This is based on gimple_statement_base, not g_s_omp, because g_s_omp
    contains a sequence, which we don't need here.  */
 
-struct GTY(()) gimple_statement_omp_atomic_load {
-  /* [ WORD 1-6 ]  */
-  struct gimple_statement_base gsbase;
+struct GTY((user)) gimple_statement_omp_atomic_load
+  : public gimple_statement_base
+{
+  /* [ WORD 1-6 ] : base class */
 
   /* [ WORD 7-8 ]  */
   tree rhs, lhs;
@@ -641,9 +632,10 @@ struct GTY(()) gimple_statement_omp_atomic_load {
 /* GIMPLE_OMP_ATOMIC_STORE.
    See note on GIMPLE_OMP_ATOMIC_LOAD.  */
 
-struct GTY(()) gimple_statement_omp_atomic_store {
-  /* [ WORD 1-6 ]  */
-  struct gimple_statement_base gsbase;
+struct GTY((user)) gimple_statement_omp_atomic_store
+  : public gimple_statement_base
+{
+  /* [ WORD 1-6 ] : base class */
 
   /* [ WORD 7 ]  */
   tree val;
@@ -677,10 +669,10 @@ struct GTY(()) gimple_statement_omp_atomic_store {
    likely because it is guaranteed to go irrevocable upon entry.  */
 #define GTMA_HAS_NO_INSTRUMENTATION	(1u << 7)
 
-struct GTY(()) gimple_statement_transaction
+struct GTY((user)) gimple_statement_transaction
+  : public gimple_statement_with_memory_ops_base
 {
-  /* [ WORD 1-9 ]  */
-  struct gimple_statement_with_memory_ops_base gsbase;
+  /* [ WORD 1-9 ] : base class */
 
   /* [ WORD 10 ] */
   gimple_seq body;
@@ -697,39 +689,6 @@ enum gimple_statement_structure_enum {
 #undef DEFGSSTRUCT
 
 
-/* Define the overall contents of a gimple tuple.  It may be any of the
-   structures declared above for various types of tuples.  */
-
-union GTY ((desc ("gimple_statement_structure (&%h)"),
-	    chain_next ("%h.gsbase.next"), variable_size)) gimple_statement_d {
-  struct gimple_statement_base GTY ((tag ("GSS_BASE"))) gsbase;
-  struct gimple_statement_with_ops GTY ((tag ("GSS_WITH_OPS"))) gsops;
-  struct gimple_statement_with_memory_ops_base GTY ((tag ("GSS_WITH_MEM_OPS_BASE"))) gsmembase;
-  struct gimple_statement_with_memory_ops GTY ((tag ("GSS_WITH_MEM_OPS"))) gsmem;
-  struct gimple_statement_call GTY ((tag ("GSS_CALL"))) gimple_call;
-  struct gimple_statement_omp GTY ((tag ("GSS_OMP"))) omp;
-  struct gimple_statement_bind GTY ((tag ("GSS_BIND"))) gimple_bind;
-  struct gimple_statement_catch GTY ((tag ("GSS_CATCH"))) gimple_catch;
-  struct gimple_statement_eh_filter GTY ((tag ("GSS_EH_FILTER"))) gimple_eh_filter;
-  struct gimple_statement_eh_mnt GTY ((tag ("GSS_EH_MNT"))) gimple_eh_mnt;
-  struct gimple_statement_eh_else GTY ((tag ("GSS_EH_ELSE"))) gimple_eh_else;
-  struct gimple_statement_phi GTY ((tag ("GSS_PHI"))) gimple_phi;
-  struct gimple_statement_eh_ctrl GTY ((tag ("GSS_EH_CTRL"))) gimple_eh_ctrl;
-  struct gimple_statement_try GTY ((tag ("GSS_TRY"))) gimple_try;
-  struct gimple_statement_wce GTY ((tag ("GSS_WCE"))) gimple_wce;
-  struct gimple_statement_asm GTY ((tag ("GSS_ASM"))) gimple_asm;
-  struct gimple_statement_omp_critical GTY ((tag ("GSS_OMP_CRITICAL"))) gimple_omp_critical;
-  struct gimple_statement_omp_for GTY ((tag ("GSS_OMP_FOR"))) gimple_omp_for;
-  struct gimple_statement_omp_parallel GTY ((tag ("GSS_OMP_PARALLEL"))) gimple_omp_parallel;
-  struct gimple_statement_omp_task GTY ((tag ("GSS_OMP_TASK"))) gimple_omp_task;
-  struct gimple_statement_omp_sections GTY ((tag ("GSS_OMP_SECTIONS"))) gimple_omp_sections;
-  struct gimple_statement_omp_single GTY ((tag ("GSS_OMP_SINGLE"))) gimple_omp_single;
-  struct gimple_statement_omp_continue GTY ((tag ("GSS_OMP_CONTINUE"))) gimple_omp_continue;
-  struct gimple_statement_omp_atomic_load GTY ((tag ("GSS_OMP_ATOMIC_LOAD"))) gimple_omp_atomic_load;
-  struct gimple_statement_omp_atomic_store GTY ((tag ("GSS_OMP_ATOMIC_STORE"))) gimple_omp_atomic_store;
-  struct gimple_statement_transaction GTY((tag ("GSS_TRANSACTION"))) gimple_transaction;
-};
-
 /* In gimple.c.  */
 
 /* Helper functions to build GIMPLE statements.  */
diff --git a/gcc/system.h b/gcc/system.h
index b735a96..387a2f1 100644
--- a/gcc/system.h
+++ b/gcc/system.h
@@ -1015,7 +1015,7 @@ helper_const_non_const_cast (const char *p)
 #define CONST_CAST_TREE(X) CONST_CAST(union tree_node *, (X))
 #define CONST_CAST_RTX(X) CONST_CAST(struct rtx_def *, (X))
 #define CONST_CAST_BB(X) CONST_CAST(struct basic_block_def *, (X))
-#define CONST_CAST_GIMPLE(X) CONST_CAST(union gimple_statement_d *, (X))
+#define CONST_CAST_GIMPLE(X) CONST_CAST(struct gimple_statement_base *, (X))
 
 /* Activate certain diagnostics as warnings (not errors via the
    -Werror flag).  */
diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
index 3ba321d..911d429 100644
--- a/gcc/tree-ssa-ccp.c
+++ b/gcc/tree-ssa-ccp.c
@@ -1689,7 +1689,7 @@ evaluate_stmt (gimple stmt)
   return val;
 }
 
-typedef hash_table <pointer_hash <gimple_statement_d> > gimple_htab;
+typedef hash_table <pointer_hash <gimple_statement_base> > gimple_htab;
 
 /* Given a BUILT_IN_STACK_SAVE value SAVED_VAL, insert a clobber of VAR before
    each matching BUILT_IN_STACK_RESTORE.  Mark visited phis in VISITED.  */
-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH 0/6] Convert gimple to a C++ class hierarchy
@ 2013-08-29 16:20 David Malcolm
  2013-08-29 16:20 ` [PATCH 2/6] Hand-written port of various accessors within gimple.h David Malcolm
                   ` (7 more replies)
  0 siblings, 8 replies; 44+ messages in thread
From: David Malcolm @ 2013-08-29 16:20 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

The various gimple types are currently implemented using a hand-coded
C inheritance scheme, with a "union gimple_statement_d" holding the
various possible structs for a statement.

The following series of patches convert it to a C++ class
hierarchy, using the existing structs, eliminating the union. The
"gimple" typedef changes from being a
  (union gimple_statement_d *)
to being a:
  (struct gimple_statement_base *)

There are no virtual functions in the new code: the sizes of the various
structs are unchanged.

It makes use of "is-a.h", using the as_a <T> template function to
perform downcasts, which are checked (via gcc_checking_assert) in an
ENABLE_CHECKING build, and are simple casts in an unchecked build,
albeit it in an inlined function rather than a macro.

For example, one can write:

  gimple_statement_phi *phi =
    as_a <gimple_statement_phi> (gsi_stmt (gsi));

and then directly access the fields of the phi, as a phi.  The existing
accessor functions in gimple.h become somewhat redundant in this
scheme, but are preserved.

I believe this is a superior scheme to the C implementation:

  * We can get closer to compile-time type-safety, checking the gimple
    code once and downcasting with an as_a, then directly accessing
    fields, rather than going through accessor functions that check
    each time.  In some places we may want to replace a "gimple" with
    a subclass e.g. phis are always of the phi subclass, to get full
    compile-time type-safety.

  * This scheme is likely to be easier for newbies to understand.
  
  * Currently in gdb, dereferencing a gimple leads to screenfuls of text,
    showing all the various union values.  With this, you get just the base
    class, and can cast it to the appropriate subclass.

  * We're working directly with the language constructs, rather than
    rolling our own, and thus other tools can better understand the
    code.  For example, you can see Doxygen's rendering of the gimple
    class hierarchy (in the modified code) here:
    http://fedorapeople.org/~dmalcolm/gcc/2013-08-28/doxygen/html/structgimple__statement__base.html

The names of the structs are rather verbose.  I would prefer to also
rename them all to eliminate the "_statement" component:
  "gimple_statement_base" -> "gimple_base"
  "gimple_statement_phi"  -> "gimple_phi"
  "gimple_statement_omp"  -> "gimple_omp"
etc, but I didn't do this to mimimize the patch size.  But if the core
maintainers are up for that, I can redo the patch series with that
change also.

The patch is in 6 parts; all of them are needed together.

  * Patch 1 of 6: This patch adds inheritance to the various gimple
    types, eliminating the initial baseclass fields, and eliminating the
    union gimple_statement_d.   All the types remain structs.  They
    become marked with GTY((user)).

  * Patch 2 of 6: This patch ports various accessor functions within
    gimple.h to the new scheme.

  * Patch 3 of 6: This patch is autogenerated by "refactor_gimple.py"
    from https://github.com/davidmalcolm/gcc-refactoring-scripts
    There is a test suite "test_refactor_gimple.py" which may give a
    clearer idea of the changes that the script makes (and add
    confidence that it's doing the right thing).
    The patch converts code of the form:
      {
        GIMPLE_CHECK (gs, SOME_CODE);
        gimple_subclass_get/set_some_field (gs, value);
      }
    to code of this form:
      {
        some_subclass *stmt = as_a <some_subclass> (gs);
        stmt->some_field = value;
      }
    It also autogenerates specializations of 
        is_a_helper <T>::test
    equivalent to a GIMPLE_CHECK() for use by is_a and as_a.

  * Patch 4 of 6: This patch implement further specializations of
    is_a_helper <T>::test, for gimple_has_ops and gimple_has_mem_ops.

  * Patch 5 of 6: This patch does the rest of porting from union access
    to subclass access (all the fiddly places that the script in patch 3
    couldn't handle).

  * Patch 6 of 6: This patch adds manual implementation of the GTY
    hooks, written by heavily-editing the autogenerated gtype-desc.c
    code from an earlier incarnation.   It implements the equivalent
    of chain_next for gimple statements.

Successfully bootstrapped and tested on x86_64-unknown-linux-gnu: all
testcases show the same results as an unpatched build (relative to
r202029).

OK for trunk?

 gcc/Makefile.in           |    2 +-
 gcc/coretypes.h           |    5 +-
 gcc/ggc.h                 |    6 +-
 gcc/gimple-iterator.c     |   72 +--
 gcc/gimple-pretty-print.c |    6 +-
 gcc/gimple-pretty-print.h |    4 +-
 gcc/gimple-streamer-in.c  |   19 +-
 gcc/gimple-streamer-out.c |    2 +-
 gcc/gimple.c              |  819 +++++++++++++++++++++++-
 gcc/gimple.h              | 1545 +++++++++++++++++++++++++++++----------------
 gcc/gimplify.c            |    4 +-
 gcc/system.h              |    2 +-
 gcc/tree-cfg.c            |    6 +-
 gcc/tree-inline.c         |    2 +-
 gcc/tree-phinodes.c       |   39 +-
 gcc/tree-ssa-ccp.c        |    2 +-
 16 files changed, 1860 insertions(+), 675 deletions(-)

-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH 5/6] Port various places from union access to subclass access.
  2013-08-29 16:20 [PATCH 0/6] Convert gimple to a C++ class hierarchy David Malcolm
  2013-08-29 16:20 ` [PATCH 2/6] Hand-written port of various accessors within gimple.h David Malcolm
  2013-08-29 16:20 ` [PATCH 4/6] Implement is_a_helper <>::test specializations for various gimple types David Malcolm
@ 2013-08-29 16:20 ` David Malcolm
  2013-08-29 16:20 ` [PATCH 1/6] Convert gimple types from a union to a C++ class hierarchy David Malcolm
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 44+ messages in thread
From: David Malcolm @ 2013-08-29 16:20 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

	* gimple-streamer-in.c (input_gimple_stmt): Port from union
	access to use of as_a.
	* gimple.c (gimple_build_asm_1): Likewise.
	(gimple_build_try): Likewise.  Also, return a specific subclass
	rather than just gimple.
	(gimple_build_resx): Port from union access to use of as_a.
	(gimple_build_eh_dispatch): Likewise.
	(gimple_build_omp_for): Likewise.  Also, convert allocation of iter
	now that gengtype no longer provides a typed allocator function.
	(gimple_copy): Likewise.
	* gimple.h (gimple_build_try): Return a specific subclass rather
	than just gimple.
	* gimplify.c (gimplify_cleanup_point_expr): Replace union access
	with subclass access by making use of new return type of
	gimple_build_try.
	* tree-phinodes.c: (allocate_phi_node): Return a
	"gimple_statement_phi *" rather than just a gimple.
	(resize_phi_node): Likewise.
	(make_phi_node): Replace union access with subclass access by
	making use of new return type of allocate_phi_node.
	(reserve_phi_args_for_new_edge): Replace union access with as_a.
	(remove_phi_arg_num): Accept a "gimple_statement_phi *" rather
	than just a gimple.
	(remove_phi_args): Update for change to remove_phi_arg_num.
---
 gcc/gimple-streamer-in.c | 11 ++++-----
 gcc/gimple.c             | 58 ++++++++++++++++++++++++++++++------------------
 gcc/gimple.h             |  3 ++-
 gcc/gimplify.c           |  4 ++--
 gcc/tree-phinodes.c      | 37 ++++++++++++++++--------------
 5 files changed, 66 insertions(+), 47 deletions(-)

diff --git a/gcc/gimple-streamer-in.c b/gcc/gimple-streamer-in.c
index fc0b20a..f89f42e 100644
--- a/gcc/gimple-streamer-in.c
+++ b/gcc/gimple-streamer-in.c
@@ -126,13 +126,14 @@ input_gimple_stmt (struct lto_input_block *ib, struct data_in *data_in,
     case GIMPLE_ASM:
       {
 	/* FIXME lto.  Move most of this into a new gimple_asm_set_string().  */
+	gimple_statement_asm *asm_stmt = as_a <gimple_statement_asm> (stmt);
 	tree str;
-	stmt->gimple_asm.ni = streamer_read_uhwi (ib);
-	stmt->gimple_asm.no = streamer_read_uhwi (ib);
-	stmt->gimple_asm.nc = streamer_read_uhwi (ib);
-	stmt->gimple_asm.nl = streamer_read_uhwi (ib);
+	asm_stmt->ni = streamer_read_uhwi (ib);
+	asm_stmt->no = streamer_read_uhwi (ib);
+	asm_stmt->nc = streamer_read_uhwi (ib);
+	asm_stmt->nl = streamer_read_uhwi (ib);
 	str = streamer_read_string_cst (data_in, ib);
-	stmt->gimple_asm.string = TREE_STRING_POINTER (str);
+	asm_stmt->string = TREE_STRING_POINTER (str);
       }
       /* Fallthru  */
 
diff --git a/gcc/gimple.c b/gcc/gimple.c
index aceb1b0..1ad36d1 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -626,21 +626,22 @@ static inline gimple
 gimple_build_asm_1 (const char *string, unsigned ninputs, unsigned noutputs,
                     unsigned nclobbers, unsigned nlabels)
 {
-  gimple p;
+  gimple_statement_asm *p;
   int size = strlen (string);
 
   /* ASMs with labels cannot have outputs.  This should have been
      enforced by the front end.  */
   gcc_assert (nlabels == 0 || noutputs == 0);
 
-  p = gimple_build_with_ops (GIMPLE_ASM, ERROR_MARK,
-			     ninputs + noutputs + nclobbers + nlabels);
+  p = as_a <gimple_statement_asm> (
+        gimple_build_with_ops (GIMPLE_ASM, ERROR_MARK,
+			       ninputs + noutputs + nclobbers + nlabels));
 
-  p->gimple_asm.ni = ninputs;
-  p->gimple_asm.no = noutputs;
-  p->gimple_asm.nc = nclobbers;
-  p->gimple_asm.nl = nlabels;
-  p->gimple_asm.string = ggc_alloc_string (string, size);
+  p->ni = ninputs;
+  p->no = noutputs;
+  p->nc = nclobbers;
+  p->nl = nlabels;
+  p->string = ggc_alloc_string (string, size);
 
   if (GATHER_STATISTICS)
     gimple_alloc_sizes[(int) gimple_alloc_kind (GIMPLE_ASM)] += size;
@@ -752,14 +753,14 @@ gimple_build_eh_else (gimple_seq n_body, gimple_seq e_body)
    KIND is either GIMPLE_TRY_CATCH or GIMPLE_TRY_FINALLY depending on
    whether this is a try/catch or a try/finally respectively.  */
 
-gimple
+gimple_statement_try *
 gimple_build_try (gimple_seq eval, gimple_seq cleanup,
     		  enum gimple_try_flags kind)
 {
-  gimple p;
+  gimple_statement_try *p;
 
   gcc_assert (kind == GIMPLE_TRY_CATCH || kind == GIMPLE_TRY_FINALLY);
-  p = gimple_alloc (GIMPLE_TRY, 0);
+  p = as_a <gimple_statement_try> (gimple_alloc (GIMPLE_TRY, 0));
   gimple_set_subcode (p, kind);
   if (eval)
     gimple_try_set_eval (p, eval);
@@ -789,8 +790,10 @@ gimple_build_wce (gimple_seq cleanup)
 gimple
 gimple_build_resx (int region)
 {
-  gimple p = gimple_build_with_ops (GIMPLE_RESX, ERROR_MARK, 0);
-  p->gimple_eh_ctrl.region = region;
+  gimple_statement_eh_ctrl *p =
+    as_a <gimple_statement_eh_ctrl> (
+      gimple_build_with_ops (GIMPLE_RESX, ERROR_MARK, 0));
+  p->region = region;
   return p;
 }
 
@@ -837,8 +840,10 @@ gimple_build_switch (tree index, tree default_label, vec<tree> args)
 gimple
 gimple_build_eh_dispatch (int region)
 {
-  gimple p = gimple_build_with_ops (GIMPLE_EH_DISPATCH, ERROR_MARK, 0);
-  p->gimple_eh_ctrl.region = region;
+  gimple_statement_eh_ctrl *p =
+    as_a <gimple_statement_eh_ctrl> (
+      gimple_build_with_ops (GIMPLE_EH_DISPATCH, ERROR_MARK, 0));
+  p->region = region;
   return p;
 }
 
@@ -912,14 +917,17 @@ gimple
 gimple_build_omp_for (gimple_seq body, int kind, tree clauses, size_t collapse,
 		      gimple_seq pre_body)
 {
-  gimple p = gimple_alloc (GIMPLE_OMP_FOR, 0);
+  gimple_statement_omp_for *p =
+    as_a <gimple_statement_omp_for> (gimple_alloc (GIMPLE_OMP_FOR, 0));
   if (body)
     gimple_omp_set_body (p, body);
   gimple_omp_for_set_clauses (p, clauses);
   gimple_omp_for_set_kind (p, kind);
-  p->gimple_omp_for.collapse = collapse;
-  p->gimple_omp_for.iter
-      = ggc_alloc_cleared_vec_gimple_omp_for_iter (collapse);
+  p->collapse = collapse;
+  p->iter =  static_cast <struct gimple_omp_for_iter *> (
+   ggc_internal_cleared_vec_alloc_stat (sizeof (*p->iter),
+					collapse MEM_STAT_INFO));
+
   if (pre_body)
     gimple_omp_for_set_pre_body (p, pre_body);
 
@@ -2255,9 +2263,15 @@ gimple_copy (gimple stmt)
 	  gimple_omp_for_set_pre_body (copy, new_seq);
 	  t = unshare_expr (gimple_omp_for_clauses (stmt));
 	  gimple_omp_for_set_clauses (copy, t);
-	  copy->gimple_omp_for.iter
-	    = ggc_alloc_vec_gimple_omp_for_iter
-	    (gimple_omp_for_collapse (stmt));
+	  {
+	    gimple_statement_omp_for *omp_for_copy =
+	      as_a <gimple_statement_omp_for> (copy);
+	    omp_for_copy->iter =
+	      static_cast <struct gimple_omp_for_iter *> (
+		  ggc_internal_vec_alloc_stat (sizeof (struct gimple_omp_for_iter),
+					       gimple_omp_for_collapse (stmt)
+					       MEM_STAT_INFO));
+          }
 	  for (i = 0; i < gimple_omp_for_collapse (stmt); i++)
 	    {
 	      gimple_omp_for_set_cond (copy, i,
diff --git a/gcc/gimple.h b/gcc/gimple.h
index e2cd383..daab54e 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -1057,7 +1057,8 @@ gimple gimple_build_catch (tree, gimple_seq);
 gimple gimple_build_eh_filter (tree, gimple_seq);
 gimple gimple_build_eh_must_not_throw (tree);
 gimple gimple_build_eh_else (gimple_seq, gimple_seq);
-gimple gimple_build_try (gimple_seq, gimple_seq, enum gimple_try_flags);
+gimple_statement_try *gimple_build_try (gimple_seq, gimple_seq,
+					enum gimple_try_flags);
 gimple gimple_build_wce (gimple_seq);
 gimple gimple_build_resx (int);
 gimple gimple_build_eh_dispatch (int);
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 3b3adb3..7fff03d 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -5497,7 +5497,7 @@ gimplify_cleanup_point_expr (tree *expr_p, gimple_seq *pre_p)
 	    }
 	  else
 	    {
-	      gimple gtry;
+	      gimple_statement_try *gtry;
 	      gimple_seq seq;
 	      enum gimple_try_flags kind;
 
@@ -5511,7 +5511,7 @@ gimplify_cleanup_point_expr (tree *expr_p, gimple_seq *pre_p)
               /* Do not use gsi_replace here, as it may scan operands.
                  We want to do a simple structural modification only.  */
 	      gsi_set_stmt (&iter, gtry);
-	      iter = gsi_start (gtry->gimple_try.eval);
+	      iter = gsi_start (gtry->eval);
 	    }
 	}
       else
diff --git a/gcc/tree-phinodes.c b/gcc/tree-phinodes.c
index 60414e5..50c17bb 100644
--- a/gcc/tree-phinodes.c
+++ b/gcc/tree-phinodes.c
@@ -88,10 +88,10 @@ phinodes_print_statistics (void)
    happens to contain a PHI node with LEN arguments or more, return
    that one.  */
 
-static inline gimple
+static inline gimple_statement_phi *
 allocate_phi_node (size_t len)
 {
-  gimple phi;
+  gimple_statement_phi *phi;
   size_t bucket = NUM_BUCKETS - 2;
   size_t size = sizeof (struct gimple_statement_phi)
 	        + (len - 1) * sizeof (struct phi_arg_d);
@@ -106,7 +106,7 @@ allocate_phi_node (size_t len)
       && gimple_phi_capacity ((*free_phinodes[bucket])[0]) >= len)
     {
       free_phinode_count--;
-      phi = free_phinodes[bucket]->pop ();
+      phi = as_a <gimple_statement_phi> (free_phinodes[bucket]->pop ());
       if (free_phinodes[bucket]->is_empty ())
 	vec_free (free_phinodes[bucket]);
       if (GATHER_STATISTICS)
@@ -114,7 +114,8 @@ allocate_phi_node (size_t len)
     }
   else
     {
-      phi = ggc_alloc_gimple_statement_d (size);
+      phi = static_cast <gimple_statement_phi *> (
+	ggc_internal_alloc_stat (size MEM_STAT_INFO));
       if (GATHER_STATISTICS)
 	{
 	  enum gimple_alloc_kind kind = gimple_alloc_kind (GIMPLE_PHI);
@@ -166,7 +167,7 @@ ideal_phi_node_len (int len)
 static gimple
 make_phi_node (tree var, int len)
 {
-  gimple phi;
+  gimple_statement_phi *phi;
   int capacity, i;
 
   capacity = ideal_phi_node_len (len);
@@ -181,8 +182,8 @@ make_phi_node (tree var, int len)
 		   + sizeof (struct phi_arg_d) * len));
   phi->code = GIMPLE_PHI;
   gimple_init_singleton (phi);
-  phi->gimple_phi.nargs = len;
-  phi->gimple_phi.capacity = capacity;
+  phi->nargs = len;
+  phi->capacity = capacity;
   if (!var)
     ;
   else if (TREE_CODE (var) == SSA_NAME)
@@ -231,11 +232,11 @@ release_phi_node (gimple phi)
 /* Resize an existing PHI node.  The only way is up.  Return the
    possibly relocated phi.  */
 
-static gimple
-resize_phi_node (gimple phi, size_t len)
+static gimple_statement_phi *
+resize_phi_node (gimple_statement_phi *phi, size_t len)
 {
   size_t old_size, i;
-  gimple new_phi;
+  gimple_statement_phi *new_phi;
 
   gcc_assert (len > gimple_phi_capacity (phi));
 
@@ -258,7 +259,7 @@ resize_phi_node (gimple phi, size_t len)
       relink_imm_use_stmt (imm, old_imm, new_phi);
     }
 
-  new_phi->gimple_phi.capacity = len;
+  new_phi->capacity = len;
 
   for (i = gimple_phi_num_args (new_phi); i < len; i++)
     {
@@ -286,11 +287,12 @@ reserve_phi_args_for_new_edge (basic_block bb)
 
   for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi))
     {
-      gimple stmt = gsi_stmt (gsi);
+      gimple_statement_phi *stmt =
+	as_a <gimple_statement_phi> (gsi_stmt (gsi));
 
       if (len > gimple_phi_capacity (stmt))
 	{
-	  gimple new_phi = resize_phi_node (stmt, cap);
+	  gimple_statement_phi *new_phi = resize_phi_node (stmt, cap);
 
 	  /* The result of the PHI is defined by this PHI node.  */
 	  SSA_NAME_DEF_STMT (gimple_phi_result (new_phi)) = new_phi;
@@ -310,7 +312,7 @@ reserve_phi_args_for_new_edge (basic_block bb)
       SET_PHI_ARG_DEF (stmt, len - 1, NULL_TREE);
       gimple_phi_arg_set_location (stmt, len - 1, UNKNOWN_LOCATION);
 
-      stmt->gimple_phi.nargs++;
+      stmt->nargs++;
     }
 }
 
@@ -386,7 +388,7 @@ add_phi_arg (gimple phi, tree def, edge e, source_location locus)
    is consistent with how we remove an edge from the edge vector.  */
 
 static void
-remove_phi_arg_num (gimple phi, int i)
+remove_phi_arg_num (gimple_statement_phi *phi, int i)
 {
   int num_elem = gimple_phi_num_args (phi);
 
@@ -413,7 +415,7 @@ remove_phi_arg_num (gimple phi, int i)
   /* Shrink the vector and return.  Note that we do not have to clear
      PHI_ARG_DEF because the garbage collector will not look at those
      elements beyond the first PHI_NUM_ARGS elements of the array.  */
-  phi->gimple_phi.nargs--;
+  phi->nargs--;
 }
 
 
@@ -425,7 +427,8 @@ remove_phi_args (edge e)
   gimple_stmt_iterator gsi;
 
   for (gsi = gsi_start_phis (e->dest); !gsi_end_p (gsi); gsi_next (&gsi))
-    remove_phi_arg_num (gsi_stmt (gsi), e->dest_idx);
+    remove_phi_arg_num (as_a <gimple_statement_phi> (gsi_stmt (gsi)),
+			e->dest_idx);
 }
 
 
-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH 2/6] Hand-written port of various accessors within gimple.h
  2013-08-29 16:20 [PATCH 0/6] Convert gimple to a C++ class hierarchy David Malcolm
@ 2013-08-29 16:20 ` David Malcolm
  2013-08-29 16:20 ` [PATCH 4/6] Implement is_a_helper <>::test specializations for various gimple types David Malcolm
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 44+ messages in thread
From: David Malcolm @ 2013-08-29 16:20 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

	* gimple.h (gimple_use_ops): Port from union to usage of
	dyn_cast.
	(gimple_set_use_ops): Port from union to usage of as_a.
	(gimple_set_vuse): Likewise.
	(gimple_set_vdef): Likewise.
	(gimple_call_internal_fn): Port from union to a static_cast,
	given that the type has already been asserted.
	(gimple_omp_body_ptr): Port from unchecked union usage to
	a static_cast.
	(gimple_omp_set_body): Likewise.
---
 gcc/gimple.h | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/gcc/gimple.h b/gcc/gimple.h
index d7ea2e4..1ee6238 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -1433,9 +1433,11 @@ gimple_has_mem_ops (const_gimple g)
 static inline struct use_optype_d *
 gimple_use_ops (const_gimple g)
 {
-  if (!gimple_has_ops (g))
+  const gimple_statement_with_ops *ops_stmt =
+    dyn_cast <const gimple_statement_with_ops> (g);
+  if (!ops_stmt)
     return NULL;
-  return g->gsops.opbase.use_ops;
+  return ops_stmt->use_ops;
 }
 
 
@@ -1444,8 +1446,9 @@ gimple_use_ops (const_gimple g)
 static inline void
 gimple_set_use_ops (gimple g, struct use_optype_d *use)
 {
-  gcc_gimple_checking_assert (gimple_has_ops (g));
-  g->gsops.opbase.use_ops = use;
+  gimple_statement_with_ops *ops_stmt =
+    as_a <gimple_statement_with_ops> (g);
+  ops_stmt->use_ops = use;
 }
 
 
@@ -1522,8 +1525,9 @@ gimple_vdef_ptr (gimple g)
 static inline void
 gimple_set_vuse (gimple g, tree vuse)
 {
-  gcc_gimple_checking_assert (gimple_has_mem_ops (g));
-  g->gsmembase.vuse = vuse;
+  gimple_statement_with_memory_ops *mem_ops_stmt =
+    as_a <gimple_statement_with_memory_ops> (g);
+  mem_ops_stmt->vuse = vuse;
 }
 
 /* Set the single VDEF operand of the statement G.  */
@@ -1531,8 +1535,9 @@ gimple_set_vuse (gimple g, tree vuse)
 static inline void
 gimple_set_vdef (gimple g, tree vdef)
 {
-  gcc_gimple_checking_assert (gimple_has_mem_ops (g));
-  g->gsmembase.vdef = vdef;
+  gimple_statement_with_memory_ops *mem_ops_stmt =
+    as_a <gimple_statement_with_memory_ops> (g);
+  mem_ops_stmt->vdef = vdef;
 }
 
 
@@ -2188,7 +2193,7 @@ static inline enum internal_fn
 gimple_call_internal_fn (const_gimple gs)
 {
   gcc_gimple_checking_assert (gimple_call_internal_p (gs));
-  return gs->gimple_call.u.internal_fn;
+  return static_cast <const gimple_statement_call *> (gs)->u.internal_fn;
 }
 
 
@@ -3860,7 +3865,7 @@ gimple_debug_source_bind_set_value (gimple dbg, tree value)
 static inline gimple_seq *
 gimple_omp_body_ptr (gimple gs)
 {
-  return &gs->omp.body;
+  return &static_cast <gimple_statement_omp *> (gs)->body;
 }
 
 /* Return the body for the OMP statement GS.  */
@@ -3876,7 +3881,7 @@ gimple_omp_body (gimple gs)
 static inline void
 gimple_omp_set_body (gimple gs, gimple_seq body)
 {
-  gs->omp.body = body;
+  static_cast <gimple_statement_omp *> (gs)->body = body;
 }
 
 
-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH 3/6] Autogenerated conversion of gimple to C++
  2013-08-29 16:20 [PATCH 0/6] Convert gimple to a C++ class hierarchy David Malcolm
                   ` (3 preceding siblings ...)
  2013-08-29 16:20 ` [PATCH 1/6] Convert gimple types from a union to a C++ class hierarchy David Malcolm
@ 2013-08-29 16:20 ` David Malcolm
  2013-08-31 13:40   ` [PATCH 3/6 v2] " David Malcolm
  2013-08-29 17:04 ` [PATCH 6/6] Add manual GTY hooks David Malcolm
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 44+ messages in thread
From: David Malcolm @ 2013-08-29 16:20 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

This patch is 110KB in size, so to avoid mailing-list size limits I've uploaded it to:

http://dmalcolm.fedorapeople.org/gcc/large-patches/a89d361b4f95dd216e1d29cb44fbaf90372c48b8-0003-Autogenerated-conversion-of-gimple-to-C.patch

The ChangeLog entry and diffstat follow:

gcc/

	Patch autogenerated by refactor_gimple.py from
	https://github.com/davidmalcolm/gcc-refactoring-scripts
	revision 26aabff11750d29e6129930a426242a564538d1a

	* gimple-iterator.c (update_bb_for_stmts): Update for conversion of
	gimple types to a true class hierarchy.
	(update_call_edge_frequencies): Likewise.
	(gsi_insert_seq_nodes_before): Likewise.
	(gsi_insert_seq_nodes_after): Likewise.
	(gsi_split_seq_after): Likewise.
	(gsi_set_stmt): Likewise.
	(gsi_split_seq_before): Likewise.
	(gsi_remove): Likewise.
	* gimple-pretty-print.c (dump_gimple_debug): Likewise.
	* gimple-streamer-in.c (input_gimple_stmt): Likewise.
	* gimple-streamer-out.c (output_gimple_stmt): Likewise.
	* gimple.c (gimple_set_code): Likewise.
	(gimple_alloc_stat): Likewise.
	(gimple_set_subcode): Likewise.
	(gimple_build_call_internal_1): Likewise.
	(gimple_check_failed): Likewise.
	(gimple_call_flags): Likewise.
	(gimple_set_bb): Likewise.
	* gimple.h (is_a_helper <gimple_statement_asm> (gimple)): New.
	(is_a_helper <gimple_statement_bind> (gimple)): Likewise.
	(is_a_helper <gimple_statement_call> (gimple)): Likewise.
	(is_a_helper <gimple_statement_catch> (gimple)): Likewise.
	(is_a_helper <gimple_statement_eh_ctrl> (gimple)): Likewise.
	(is_a_helper <gimple_statement_eh_else> (gimple)): Likewise.
	(is_a_helper <gimple_statement_eh_filter> (gimple)): Likewise.
	(is_a_helper <gimple_statement_eh_mnt> (gimple)): Likewise.
	(is_a_helper <gimple_statement_omp_atomic_load> (gimple)): Likewise.
	(is_a_helper <gimple_statement_omp_atomic_store> (gimple)): Likewise.
	(is_a_helper <gimple_statement_omp_continue> (gimple)): Likewise.
	(is_a_helper <gimple_statement_omp_critical> (gimple)): Likewise.
	(is_a_helper <gimple_statement_omp_for> (gimple)): Likewise.
	(is_a_helper <gimple_statement_omp_parallel> (gimple)): Likewise.
	(is_a_helper <gimple_statement_omp_sections> (gimple)): Likewise.
	(is_a_helper <gimple_statement_omp_single> (gimple)): Likewise.
	(is_a_helper <gimple_statement_omp_task> (gimple)): Likewise.
	(is_a_helper <gimple_statement_phi> (gimple)): Likewise.
	(is_a_helper <gimple_statement_transaction> (gimple)): Likewise.
	(is_a_helper <gimple_statement_try> (gimple)): Likewise.
	(is_a_helper <gimple_statement_wce> (gimple)): Likewise.
	(is_a_helper <const gimple_statement_asm> (const_gimple)): Likewise.
	(is_a_helper <const gimple_statement_bind> (const_gimple)): Likewise.
	(is_a_helper <const gimple_statement_call> (const_gimple)): Likewise.
	(is_a_helper <const gimple_statement_catch> (const_gimple)): Likewise.
	(is_a_helper <const gimple_statement_eh_ctrl> (const_gimple)):
	Likewise.
	(is_a_helper <const gimple_statement_eh_filter> (const_gimple)):
	Likewise.
	(is_a_helper <const gimple_statement_omp_atomic_load> (const_gimple)):
	Likewise.
	(is_a_helper <const gimple_statement_omp_atomic_store>
	(const_gimple)): Likewise.
	(is_a_helper <const gimple_statement_omp_continue> (const_gimple)):
	Likewise.
	(is_a_helper <const gimple_statement_omp_critical> (const_gimple)):
	Likewise.
	(is_a_helper <const gimple_statement_omp_for> (const_gimple)):
	Likewise.
	(is_a_helper <const gimple_statement_omp_parallel> (const_gimple)):
	Likewise.
	(is_a_helper <const gimple_statement_omp_sections> (const_gimple)):
	Likewise.
	(is_a_helper <const gimple_statement_omp_single> (const_gimple)):
	Likewise.
	(is_a_helper <const gimple_statement_omp_task> (const_gimple)):
	Likewise.
	(is_a_helper <const gimple_statement_phi> (const_gimple)): Likewise.
	(is_a_helper <const gimple_statement_transaction> (const_gimple)):
	Likewise.
	(gimple_seq_last): Update for conversion of gimple types to a true
	class hierarchy.
	(gimple_seq_set_last): Likewise.
	(gimple_code): Likewise.
	(gimple_bb): Likewise.
	(gimple_block): Likewise.
	(gimple_set_block): Likewise.
	(gimple_location): Likewise.
	(gimple_location_ptr): Likewise.
	(gimple_set_location): Likewise.
	(gimple_no_warning_p): Likewise.
	(gimple_set_no_warning): Likewise.
	(gimple_set_visited): Likewise.
	(gimple_visited_p): Likewise.
	(gimple_set_plf): Likewise.
	(gimple_plf): Likewise.
	(gimple_set_uid): Likewise.
	(gimple_uid): Likewise.
	(gimple_init_singleton): Likewise.
	(gimple_modified_p): Likewise.
	(gimple_set_modified): Likewise.
	(gimple_expr_code): Likewise.
	(gimple_has_volatile_ops): Likewise.
	(gimple_set_has_volatile_ops): Likewise.
	(gimple_omp_subcode): Likewise.
	(gimple_omp_set_subcode): Likewise.
	(gimple_omp_return_set_nowait): Likewise.
	(gimple_omp_section_set_last): Likewise.
	(gimple_omp_parallel_set_combined_p): Likewise.
	(gimple_omp_atomic_set_need_value): Likewise.
	(gimple_num_ops): Likewise.
	(gimple_set_num_ops): Likewise.
	(gimple_assign_nontemporal_move_p): Likewise.
	(gimple_assign_set_nontemporal_move): Likewise.
	(gimple_assign_rhs_code): Likewise.
	(gimple_assign_set_rhs_code): Likewise.
	(gimple_call_internal_p): Likewise.
	(gimple_call_set_tail): Likewise.
	(gimple_call_tail_p): Likewise.
	(gimple_call_set_return_slot_opt): Likewise.
	(gimple_call_return_slot_opt_p): Likewise.
	(gimple_call_set_from_thunk): Likewise.
	(gimple_call_from_thunk_p): Likewise.
	(gimple_call_set_va_arg_pack): Likewise.
	(gimple_call_va_arg_pack_p): Likewise.
	(gimple_call_set_nothrow): Likewise.
	(gimple_call_set_alloca_for_var): Likewise.
	(gimple_call_alloca_for_var_p): Likewise.
	(gimple_call_copy_flags): Likewise.
	(gimple_cond_code): Likewise.
	(gimple_cond_set_code): Likewise.
	(gimple_cond_make_false): Likewise.
	(gimple_cond_make_true): Likewise.
	(gimple_asm_volatile_p): Likewise.
	(gimple_asm_set_volatile): Likewise.
	(gimple_asm_set_input): Likewise.
	(gimple_asm_input_p): Likewise.
	(gimple_try_kind): Likewise.
	(gimple_try_set_kind): Likewise.
	(gimple_try_catch_is_cleanup): Likewise.
	(gimple_try_set_catch_is_cleanup): Likewise.
	(gimple_wce_cleanup_eh_only): Likewise.
	(gimple_wce_set_cleanup_eh_only): Likewise.
	(gimple_debug_bind_p): Likewise.
	(gimple_debug_source_bind_p): Likewise.
	(gimple_transaction_subcode): Likewise.
	(gimple_transaction_set_subcode): Likewise.
	(gimple_predict_predictor): Likewise.
	(gimple_predict_set_predictor): Likewise.
	(gimple_predict_outcome): Likewise.
	(gimple_predict_set_outcome): Likewise.
	(gsi_one_before_end_p): Likewise.
	(gsi_next): Likewise.
	(gsi_prev): Likewise.
	(gimple_transaction_set_label): Likewise.
	(gimple_transaction_set_body): Likewise.
	(gimple_transaction_label_ptr): Likewise.
	(gimple_transaction_label): Likewise.
	(gimple_transaction_body_ptr): Likewise.
	(gimple_omp_continue_set_control_use): Likewise.
	(gimple_omp_continue_control_use_ptr): Likewise.
	(gimple_omp_continue_control_use): Likewise.
	(gimple_omp_continue_set_control_def): Likewise.
	(gimple_omp_continue_control_def_ptr): Likewise.
	(gimple_omp_continue_control_def): Likewise.
	(gimple_omp_atomic_load_rhs_ptr): Likewise.
	(gimple_omp_atomic_load_rhs): Likewise.
	(gimple_omp_atomic_load_set_rhs): Likewise.
	(gimple_omp_atomic_load_lhs_ptr): Likewise.
	(gimple_omp_atomic_load_lhs): Likewise.
	(gimple_omp_atomic_load_set_lhs): Likewise.
	(gimple_omp_atomic_store_val_ptr): Likewise.
	(gimple_omp_atomic_store_val): Likewise.
	(gimple_omp_atomic_store_set_val): Likewise.
	(gimple_omp_for_cond): Likewise.
	(gimple_omp_for_set_cond): Likewise.
	(gimple_omp_sections_set_control): Likewise.
	(gimple_omp_sections_control_ptr): Likewise.
	(gimple_omp_sections_control): Likewise.
	(gimple_omp_sections_set_clauses): Likewise.
	(gimple_omp_sections_clauses_ptr): Likewise.
	(gimple_omp_sections_clauses): Likewise.
	(gimple_omp_single_set_clauses): Likewise.
	(gimple_omp_single_clauses_ptr): Likewise.
	(gimple_omp_single_clauses): Likewise.
	(gimple_omp_task_set_arg_align): Likewise.
	(gimple_omp_task_arg_align_ptr): Likewise.
	(gimple_omp_task_arg_align): Likewise.
	(gimple_omp_task_set_arg_size): Likewise.
	(gimple_omp_task_arg_size_ptr): Likewise.
	(gimple_omp_task_arg_size): Likewise.
	(gimple_omp_task_set_copy_fn): Likewise.
	(gimple_omp_task_copy_fn_ptr): Likewise.
	(gimple_omp_task_copy_fn): Likewise.
	(gimple_omp_task_set_data_arg): Likewise.
	(gimple_omp_task_data_arg_ptr): Likewise.
	(gimple_omp_task_data_arg): Likewise.
	(gimple_omp_task_set_child_fn): Likewise.
	(gimple_omp_task_child_fn_ptr): Likewise.
	(gimple_omp_task_child_fn): Likewise.
	(gimple_omp_task_set_clauses): Likewise.
	(gimple_omp_task_clauses_ptr): Likewise.
	(gimple_omp_task_clauses): Likewise.
	(gimple_omp_parallel_set_data_arg): Likewise.
	(gimple_omp_parallel_data_arg_ptr): Likewise.
	(gimple_omp_parallel_data_arg): Likewise.
	(gimple_omp_parallel_set_child_fn): Likewise.
	(gimple_omp_parallel_child_fn_ptr): Likewise.
	(gimple_omp_parallel_child_fn): Likewise.
	(gimple_omp_parallel_set_clauses): Likewise.
	(gimple_omp_parallel_clauses_ptr): Likewise.
	(gimple_omp_parallel_clauses): Likewise.
	(gimple_omp_for_set_pre_body): Likewise.
	(gimple_omp_for_pre_body_ptr): Likewise.
	(gimple_omp_for_set_incr): Likewise.
	(gimple_omp_for_incr_ptr): Likewise.
	(gimple_omp_for_incr): Likewise.
	(gimple_omp_for_set_final): Likewise.
	(gimple_omp_for_final_ptr): Likewise.
	(gimple_omp_for_final): Likewise.
	(gimple_omp_for_set_initial): Likewise.
	(gimple_omp_for_initial_ptr): Likewise.
	(gimple_omp_for_initial): Likewise.
	(gimple_omp_for_set_index): Likewise.
	(gimple_omp_for_index_ptr): Likewise.
	(gimple_omp_for_index): Likewise.
	(gimple_omp_for_collapse): Likewise.
	(gimple_omp_for_set_clauses): Likewise.
	(gimple_omp_for_clauses_ptr): Likewise.
	(gimple_omp_for_clauses): Likewise.
	(gimple_omp_for_set_kind): Likewise.
	(gimple_omp_critical_set_name): Likewise.
	(gimple_omp_critical_name_ptr): Likewise.
	(gimple_omp_critical_name): Likewise.
	(gimple_eh_dispatch_set_region): Likewise.
	(gimple_eh_dispatch_region): Likewise.
	(gimple_resx_set_region): Likewise.
	(gimple_resx_region): Likewise.
	(gimple_phi_set_arg): Likewise.
	(gimple_phi_arg): Likewise.
	(gimple_phi_set_result): Likewise.
	(gimple_phi_result_ptr): Likewise.
	(gimple_phi_result): Likewise.
	(gimple_phi_num_args): Likewise.
	(gimple_phi_capacity): Likewise.
	(gimple_wce_set_cleanup): Likewise.
	(gimple_wce_cleanup_ptr): Likewise.
	(gimple_try_set_cleanup): Likewise.
	(gimple_try_set_eval): Likewise.
	(gimple_try_cleanup_ptr): Likewise.
	(gimple_try_eval_ptr): Likewise.
	(gimple_eh_else_set_e_body): Likewise.
	(gimple_eh_else_set_n_body): Likewise.
	(gimple_eh_else_e_body_ptr): Likewise.
	(gimple_eh_else_n_body_ptr): Likewise.
	(gimple_eh_must_not_throw_set_fndecl): Likewise.
	(gimple_eh_must_not_throw_fndecl): Likewise.
	(gimple_eh_filter_set_failure): Likewise.
	(gimple_eh_filter_set_types): Likewise.
	(gimple_eh_filter_failure_ptr): Likewise.
	(gimple_eh_filter_types_ptr): Likewise.
	(gimple_eh_filter_types): Likewise.
	(gimple_catch_set_handler): Likewise.
	(gimple_catch_set_types): Likewise.
	(gimple_catch_handler_ptr): Likewise.
	(gimple_catch_types_ptr): Likewise.
	(gimple_catch_types): Likewise.
	(gimple_asm_string): Likewise.
	(gimple_asm_set_label_op): Likewise.
	(gimple_asm_label_op): Likewise.
	(gimple_asm_set_clobber_op): Likewise.
	(gimple_asm_clobber_op): Likewise.
	(gimple_asm_set_output_op): Likewise.
	(gimple_asm_output_op_ptr): Likewise.
	(gimple_asm_output_op): Likewise.
	(gimple_asm_set_input_op): Likewise.
	(gimple_asm_input_op_ptr): Likewise.
	(gimple_asm_input_op): Likewise.
	(gimple_asm_nlabels): Likewise.
	(gimple_asm_nclobbers): Likewise.
	(gimple_asm_noutputs): Likewise.
	(gimple_asm_ninputs): Likewise.
	(gimple_bind_set_block): Likewise.
	(gimple_bind_block): Likewise.
	(gimple_bind_add_seq): Likewise.
	(gimple_bind_add_stmt): Likewise.
	(gimple_bind_set_body): Likewise.
	(gimple_bind_body_ptr): Likewise.
	(gimple_bind_append_vars): Likewise.
	(gimple_bind_set_vars): Likewise.
	(gimple_bind_vars): Likewise.
	(gimple_call_clobber_set): Likewise.
	(gimple_call_use_set): Likewise.
	(gimple_call_set_internal_fn): Likewise.
	(gimple_call_set_fntype): Likewise.
	(gimple_call_fntype): Likewise.
	(gimple_omp_taskreg_set_data_arg): Likewise.
	(gimple_omp_taskreg_data_arg_ptr): Likewise.
	(gimple_omp_taskreg_data_arg): Likewise.
	(gimple_omp_taskreg_set_child_fn): Likewise.
	(gimple_omp_taskreg_child_fn_ptr): Likewise.
	(gimple_omp_taskreg_child_fn): Likewise.
	(gimple_omp_taskreg_set_clauses): Likewise.
	(gimple_omp_taskreg_clauses_ptr): Likewise.
	(gimple_omp_taskreg_clauses): Likewise.
	(gimple_vuse_op): Likewise.
	(gimple_vdef_op): Likewise.
	(gimple_vuse): Likewise.
	(gimple_vdef): Likewise.
	(gimple_vuse_ptr): Likewise.
	(gimple_vdef_ptr): Likewise.
	* tree-inline.c (copy_debug_stmt): Likewise.
	* tree-phinodes.c (make_phi_node): Likewise.
---
 gcc/gimple-iterator.c     |   72 +--
 gcc/gimple-pretty-print.c |    2 +-
 gcc/gimple-streamer-in.c  |    8 +-
 gcc/gimple-streamer-out.c |    2 +-
 gcc/gimple.c              |   16 +-
 gcc/gimple.h              | 1298 ++++++++++++++++++++++++++++++---------------
 gcc/tree-inline.c         |    2 +-
 gcc/tree-phinodes.c       |    2 +-
 8 files changed, 914 insertions(+), 488 deletions(-)

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH 4/6] Implement is_a_helper <>::test specializations for various gimple types
  2013-08-29 16:20 [PATCH 0/6] Convert gimple to a C++ class hierarchy David Malcolm
  2013-08-29 16:20 ` [PATCH 2/6] Hand-written port of various accessors within gimple.h David Malcolm
@ 2013-08-29 16:20 ` David Malcolm
  2013-08-29 16:20 ` [PATCH 5/6] Port various places from union access to subclass access David Malcolm
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 44+ messages in thread
From: David Malcolm @ 2013-08-29 16:20 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

	* gimple.h (is_a_helper <const gimple_statement_with_ops>::test): New.
	(is_a_helper <gimple_statement_with_ops>::test): New.
	(is_a_helper <const gimple_statement_with_memory_ops>::test): New.
	(is_a_helper <gimple_statement_with_memory_ops>::test): New.
---
 gcc/gimple.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/gcc/gimple.h b/gcc/gimple.h
index 43573a1..e2cd383 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -1722,6 +1722,21 @@ gimple_has_ops (const_gimple g)
   return gimple_code (g) >= GIMPLE_COND && gimple_code (g) <= GIMPLE_RETURN;
 }
 
+template <>
+template <>
+inline bool
+is_a_helper <const gimple_statement_with_ops>::test (const_gimple gs)
+{
+  return gimple_has_ops (gs);
+}
+
+template <>
+template <>
+inline bool
+is_a_helper <gimple_statement_with_ops>::test (gimple gs)
+{
+  return gimple_has_ops (gs);
+}
 
 /* Return true if GIMPLE statement G has memory operands.  */
 
@@ -1731,6 +1746,21 @@ gimple_has_mem_ops (const_gimple g)
   return gimple_code (g) >= GIMPLE_ASSIGN && gimple_code (g) <= GIMPLE_RETURN;
 }
 
+template <>
+template <>
+inline bool
+is_a_helper <const gimple_statement_with_memory_ops>::test (const_gimple gs)
+{
+  return gimple_has_mem_ops (gs);
+}
+
+template <>
+template <>
+inline bool
+is_a_helper <gimple_statement_with_memory_ops>::test (gimple gs)
+{
+  return gimple_has_mem_ops (gs);
+}
 
 /* Return the set of USE operands for statement G.  */
 
-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH 6/6] Add manual GTY hooks
  2013-08-29 16:20 [PATCH 0/6] Convert gimple to a C++ class hierarchy David Malcolm
                   ` (4 preceding siblings ...)
  2013-08-29 16:20 ` [PATCH 3/6] Autogenerated conversion of gimple to C++ David Malcolm
@ 2013-08-29 17:04 ` David Malcolm
  2013-08-29 19:50   ` Steven Bosscher
                     ` (2 more replies)
  2013-08-30 13:58 ` [PATCH 0/6] Convert gimple to a C++ class hierarchy Michael Matz
  2013-08-30 21:51 ` David Malcolm
  7 siblings, 3 replies; 44+ messages in thread
From: David Malcolm @ 2013-08-29 17:04 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

	* gimple.c (gt_ggc_mx (gimple)): New, as required by GTY((user)).
	(gt_pch_nx (gimple)): Likewise.
	(gt_pch_nx (gimple, gt_pointer_operator, void *)): Likewise.
	* gimple.h  (gt_ggc_mx (gimple)): Declare.
	(gt_pch_nx (gimple)): Declare.
	(gt_pch_nx (gimple, gt_pointer_operator, void *)): Declare.
	* tree-cfg.c (ggc_mx (gimple&)): Remove declaration, as this
	collides with the function that GTY((user)) expects.
	(gt_ggc_mx (edge_def *)): Replace call to gt_ggc_mx on the
	gimple with gt_ggc_mx_gimple_statement_base: in the
	pre-GTY((user)) world, "gt_ggc_mx" was the function to be called
	on a possibly NULL pointed to check if needed marking and if so
	to traverse its fields.  In the GTY((user)) world, "gt_ggc_mx"
	is the function to be called on non-NULL objects immediately *after*
	they have been marked: it does not mark the object itself.
	(gt_pch_nx (gimple&)): Remove declaration.
	(gt_pch_nx (edge_def *)): Update as per the mx hook.
---
 gcc/gimple.c   | 743 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gcc/gimple.h   |   6 +
 gcc/tree-cfg.c |   6 +-
 3 files changed, 751 insertions(+), 4 deletions(-)

diff --git a/gcc/gimple.c b/gcc/gimple.c
index 1ad36d1..dd99cda 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -4338,4 +4338,747 @@ build_type_cast (tree to_type, gimple op, enum ssa_mode mode)
   return build_type_cast (to_type, gimple_assign_lhs (op), mode);
 }
 
+void
+gt_ggc_mx (gimple gs)
+{
+  gimple x = gs;
+  /* Emulation of the "chain_next" GTY attribute.
+
+     gs has already been marked.
+     Iterate the chain of next statements, marking until we reach one that
+     has already been marked, or NULL.   */
+  gimple xlimit = gs->next;
+  while (ggc_test_and_set_mark (xlimit))
+    xlimit = xlimit->next;
+
+  /* All of the statements within the half-open interval [x..xlimit) have
+     just been marked.  Iterate through the list, visiting their fields.  */
+  while (x != xlimit)
+    {
+      gt_ggc_m_15basic_block_def (x->bb);
+      switch (gimple_statement_structure (&((*x))))
+	{
+	case GSS_BASE:
+	  break;
+	case GSS_WITH_OPS:
+	  {
+	    gimple_statement_with_ops *stmt
+	      = static_cast <gimple_statement_with_ops *> (x);
+	    size_t num = (size_t)(stmt->num_ops);
+	    for (size_t i = 0; i != num; i++)
+	      gt_ggc_m_9tree_node (stmt->op[i]);
+	  }
+	  break;
+	case GSS_WITH_MEM_OPS_BASE:
+	  break;
+	case GSS_WITH_MEM_OPS:
+	  {
+	    gimple_statement_with_memory_ops *stmt
+	      = static_cast <gimple_statement_with_memory_ops *> (x);
+	    size_t num = (size_t)(stmt->num_ops);
+	    for (size_t i = 0; i != num; i++)
+	      gt_ggc_m_9tree_node (stmt->op[i]);
+	  }
+	  break;
+	case GSS_CALL:
+	  {
+	    gimple_statement_call *stmt
+	      = static_cast <gimple_statement_call *> (x);
+	    gt_ggc_m_15bitmap_head_def (stmt->call_used.vars);
+	    gt_ggc_m_15bitmap_head_def (stmt->call_clobbered.vars);
+	    switch (stmt->subcode & GF_CALL_INTERNAL)
+	      {
+	      case 0:
+		gt_ggc_m_9tree_node (stmt->u.fntype);
+		break;
+	      case GF_CALL_INTERNAL:
+		break;
+	      default:
+		break;
+	      }
+	    size_t num = (size_t)(stmt->num_ops);
+	    for (size_t i = 0; i != num; i++)
+	      gt_ggc_m_9tree_node (stmt->op[i]);
+	  }
+	  break;
+	case GSS_OMP:
+	  {
+	    gimple_statement_omp *stmt
+	      = static_cast <gimple_statement_omp *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	  }
+	  break;
+	case GSS_BIND:
+	  {
+	     gimple_statement_bind *stmt
+	      = static_cast <gimple_statement_bind *> (x);
+	    gt_ggc_m_9tree_node (stmt->vars);
+	    gt_ggc_m_9tree_node (stmt->block);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	  }
+	  break;
+	case GSS_CATCH:
+	  {
+	    gimple_statement_catch *stmt
+	      = static_cast <gimple_statement_catch *> (x);
+	    gt_ggc_m_9tree_node (stmt->types);
+	    gt_ggc_mx_gimple_statement_base (stmt->handler);
+	  }
+	  break;
+	case GSS_EH_FILTER:
+	  {
+	    gimple_statement_eh_filter *stmt
+	      = static_cast <gimple_statement_eh_filter *> (x);
+	    gt_ggc_m_9tree_node (stmt->types);
+	    gt_ggc_mx_gimple_statement_base (stmt->failure);
+	  }
+	  break;
+	case GSS_EH_MNT:
+	  {
+	    gimple_statement_eh_mnt *stmt
+	      = static_cast <gimple_statement_eh_mnt *> (x);
+	    gt_ggc_m_9tree_node (stmt->fndecl);
+	  }
+	  break;
+	case GSS_EH_ELSE:
+	  {
+	    gimple_statement_eh_else*stmt
+	      = static_cast <gimple_statement_eh_else *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->n_body);
+	    gt_ggc_mx_gimple_statement_base (stmt->e_body);
+	  }
+	  break;
+	case GSS_PHI:
+	  {
+	    gimple_statement_phi *stmt
+	      = static_cast <gimple_statement_phi *> (x);
+	    size_t num = (size_t)(stmt->nargs);
+	    gt_ggc_m_9tree_node (stmt->result);
+	    for (size_t i = 0; i != num; i++)
+	      gt_ggc_m_9tree_node (stmt->args[i].def);
+	  }
+	  break;
+	case GSS_EH_CTRL:
+	  break;
+	case GSS_TRY:
+	  {
+	    gimple_statement_try *stmt
+	      = static_cast <gimple_statement_try *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->eval);
+	    gt_ggc_mx_gimple_statement_base (stmt->cleanup);
+	  }
+	  break;
+	case GSS_WCE:
+	  {
+	    gimple_statement_wce *stmt
+	      = static_cast <gimple_statement_wce *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->cleanup);
+	  }
+	  break;
+	case GSS_ASM:
+	  {
+	    gimple_statement_asm *stmt
+	      = static_cast <gimple_statement_asm *> (x);
+	    size_t num = (size_t)(stmt->num_ops);
+	    gt_ggc_m_S (stmt->string);
+	    for (size_t i = 0; i != num; i++)
+	      gt_ggc_m_9tree_node (stmt->op[i]);
+	  }
+	  break;
+	case GSS_OMP_CRITICAL:
+	  {
+	    gimple_statement_omp_critical *stmt
+	      = static_cast <gimple_statement_omp_critical *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	    gt_ggc_m_9tree_node (stmt->name);
+	  }
+	  break;
+	case GSS_OMP_FOR:
+	  {
+	    gimple_statement_omp_for *stmt
+	      = static_cast <gimple_statement_omp_for *> (x);
+	    size_t num = (size_t)(stmt->collapse);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	    gt_ggc_m_9tree_node (stmt->clauses);
+	    if (stmt->iter != NULL) {
+	      for (size_t i = 0; i != num; i++) {
+		gt_ggc_m_9tree_node (stmt->iter[i].index);
+		gt_ggc_m_9tree_node (stmt->iter[i].initial);
+		gt_ggc_m_9tree_node (stmt->iter[i].final);
+		gt_ggc_m_9tree_node (stmt->iter[i].incr);
+	      }
+	      ggc_mark (stmt->iter);
+	    }
+	    gt_ggc_mx_gimple_statement_base (stmt->pre_body);
+	  }
+	  break;
+	case GSS_OMP_PARALLEL:
+	  {
+	    gimple_statement_omp_parallel *stmt
+	      = static_cast <gimple_statement_omp_parallel *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	    gt_ggc_m_9tree_node (stmt->clauses);
+	    gt_ggc_m_9tree_node (stmt->child_fn);
+	    gt_ggc_m_9tree_node (stmt->data_arg);
+	  }
+	  break;
+	case GSS_OMP_TASK:
+	  {
+	    gimple_statement_omp_task *stmt
+	      = static_cast <gimple_statement_omp_task *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	    gt_ggc_m_9tree_node (stmt->clauses);
+	    gt_ggc_m_9tree_node (stmt->child_fn);
+	    gt_ggc_m_9tree_node (stmt->data_arg);
+	    gt_ggc_m_9tree_node (stmt->copy_fn);
+	    gt_ggc_m_9tree_node (stmt->arg_size);
+	    gt_ggc_m_9tree_node (stmt->arg_align);
+	  }
+	  break;
+	case GSS_OMP_SECTIONS:
+	  {
+	    gimple_statement_omp_sections *stmt
+	      = static_cast <gimple_statement_omp_sections *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	    gt_ggc_m_9tree_node (stmt->clauses);
+	    gt_ggc_m_9tree_node (stmt->control);
+	  }
+	  break;
+	case GSS_OMP_SINGLE:
+	  {
+	    gimple_statement_omp_single *stmt
+	      = static_cast <gimple_statement_omp_single *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	    gt_ggc_m_9tree_node (stmt->clauses);
+	  }
+	  break;
+	case GSS_OMP_CONTINUE:
+	  {
+	    gimple_statement_omp_continue *stmt
+	      = static_cast <gimple_statement_omp_continue *> (x);
+	    gt_ggc_m_9tree_node (stmt->control_def);
+	    gt_ggc_m_9tree_node (stmt->control_use);
+	  }
+	  break;
+	case GSS_OMP_ATOMIC_LOAD:
+	  {
+	    gimple_statement_omp_atomic_load *stmt
+	      = static_cast <gimple_statement_omp_atomic_load *> (x);
+	    gt_ggc_m_9tree_node (stmt->rhs);
+	    gt_ggc_m_9tree_node (stmt->lhs);
+	  }
+	  break;
+	case GSS_OMP_ATOMIC_STORE:
+	  {
+	    gimple_statement_omp_atomic_store *stmt
+	      = static_cast <gimple_statement_omp_atomic_store *> (x);
+	    gt_ggc_m_9tree_node (stmt->val);
+	  }
+	  break;
+	case GSS_TRANSACTION:
+	  {
+	    gimple_statement_transaction *stmt
+	      = static_cast <gimple_statement_transaction *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	    gt_ggc_m_9tree_node (stmt->label);
+	  }
+	  break;
+	default:
+	  break;
+	}
+      x = x->next;
+    }
+}
+
+void
+gt_pch_nx (gimple gs)
+{
+  gimple x = gs;
+  /* Emulation of the "chain_next" GTY attribute.
+
+     gs has already been marked.
+     Iterate the chain of next statements, marking until we reach one that
+     has already been marked, or NULL.   */
+  gimple xlimit = gs->next;
+  while (gt_pch_note_object (xlimit, xlimit, gt_pch_p_21gimple_statement_base))
+    xlimit = xlimit->next;
+
+  /* All of the statements within the half-open interval [x..xlimit) have
+     just been marked.  Iterate through the list, visiting their fields.  */
+  while (x != xlimit)
+    {
+      gt_pch_n_15basic_block_def (x->bb);
+      switch (gimple_statement_structure (&((*x))))
+	{
+	case GSS_BASE:
+	  break;
+	case GSS_WITH_OPS:
+	  {
+	    gimple_statement_with_ops *stmt
+	      = static_cast <gimple_statement_with_ops *> (x);
+	    size_t num = (size_t)(stmt->num_ops);
+	    for (size_t i = 0; i != num; i++)
+	      gt_pch_n_9tree_node (stmt->op[i]);
+	  }
+	  break;
+	case GSS_WITH_MEM_OPS_BASE:
+	  break;
+	case GSS_WITH_MEM_OPS:
+	  {
+	    gimple_statement_with_memory_ops *stmt
+	      = static_cast <gimple_statement_with_memory_ops *> (x);
+	    size_t num = (size_t)(stmt->num_ops);
+	    for (size_t i = 0; i != num; i++)
+	      gt_pch_n_9tree_node (stmt->op[i]);
+	  }
+	  break;
+	case GSS_CALL:
+	  {
+	    gimple_statement_call *stmt
+	      = static_cast <gimple_statement_call *> (x);
+	    gt_pch_n_15bitmap_head_def (stmt->call_used.vars);
+	    gt_pch_n_15bitmap_head_def (stmt->call_clobbered.vars);
+	    switch (stmt->subcode & GF_CALL_INTERNAL)
+	      {
+	      case 0:
+		gt_pch_n_9tree_node (stmt->u.fntype);
+		break;
+	      case GF_CALL_INTERNAL:
+		break;
+	      default:
+		break;
+	      }
+	    size_t num = (size_t)(stmt->num_ops);
+	    for (size_t i = 0; i != num; i++)
+	      gt_pch_n_9tree_node (stmt->op[i]);
+	  }
+	  break;
+	case GSS_OMP:
+	  {
+	    gimple_statement_omp *stmt
+	      = static_cast <gimple_statement_omp *> (x);
+	    gt_pch_nx_gimple_statement_base (stmt->body);
+	  }
+	  break;
+	case GSS_BIND:
+	  {
+	     gimple_statement_bind *stmt
+	      = static_cast <gimple_statement_bind *> (x);
+	    gt_pch_n_9tree_node (stmt->vars);
+	    gt_pch_n_9tree_node (stmt->block);
+	    gt_pch_nx_gimple_statement_base (stmt->body);
+	  }
+	  break;
+	case GSS_CATCH:
+	  {
+	    gimple_statement_catch *stmt
+	      = static_cast <gimple_statement_catch *> (x);
+	    gt_pch_n_9tree_node (stmt->types);
+	    gt_pch_nx_gimple_statement_base (stmt->handler);
+	  }
+	  break;
+	case GSS_EH_FILTER:
+	  {
+	    gimple_statement_eh_filter *stmt
+	      = static_cast <gimple_statement_eh_filter *> (x);
+	    gt_pch_n_9tree_node (stmt->types);
+	    gt_pch_nx_gimple_statement_base (stmt->failure);
+	  }
+	  break;
+	case GSS_EH_MNT:
+	  {
+	    gimple_statement_eh_mnt *stmt
+	      = static_cast <gimple_statement_eh_mnt *> (x);
+	    gt_pch_n_9tree_node (stmt->fndecl);
+	  }
+	  break;
+	case GSS_EH_ELSE:
+	  {
+	    gimple_statement_eh_else*stmt
+	      = static_cast <gimple_statement_eh_else *> (x);
+	    gt_pch_nx_gimple_statement_base (stmt->n_body);
+	    gt_pch_nx_gimple_statement_base (stmt->e_body);
+	  }
+	  break;
+	case GSS_PHI:
+	  {
+	    gimple_statement_phi *stmt
+	      = static_cast <gimple_statement_phi *> (x);
+	    size_t num = (size_t)(stmt->nargs);
+	    gt_pch_n_9tree_node (stmt->result);
+	    for (size_t i = 0; i != num; i++)
+	      gt_pch_n_9tree_node (stmt->args[i].def);
+	  }
+	  break;
+	case GSS_EH_CTRL:
+	  break;
+	case GSS_TRY:
+	  {
+	    gimple_statement_try *stmt
+	      = static_cast <gimple_statement_try *> (x);
+	    gt_pch_nx_gimple_statement_base (stmt->eval);
+	    gt_pch_nx_gimple_statement_base (stmt->cleanup);
+	  }
+	  break;
+	case GSS_WCE:
+	  {
+	    gimple_statement_wce *stmt
+	      = static_cast <gimple_statement_wce *> (x);
+	    gt_pch_nx_gimple_statement_base (stmt->cleanup);
+	  }
+	  break;
+	case GSS_ASM:
+	  {
+	    gimple_statement_asm *stmt
+	      = static_cast <gimple_statement_asm *> (x);
+	    size_t num = (size_t)(stmt->num_ops);
+	    gt_pch_n_S (stmt->string);
+	    for (size_t i = 0; i != num; i++)
+	      gt_pch_n_9tree_node (stmt->op[i]);
+	  }
+	  break;
+	case GSS_OMP_CRITICAL:
+	  {
+	    gimple_statement_omp_critical *stmt
+	      = static_cast <gimple_statement_omp_critical *> (x);
+	    gt_pch_nx_gimple_statement_base (stmt->body);
+	    gt_pch_n_9tree_node (stmt->name);
+	  }
+	  break;
+	case GSS_OMP_FOR:
+	  {
+	    gimple_statement_omp_for *stmt
+	      = static_cast <gimple_statement_omp_for *> (x);
+	    size_t num = (size_t)(stmt->collapse);
+	    gt_pch_nx_gimple_statement_base (stmt->body);
+	    gt_pch_n_9tree_node (stmt->clauses);
+	    if (stmt->iter != NULL) {
+	      for (size_t i = 0; i != num; i++) {
+		gt_pch_n_9tree_node (stmt->iter[i].index);
+		gt_pch_n_9tree_node (stmt->iter[i].initial);
+		gt_pch_n_9tree_node (stmt->iter[i].final);
+		gt_pch_n_9tree_node (stmt->iter[i].incr);
+	      }
+	      gt_pch_note_object (stmt->iter, x,
+				  gt_pch_p_21gimple_statement_base);
+	    }
+	    gt_pch_nx_gimple_statement_base (stmt->pre_body);
+	  }
+	  break;
+	case GSS_OMP_PARALLEL:
+	  {
+	    gimple_statement_omp_parallel *stmt
+	      = static_cast <gimple_statement_omp_parallel *> (x);
+	    gt_pch_nx_gimple_statement_base (stmt->body);
+	    gt_pch_n_9tree_node (stmt->clauses);
+	    gt_pch_n_9tree_node (stmt->child_fn);
+	    gt_pch_n_9tree_node (stmt->data_arg);
+	  }
+	  break;
+	case GSS_OMP_TASK:
+	  {
+	    gimple_statement_omp_task *stmt
+	      = static_cast <gimple_statement_omp_task *> (x);
+	    gt_pch_nx_gimple_statement_base (stmt->body);
+	    gt_pch_n_9tree_node (stmt->clauses);
+	    gt_pch_n_9tree_node (stmt->child_fn);
+	    gt_pch_n_9tree_node (stmt->data_arg);
+	    gt_pch_n_9tree_node (stmt->copy_fn);
+	    gt_pch_n_9tree_node (stmt->arg_size);
+	    gt_pch_n_9tree_node (stmt->arg_align);
+	  }
+	  break;
+	case GSS_OMP_SECTIONS:
+	  {
+	    gimple_statement_omp_sections *stmt
+	      = static_cast <gimple_statement_omp_sections *> (x);
+	    gt_pch_nx_gimple_statement_base (stmt->body);
+	    gt_pch_n_9tree_node (stmt->clauses);
+	    gt_pch_n_9tree_node (stmt->control);
+	  }
+	  break;
+	case GSS_OMP_SINGLE:
+	  {
+	    gimple_statement_omp_single *stmt
+	      = static_cast <gimple_statement_omp_single *> (x);
+	    gt_pch_nx_gimple_statement_base (stmt->body);
+	    gt_pch_n_9tree_node (stmt->clauses);
+	  }
+	  break;
+	case GSS_OMP_CONTINUE:
+	  {
+	    gimple_statement_omp_continue *stmt
+	      = static_cast <gimple_statement_omp_continue *> (x);
+	    gt_pch_n_9tree_node (stmt->control_def);
+	    gt_pch_n_9tree_node (stmt->control_use);
+	  }
+	  break;
+	case GSS_OMP_ATOMIC_LOAD:
+	  {
+	    gimple_statement_omp_atomic_load *stmt
+	      = static_cast <gimple_statement_omp_atomic_load *> (x);
+	    gt_pch_n_9tree_node (stmt->rhs);
+	    gt_pch_n_9tree_node (stmt->lhs);
+	  }
+	  break;
+	case GSS_OMP_ATOMIC_STORE:
+	  {
+	    gimple_statement_omp_atomic_store *stmt
+	      = static_cast <gimple_statement_omp_atomic_store *> (x);
+	    gt_pch_n_9tree_node (stmt->val);
+	  }
+	  break;
+	case GSS_TRANSACTION:
+	  {
+	    gimple_statement_transaction *stmt
+	      = static_cast <gimple_statement_transaction *> (x);
+	    gt_pch_nx_gimple_statement_base (stmt->body);
+	    gt_pch_n_9tree_node (stmt->label);
+	  }
+	  break;
+	default:
+	  break;
+	}
+      x = x->next;
+    }
+}
+
+void
+gt_pch_nx (gimple gs, gt_pointer_operator op, void *cookie)
+{
+  op (&(gs->bb), cookie);
+  op (&(gs->next), cookie);
+  switch (gimple_statement_structure (gs))
+    {
+    case GSS_BASE:
+      break;
+    case GSS_WITH_OPS:
+      {
+	gimple_statement_with_ops *stmt
+	  = static_cast <gimple_statement_with_ops *> (gs);
+	size_t num = (size_t)(stmt->num_ops);
+	for (size_t i = 0; i != num; i++)
+	  op (&(stmt->op[i]), cookie);
+      }
+      break;
+    case GSS_WITH_MEM_OPS_BASE:
+      break;
+    case GSS_WITH_MEM_OPS:
+      {
+	gimple_statement_with_memory_ops *stmt
+	  = static_cast <gimple_statement_with_memory_ops *> (gs);
+	size_t num = (size_t)(stmt->num_ops);
+	for (size_t i = 0; i != num; i++)
+	  op (&(stmt->op[i]), cookie);
+      }
+      break;
+    case GSS_CALL:
+      {
+	gimple_statement_call *stmt
+	  = static_cast <gimple_statement_call *> (gs);
+	size_t num = (size_t)(stmt->num_ops);
+	op (&(stmt->call_used.vars), cookie);
+	op (&(stmt->call_clobbered.vars), cookie);
+	switch (stmt->subcode & GF_CALL_INTERNAL)
+	  {
+	  case 0:
+	    op (&(stmt->u.fntype), cookie);
+	    break;
+	  case GF_CALL_INTERNAL:
+	    break;
+	  default:
+	    break;
+	  }
+	for (size_t i = 0; i != num; i++)
+	  op (&(stmt->op[i]), cookie);
+      }
+      break;
+    case GSS_OMP:
+      {
+	gimple_statement_omp *stmt
+	  = static_cast <gimple_statement_omp *> (gs);
+	op (&(stmt->body), cookie);
+      }
+      break;
+    case GSS_BIND:
+      {
+	gimple_statement_bind *stmt
+	  = static_cast <gimple_statement_bind *> (gs);
+	op (&(stmt->vars), cookie);
+	op (&(stmt->block), cookie);
+	op (&(stmt->body), cookie);
+      }
+      break;
+    case GSS_CATCH:
+      {
+	gimple_statement_catch *stmt
+	  = static_cast <gimple_statement_catch *> (gs);
+	op (&(stmt->types), cookie);
+	op (&(stmt->handler), cookie);
+      }
+      break;
+    case GSS_EH_FILTER:
+      {
+	gimple_statement_eh_filter *stmt
+	  = static_cast <gimple_statement_eh_filter *> (gs);
+	op (&(stmt->types), cookie);
+	op (&(stmt->failure), cookie);
+      }
+      break;
+    case GSS_EH_MNT:
+      {
+	gimple_statement_eh_mnt *stmt
+	  = static_cast <gimple_statement_eh_mnt *> (gs);
+	op (&(stmt->fndecl), cookie);
+      }
+      break;
+    case GSS_EH_ELSE:
+      {
+	gimple_statement_eh_else*stmt
+	  = static_cast <gimple_statement_eh_else *> (gs);
+	op (&(stmt->n_body), cookie);
+	op (&(stmt->e_body), cookie);
+      }
+      break;
+    case GSS_PHI:
+      {
+	gimple_statement_phi *stmt
+	  = static_cast <gimple_statement_phi *> (gs);
+	size_t num = (size_t)(stmt->nargs);
+	op (&(stmt->result), cookie);
+	for (size_t i = 0; i != num; i++)
+	  op (&(stmt->args[i].def), cookie);
+      }
+      break;
+    case GSS_EH_CTRL:
+      break;
+    case GSS_TRY:
+      {
+	gimple_statement_try *stmt
+	  = static_cast <gimple_statement_try *> (gs);
+	op (&(stmt->eval), cookie);
+	op (&(stmt->cleanup), cookie);
+      }
+      break;
+    case GSS_WCE:
+      {
+	gimple_statement_wce *stmt
+	  = static_cast <gimple_statement_wce *> (gs);
+	op (&(stmt->cleanup), cookie);
+      }
+      break;
+    case GSS_ASM:
+      {
+	gimple_statement_asm *stmt
+	  = static_cast <gimple_statement_asm *> (gs);
+	size_t num = (size_t)(stmt->num_ops);
+	op (&(stmt->string), cookie);
+	for (size_t i = 0; i != num; i++)
+	  op (&(stmt->op[i]), cookie);
+      }
+      break;
+    case GSS_OMP_CRITICAL:
+      {
+	gimple_statement_omp_critical *stmt
+	  = static_cast <gimple_statement_omp_critical *> (gs);
+	op (&(stmt->body), cookie);
+	op (&(stmt->name), cookie);
+      }
+      break;
+    case GSS_OMP_FOR:
+      {
+	gimple_statement_omp_for *stmt
+	  = static_cast <gimple_statement_omp_for *> (gs);
+	size_t num = (size_t)(stmt->collapse);
+	op (&(stmt->body), cookie);
+	op (&(stmt->clauses), cookie);
+	if (stmt->iter != NULL) {
+	  for (size_t i = 0; i != num; i++) {
+	    op (&(stmt->iter[i].index), cookie);
+	    op (&(stmt->iter[i].initial), cookie);
+	    op (&(stmt->iter[i].final), cookie);
+	    op (&(stmt->iter[i].incr), cookie);
+	  }
+	  op (&(stmt->iter), cookie);
+	}
+	op (&(stmt->pre_body), cookie);
+      }
+      break;
+    case GSS_OMP_PARALLEL:
+      {
+	gimple_statement_omp_parallel *stmt
+	  = static_cast <gimple_statement_omp_parallel *> (gs);
+	op (&(stmt->body), cookie);
+	op (&(stmt->clauses), cookie);
+	op (&(stmt->child_fn), cookie);
+	op (&(stmt->data_arg), cookie);
+      }
+      break;
+    case GSS_OMP_TASK:
+      {
+	gimple_statement_omp_task *stmt
+	  = static_cast <gimple_statement_omp_task *> (gs);
+	op (&(stmt->body), cookie);
+	op (&(stmt->clauses), cookie);
+	op (&(stmt->child_fn), cookie);
+	op (&(stmt->data_arg), cookie);
+	op (&(stmt->copy_fn), cookie);
+	op (&(stmt->arg_size), cookie);
+	op (&(stmt->arg_align), cookie);
+      }
+      break;
+    case GSS_OMP_SECTIONS:
+      {
+	gimple_statement_omp_sections *stmt
+	  = static_cast <gimple_statement_omp_sections *> (gs);
+	op (&(stmt->body), cookie);
+	op (&(stmt->clauses), cookie);
+	op (&(stmt->control), cookie);
+      }
+      break;
+    case GSS_OMP_SINGLE:
+      {
+	gimple_statement_omp_single *stmt
+	  = static_cast <gimple_statement_omp_single *> (gs);
+	op (&(stmt->body), cookie);
+	op (&(stmt->clauses), cookie);
+      }
+      break;
+    case GSS_OMP_CONTINUE:
+      {
+	gimple_statement_omp_continue *stmt
+	  = static_cast <gimple_statement_omp_continue *> (gs);
+	op (&(stmt->control_def), cookie);
+	op (&(stmt->control_use), cookie);
+      }
+      break;
+    case GSS_OMP_ATOMIC_LOAD:
+      {
+	gimple_statement_omp_atomic_load *stmt
+	  = static_cast <gimple_statement_omp_atomic_load *> (gs);
+	op (&(stmt->rhs), cookie);
+	op (&(stmt->lhs), cookie);
+      }
+      break;
+    case GSS_OMP_ATOMIC_STORE:
+      {
+	gimple_statement_omp_atomic_store *stmt
+	  = static_cast <gimple_statement_omp_atomic_store *> (gs);
+	op (&(stmt->val), cookie);
+      }
+      break;
+    case GSS_TRANSACTION:
+      {
+	gimple_statement_transaction *stmt
+	  = static_cast <gimple_statement_transaction *> (gs);
+	op (&(stmt->body), cookie);
+	op (&(stmt->label), cookie);
+      }
+      break;
+    default:
+      break;
+    }
+}
+
+
 #include "gt-gimple.h"
diff --git a/gcc/gimple.h b/gcc/gimple.h
index daab54e..9b428eb 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -222,6 +222,12 @@ struct GTY((user)) gimple_statement_base {
   gimple GTY((skip)) prev;
 };
 
+/* GTY((user)) hooks for gimple, called once per-traversal.  */
+void gt_ggc_mx (gimple gs);
+void gt_pch_nx (gimple gs);
+void gt_pch_nx (gimple gs, gt_pointer_operator op, void *cookie);
+
+
 
 /* Base structure for tuples with operands.  */
 
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index af8685c..185c072 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -8291,7 +8291,6 @@ make_pass_warn_unused_result (gcc::context *ctxt)
 /* Garbage collection support for edge_def.  */
 
 extern void gt_ggc_mx (tree&);
-extern void gt_ggc_mx (gimple&);
 extern void gt_ggc_mx (rtx&);
 extern void gt_ggc_mx (basic_block&);
 
@@ -8302,7 +8301,7 @@ gt_ggc_mx (edge_def *e)
   gt_ggc_mx (e->src);
   gt_ggc_mx (e->dest);
   if (current_ir_type () == IR_GIMPLE)
-    gt_ggc_mx (e->insns.g);
+    gt_ggc_mx_gimple_statement_base (e->insns.g);
   else
     gt_ggc_mx (e->insns.r);
   gt_ggc_mx (block);
@@ -8311,7 +8310,6 @@ gt_ggc_mx (edge_def *e)
 /* PCH support for edge_def.  */
 
 extern void gt_pch_nx (tree&);
-extern void gt_pch_nx (gimple&);
 extern void gt_pch_nx (rtx&);
 extern void gt_pch_nx (basic_block&);
 
@@ -8322,7 +8320,7 @@ gt_pch_nx (edge_def *e)
   gt_pch_nx (e->src);
   gt_pch_nx (e->dest);
   if (current_ir_type () == IR_GIMPLE)
-    gt_pch_nx (e->insns.g);
+    gt_pch_nx_gimple_statement_base (e->insns.g);
   else
     gt_pch_nx (e->insns.r);
   gt_pch_nx (block);
-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 6/6] Add manual GTY hooks
  2013-08-29 17:04 ` [PATCH 6/6] Add manual GTY hooks David Malcolm
@ 2013-08-29 19:50   ` Steven Bosscher
  2013-08-30  8:12     ` Richard Biener
  2013-08-30  8:09   ` Richard Biener
  2013-08-30  8:43   ` [PATCH 6/6] " Richard Biener
  2 siblings, 1 reply; 44+ messages in thread
From: Steven Bosscher @ 2013-08-29 19:50 UTC (permalink / raw)
  To: David Malcolm; +Cc: GCC Patches

On Thu, Aug 29, 2013 at 6:20 PM, David Malcolm <dmalcolm@redhat.com> wrote:
>         * gimple.c (gt_ggc_mx (gimple)): New, as required by GTY((user)).
>         (gt_pch_nx (gimple)): Likewise.

GIMPLE isn't supposed to end up in a PCH. Can you please make this
function simply call gcc_unreachable()?

FWIW 1: I really think all these hand-written markers aren't a good
idea, we should really figure out a way to have automatic marker
function generators, something less complex than gengtype, of course.
But to have all these calls to the type-mangled marker functions
(gt_pch_n_9tree_node, etc.) is going to be a problem in the long term.

It seems to me that the first step in all these conversions to
hand-written markers should be to make gengtype spit out the marker
functions *without* the type name mangling, i.e. all marker functions
should just be gt_ggc_mx(type) / gt_pch_nx(type).

Ciao!
Steven

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 6/6] Add manual GTY hooks
  2013-08-29 17:04 ` [PATCH 6/6] Add manual GTY hooks David Malcolm
  2013-08-29 19:50   ` Steven Bosscher
@ 2013-08-30  8:09   ` Richard Biener
  2013-08-31 13:48     ` [PATCH 6/6 v2] " David Malcolm
  2013-08-30  8:43   ` [PATCH 6/6] " Richard Biener
  2 siblings, 1 reply; 44+ messages in thread
From: Richard Biener @ 2013-08-30  8:09 UTC (permalink / raw)
  To: David Malcolm; +Cc: GCC Patches

On Thu, Aug 29, 2013 at 6:20 PM, David Malcolm <dmalcolm@redhat.com> wrote:
>         * gimple.c (gt_ggc_mx (gimple)): New, as required by GTY((user)).
>         (gt_pch_nx (gimple)): Likewise.
>         (gt_pch_nx (gimple, gt_pointer_operator, void *)): Likewise.
>         * gimple.h  (gt_ggc_mx (gimple)): Declare.
>         (gt_pch_nx (gimple)): Declare.
>         (gt_pch_nx (gimple, gt_pointer_operator, void *)): Declare.

No GIMPLE should reside in PCHs so you should be able to just put
gcc_unreachable () in them ... (if dropping them does not work)

>         * tree-cfg.c (ggc_mx (gimple&)): Remove declaration, as this
>         collides with the function that GTY((user)) expects.
>         (gt_ggc_mx (edge_def *)): Replace call to gt_ggc_mx on the
>         gimple with gt_ggc_mx_gimple_statement_base: in the
>         pre-GTY((user)) world, "gt_ggc_mx" was the function to be called
>         on a possibly NULL pointed to check if needed marking and if so
>         to traverse its fields.  In the GTY((user)) world, "gt_ggc_mx"
>         is the function to be called on non-NULL objects immediately *after*
>         they have been marked: it does not mark the object itself.
>         (gt_pch_nx (gimple&)): Remove declaration.
>         (gt_pch_nx (edge_def *)): Update as per the mx hook.
> ---
>  gcc/gimple.c   | 743 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  gcc/gimple.h   |   6 +
>  gcc/tree-cfg.c |   6 +-
>  3 files changed, 751 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/gimple.c b/gcc/gimple.c
> index 1ad36d1..dd99cda 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -4338,4 +4338,747 @@ build_type_cast (tree to_type, gimple op, enum ssa_mode mode)
>    return build_type_cast (to_type, gimple_assign_lhs (op), mode);
>  }
>
> +void
> +gt_ggc_mx (gimple gs)
> +{
> +  gimple x = gs;
> +  /* Emulation of the "chain_next" GTY attribute.
> +
> +     gs has already been marked.
> +     Iterate the chain of next statements, marking until we reach one that
> +     has already been marked, or NULL.   */
> +  gimple xlimit = gs->next;
> +  while (ggc_test_and_set_mark (xlimit))
> +    xlimit = xlimit->next;
> +
> +  /* All of the statements within the half-open interval [x..xlimit) have
> +     just been marked.  Iterate through the list, visiting their fields.  */
> +  while (x != xlimit)
> +    {
> +      gt_ggc_m_15basic_block_def (x->bb);
> +      switch (gimple_statement_structure (&((*x))))
> +       {
> +       case GSS_BASE:
> +         break;
> +       case GSS_WITH_OPS:
> +         {
> +           gimple_statement_with_ops *stmt
> +             = static_cast <gimple_statement_with_ops *> (x);
> +           size_t num = (size_t)(stmt->num_ops);
> +           for (size_t i = 0; i != num; i++)
> +             gt_ggc_m_9tree_node (stmt->op[i]);
> +         }
> +         break;
> +       case GSS_WITH_MEM_OPS_BASE:
> +         break;
> +       case GSS_WITH_MEM_OPS:
> +         {
> +           gimple_statement_with_memory_ops *stmt
> +             = static_cast <gimple_statement_with_memory_ops *> (x);
> +           size_t num = (size_t)(stmt->num_ops);
> +           for (size_t i = 0; i != num; i++)
> +             gt_ggc_m_9tree_node (stmt->op[i]);
> +         }
> +         break;
> +       case GSS_CALL:
> +         {
> +           gimple_statement_call *stmt
> +             = static_cast <gimple_statement_call *> (x);
> +           gt_ggc_m_15bitmap_head_def (stmt->call_used.vars);
> +           gt_ggc_m_15bitmap_head_def (stmt->call_clobbered.vars);
> +           switch (stmt->subcode & GF_CALL_INTERNAL)
> +             {
> +             case 0:
> +               gt_ggc_m_9tree_node (stmt->u.fntype);
> +               break;
> +             case GF_CALL_INTERNAL:
> +               break;
> +             default:
> +               break;
> +             }
> +           size_t num = (size_t)(stmt->num_ops);
> +           for (size_t i = 0; i != num; i++)
> +             gt_ggc_m_9tree_node (stmt->op[i]);
> +         }
> +         break;
> +       case GSS_OMP:
> +         {
> +           gimple_statement_omp *stmt
> +             = static_cast <gimple_statement_omp *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +         }
> +         break;
> +       case GSS_BIND:
> +         {
> +            gimple_statement_bind *stmt
> +             = static_cast <gimple_statement_bind *> (x);
> +           gt_ggc_m_9tree_node (stmt->vars);
> +           gt_ggc_m_9tree_node (stmt->block);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +         }
> +         break;
> +       case GSS_CATCH:
> +         {
> +           gimple_statement_catch *stmt
> +             = static_cast <gimple_statement_catch *> (x);
> +           gt_ggc_m_9tree_node (stmt->types);
> +           gt_ggc_mx_gimple_statement_base (stmt->handler);
> +         }
> +         break;
> +       case GSS_EH_FILTER:
> +         {
> +           gimple_statement_eh_filter *stmt
> +             = static_cast <gimple_statement_eh_filter *> (x);
> +           gt_ggc_m_9tree_node (stmt->types);
> +           gt_ggc_mx_gimple_statement_base (stmt->failure);
> +         }
> +         break;
> +       case GSS_EH_MNT:
> +         {
> +           gimple_statement_eh_mnt *stmt
> +             = static_cast <gimple_statement_eh_mnt *> (x);
> +           gt_ggc_m_9tree_node (stmt->fndecl);
> +         }
> +         break;
> +       case GSS_EH_ELSE:
> +         {
> +           gimple_statement_eh_else*stmt
> +             = static_cast <gimple_statement_eh_else *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->n_body);
> +           gt_ggc_mx_gimple_statement_base (stmt->e_body);
> +         }
> +         break;
> +       case GSS_PHI:
> +         {
> +           gimple_statement_phi *stmt
> +             = static_cast <gimple_statement_phi *> (x);
> +           size_t num = (size_t)(stmt->nargs);
> +           gt_ggc_m_9tree_node (stmt->result);
> +           for (size_t i = 0; i != num; i++)
> +             gt_ggc_m_9tree_node (stmt->args[i].def);
> +         }
> +         break;
> +       case GSS_EH_CTRL:
> +         break;
> +       case GSS_TRY:
> +         {
> +           gimple_statement_try *stmt
> +             = static_cast <gimple_statement_try *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->eval);
> +           gt_ggc_mx_gimple_statement_base (stmt->cleanup);
> +         }
> +         break;
> +       case GSS_WCE:
> +         {
> +           gimple_statement_wce *stmt
> +             = static_cast <gimple_statement_wce *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->cleanup);
> +         }
> +         break;
> +       case GSS_ASM:
> +         {
> +           gimple_statement_asm *stmt
> +             = static_cast <gimple_statement_asm *> (x);
> +           size_t num = (size_t)(stmt->num_ops);
> +           gt_ggc_m_S (stmt->string);
> +           for (size_t i = 0; i != num; i++)
> +             gt_ggc_m_9tree_node (stmt->op[i]);
> +         }
> +         break;
> +       case GSS_OMP_CRITICAL:
> +         {
> +           gimple_statement_omp_critical *stmt
> +             = static_cast <gimple_statement_omp_critical *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +           gt_ggc_m_9tree_node (stmt->name);
> +         }
> +         break;
> +       case GSS_OMP_FOR:
> +         {
> +           gimple_statement_omp_for *stmt
> +             = static_cast <gimple_statement_omp_for *> (x);
> +           size_t num = (size_t)(stmt->collapse);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +           gt_ggc_m_9tree_node (stmt->clauses);
> +           if (stmt->iter != NULL) {
> +             for (size_t i = 0; i != num; i++) {
> +               gt_ggc_m_9tree_node (stmt->iter[i].index);
> +               gt_ggc_m_9tree_node (stmt->iter[i].initial);
> +               gt_ggc_m_9tree_node (stmt->iter[i].final);
> +               gt_ggc_m_9tree_node (stmt->iter[i].incr);
> +             }
> +             ggc_mark (stmt->iter);
> +           }
> +           gt_ggc_mx_gimple_statement_base (stmt->pre_body);
> +         }
> +         break;
> +       case GSS_OMP_PARALLEL:
> +         {
> +           gimple_statement_omp_parallel *stmt
> +             = static_cast <gimple_statement_omp_parallel *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +           gt_ggc_m_9tree_node (stmt->clauses);
> +           gt_ggc_m_9tree_node (stmt->child_fn);
> +           gt_ggc_m_9tree_node (stmt->data_arg);
> +         }
> +         break;
> +       case GSS_OMP_TASK:
> +         {
> +           gimple_statement_omp_task *stmt
> +             = static_cast <gimple_statement_omp_task *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +           gt_ggc_m_9tree_node (stmt->clauses);
> +           gt_ggc_m_9tree_node (stmt->child_fn);
> +           gt_ggc_m_9tree_node (stmt->data_arg);
> +           gt_ggc_m_9tree_node (stmt->copy_fn);
> +           gt_ggc_m_9tree_node (stmt->arg_size);
> +           gt_ggc_m_9tree_node (stmt->arg_align);
> +         }
> +         break;
> +       case GSS_OMP_SECTIONS:
> +         {
> +           gimple_statement_omp_sections *stmt
> +             = static_cast <gimple_statement_omp_sections *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +           gt_ggc_m_9tree_node (stmt->clauses);
> +           gt_ggc_m_9tree_node (stmt->control);
> +         }
> +         break;
> +       case GSS_OMP_SINGLE:
> +         {
> +           gimple_statement_omp_single *stmt
> +             = static_cast <gimple_statement_omp_single *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +           gt_ggc_m_9tree_node (stmt->clauses);
> +         }
> +         break;
> +       case GSS_OMP_CONTINUE:
> +         {
> +           gimple_statement_omp_continue *stmt
> +             = static_cast <gimple_statement_omp_continue *> (x);
> +           gt_ggc_m_9tree_node (stmt->control_def);
> +           gt_ggc_m_9tree_node (stmt->control_use);
> +         }
> +         break;
> +       case GSS_OMP_ATOMIC_LOAD:
> +         {
> +           gimple_statement_omp_atomic_load *stmt
> +             = static_cast <gimple_statement_omp_atomic_load *> (x);
> +           gt_ggc_m_9tree_node (stmt->rhs);
> +           gt_ggc_m_9tree_node (stmt->lhs);
> +         }
> +         break;
> +       case GSS_OMP_ATOMIC_STORE:
> +         {
> +           gimple_statement_omp_atomic_store *stmt
> +             = static_cast <gimple_statement_omp_atomic_store *> (x);
> +           gt_ggc_m_9tree_node (stmt->val);
> +         }
> +         break;
> +       case GSS_TRANSACTION:
> +         {
> +           gimple_statement_transaction *stmt
> +             = static_cast <gimple_statement_transaction *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +           gt_ggc_m_9tree_node (stmt->label);
> +         }
> +         break;
> +       default:
> +         break;
> +       }
> +      x = x->next;
> +    }
> +}
> +
> +void
> +gt_pch_nx (gimple gs)
> +{
> +  gimple x = gs;
> +  /* Emulation of the "chain_next" GTY attribute.
> +
> +     gs has already been marked.
> +     Iterate the chain of next statements, marking until we reach one that
> +     has already been marked, or NULL.   */
> +  gimple xlimit = gs->next;
> +  while (gt_pch_note_object (xlimit, xlimit, gt_pch_p_21gimple_statement_base))
> +    xlimit = xlimit->next;
> +
> +  /* All of the statements within the half-open interval [x..xlimit) have
> +     just been marked.  Iterate through the list, visiting their fields.  */
> +  while (x != xlimit)
> +    {
> +      gt_pch_n_15basic_block_def (x->bb);
> +      switch (gimple_statement_structure (&((*x))))
> +       {
> +       case GSS_BASE:
> +         break;
> +       case GSS_WITH_OPS:
> +         {
> +           gimple_statement_with_ops *stmt
> +             = static_cast <gimple_statement_with_ops *> (x);
> +           size_t num = (size_t)(stmt->num_ops);
> +           for (size_t i = 0; i != num; i++)
> +             gt_pch_n_9tree_node (stmt->op[i]);
> +         }
> +         break;
> +       case GSS_WITH_MEM_OPS_BASE:
> +         break;
> +       case GSS_WITH_MEM_OPS:
> +         {
> +           gimple_statement_with_memory_ops *stmt
> +             = static_cast <gimple_statement_with_memory_ops *> (x);
> +           size_t num = (size_t)(stmt->num_ops);
> +           for (size_t i = 0; i != num; i++)
> +             gt_pch_n_9tree_node (stmt->op[i]);
> +         }
> +         break;
> +       case GSS_CALL:
> +         {
> +           gimple_statement_call *stmt
> +             = static_cast <gimple_statement_call *> (x);
> +           gt_pch_n_15bitmap_head_def (stmt->call_used.vars);
> +           gt_pch_n_15bitmap_head_def (stmt->call_clobbered.vars);
> +           switch (stmt->subcode & GF_CALL_INTERNAL)
> +             {
> +             case 0:
> +               gt_pch_n_9tree_node (stmt->u.fntype);
> +               break;
> +             case GF_CALL_INTERNAL:
> +               break;
> +             default:
> +               break;
> +             }
> +           size_t num = (size_t)(stmt->num_ops);
> +           for (size_t i = 0; i != num; i++)
> +             gt_pch_n_9tree_node (stmt->op[i]);
> +         }
> +         break;
> +       case GSS_OMP:
> +         {
> +           gimple_statement_omp *stmt
> +             = static_cast <gimple_statement_omp *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +         }
> +         break;
> +       case GSS_BIND:
> +         {
> +            gimple_statement_bind *stmt
> +             = static_cast <gimple_statement_bind *> (x);
> +           gt_pch_n_9tree_node (stmt->vars);
> +           gt_pch_n_9tree_node (stmt->block);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +         }
> +         break;
> +       case GSS_CATCH:
> +         {
> +           gimple_statement_catch *stmt
> +             = static_cast <gimple_statement_catch *> (x);
> +           gt_pch_n_9tree_node (stmt->types);
> +           gt_pch_nx_gimple_statement_base (stmt->handler);
> +         }
> +         break;
> +       case GSS_EH_FILTER:
> +         {
> +           gimple_statement_eh_filter *stmt
> +             = static_cast <gimple_statement_eh_filter *> (x);
> +           gt_pch_n_9tree_node (stmt->types);
> +           gt_pch_nx_gimple_statement_base (stmt->failure);
> +         }
> +         break;
> +       case GSS_EH_MNT:
> +         {
> +           gimple_statement_eh_mnt *stmt
> +             = static_cast <gimple_statement_eh_mnt *> (x);
> +           gt_pch_n_9tree_node (stmt->fndecl);
> +         }
> +         break;
> +       case GSS_EH_ELSE:
> +         {
> +           gimple_statement_eh_else*stmt
> +             = static_cast <gimple_statement_eh_else *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->n_body);
> +           gt_pch_nx_gimple_statement_base (stmt->e_body);
> +         }
> +         break;
> +       case GSS_PHI:
> +         {
> +           gimple_statement_phi *stmt
> +             = static_cast <gimple_statement_phi *> (x);
> +           size_t num = (size_t)(stmt->nargs);
> +           gt_pch_n_9tree_node (stmt->result);
> +           for (size_t i = 0; i != num; i++)
> +             gt_pch_n_9tree_node (stmt->args[i].def);
> +         }
> +         break;
> +       case GSS_EH_CTRL:
> +         break;
> +       case GSS_TRY:
> +         {
> +           gimple_statement_try *stmt
> +             = static_cast <gimple_statement_try *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->eval);
> +           gt_pch_nx_gimple_statement_base (stmt->cleanup);
> +         }
> +         break;
> +       case GSS_WCE:
> +         {
> +           gimple_statement_wce *stmt
> +             = static_cast <gimple_statement_wce *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->cleanup);
> +         }
> +         break;
> +       case GSS_ASM:
> +         {
> +           gimple_statement_asm *stmt
> +             = static_cast <gimple_statement_asm *> (x);
> +           size_t num = (size_t)(stmt->num_ops);
> +           gt_pch_n_S (stmt->string);
> +           for (size_t i = 0; i != num; i++)
> +             gt_pch_n_9tree_node (stmt->op[i]);
> +         }
> +         break;
> +       case GSS_OMP_CRITICAL:
> +         {
> +           gimple_statement_omp_critical *stmt
> +             = static_cast <gimple_statement_omp_critical *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +           gt_pch_n_9tree_node (stmt->name);
> +         }
> +         break;
> +       case GSS_OMP_FOR:
> +         {
> +           gimple_statement_omp_for *stmt
> +             = static_cast <gimple_statement_omp_for *> (x);
> +           size_t num = (size_t)(stmt->collapse);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +           gt_pch_n_9tree_node (stmt->clauses);
> +           if (stmt->iter != NULL) {
> +             for (size_t i = 0; i != num; i++) {
> +               gt_pch_n_9tree_node (stmt->iter[i].index);
> +               gt_pch_n_9tree_node (stmt->iter[i].initial);
> +               gt_pch_n_9tree_node (stmt->iter[i].final);
> +               gt_pch_n_9tree_node (stmt->iter[i].incr);
> +             }
> +             gt_pch_note_object (stmt->iter, x,
> +                                 gt_pch_p_21gimple_statement_base);
> +           }
> +           gt_pch_nx_gimple_statement_base (stmt->pre_body);
> +         }
> +         break;
> +       case GSS_OMP_PARALLEL:
> +         {
> +           gimple_statement_omp_parallel *stmt
> +             = static_cast <gimple_statement_omp_parallel *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +           gt_pch_n_9tree_node (stmt->clauses);
> +           gt_pch_n_9tree_node (stmt->child_fn);
> +           gt_pch_n_9tree_node (stmt->data_arg);
> +         }
> +         break;
> +       case GSS_OMP_TASK:
> +         {
> +           gimple_statement_omp_task *stmt
> +             = static_cast <gimple_statement_omp_task *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +           gt_pch_n_9tree_node (stmt->clauses);
> +           gt_pch_n_9tree_node (stmt->child_fn);
> +           gt_pch_n_9tree_node (stmt->data_arg);
> +           gt_pch_n_9tree_node (stmt->copy_fn);
> +           gt_pch_n_9tree_node (stmt->arg_size);
> +           gt_pch_n_9tree_node (stmt->arg_align);
> +         }
> +         break;
> +       case GSS_OMP_SECTIONS:
> +         {
> +           gimple_statement_omp_sections *stmt
> +             = static_cast <gimple_statement_omp_sections *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +           gt_pch_n_9tree_node (stmt->clauses);
> +           gt_pch_n_9tree_node (stmt->control);
> +         }
> +         break;
> +       case GSS_OMP_SINGLE:
> +         {
> +           gimple_statement_omp_single *stmt
> +             = static_cast <gimple_statement_omp_single *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +           gt_pch_n_9tree_node (stmt->clauses);
> +         }
> +         break;
> +       case GSS_OMP_CONTINUE:
> +         {
> +           gimple_statement_omp_continue *stmt
> +             = static_cast <gimple_statement_omp_continue *> (x);
> +           gt_pch_n_9tree_node (stmt->control_def);
> +           gt_pch_n_9tree_node (stmt->control_use);
> +         }
> +         break;
> +       case GSS_OMP_ATOMIC_LOAD:
> +         {
> +           gimple_statement_omp_atomic_load *stmt
> +             = static_cast <gimple_statement_omp_atomic_load *> (x);
> +           gt_pch_n_9tree_node (stmt->rhs);
> +           gt_pch_n_9tree_node (stmt->lhs);
> +         }
> +         break;
> +       case GSS_OMP_ATOMIC_STORE:
> +         {
> +           gimple_statement_omp_atomic_store *stmt
> +             = static_cast <gimple_statement_omp_atomic_store *> (x);
> +           gt_pch_n_9tree_node (stmt->val);
> +         }
> +         break;
> +       case GSS_TRANSACTION:
> +         {
> +           gimple_statement_transaction *stmt
> +             = static_cast <gimple_statement_transaction *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +           gt_pch_n_9tree_node (stmt->label);
> +         }
> +         break;
> +       default:
> +         break;
> +       }
> +      x = x->next;
> +    }
> +}
> +
> +void
> +gt_pch_nx (gimple gs, gt_pointer_operator op, void *cookie)
> +{
> +  op (&(gs->bb), cookie);
> +  op (&(gs->next), cookie);
> +  switch (gimple_statement_structure (gs))
> +    {
> +    case GSS_BASE:
> +      break;
> +    case GSS_WITH_OPS:
> +      {
> +       gimple_statement_with_ops *stmt
> +         = static_cast <gimple_statement_with_ops *> (gs);
> +       size_t num = (size_t)(stmt->num_ops);
> +       for (size_t i = 0; i != num; i++)
> +         op (&(stmt->op[i]), cookie);
> +      }
> +      break;
> +    case GSS_WITH_MEM_OPS_BASE:
> +      break;
> +    case GSS_WITH_MEM_OPS:
> +      {
> +       gimple_statement_with_memory_ops *stmt
> +         = static_cast <gimple_statement_with_memory_ops *> (gs);
> +       size_t num = (size_t)(stmt->num_ops);
> +       for (size_t i = 0; i != num; i++)
> +         op (&(stmt->op[i]), cookie);
> +      }
> +      break;
> +    case GSS_CALL:
> +      {
> +       gimple_statement_call *stmt
> +         = static_cast <gimple_statement_call *> (gs);
> +       size_t num = (size_t)(stmt->num_ops);
> +       op (&(stmt->call_used.vars), cookie);
> +       op (&(stmt->call_clobbered.vars), cookie);
> +       switch (stmt->subcode & GF_CALL_INTERNAL)
> +         {
> +         case 0:
> +           op (&(stmt->u.fntype), cookie);
> +           break;
> +         case GF_CALL_INTERNAL:
> +           break;
> +         default:
> +           break;
> +         }
> +       for (size_t i = 0; i != num; i++)
> +         op (&(stmt->op[i]), cookie);
> +      }
> +      break;
> +    case GSS_OMP:
> +      {
> +       gimple_statement_omp *stmt
> +         = static_cast <gimple_statement_omp *> (gs);
> +       op (&(stmt->body), cookie);
> +      }
> +      break;
> +    case GSS_BIND:
> +      {
> +       gimple_statement_bind *stmt
> +         = static_cast <gimple_statement_bind *> (gs);
> +       op (&(stmt->vars), cookie);
> +       op (&(stmt->block), cookie);
> +       op (&(stmt->body), cookie);
> +      }
> +      break;
> +    case GSS_CATCH:
> +      {
> +       gimple_statement_catch *stmt
> +         = static_cast <gimple_statement_catch *> (gs);
> +       op (&(stmt->types), cookie);
> +       op (&(stmt->handler), cookie);
> +      }
> +      break;
> +    case GSS_EH_FILTER:
> +      {
> +       gimple_statement_eh_filter *stmt
> +         = static_cast <gimple_statement_eh_filter *> (gs);
> +       op (&(stmt->types), cookie);
> +       op (&(stmt->failure), cookie);
> +      }
> +      break;
> +    case GSS_EH_MNT:
> +      {
> +       gimple_statement_eh_mnt *stmt
> +         = static_cast <gimple_statement_eh_mnt *> (gs);
> +       op (&(stmt->fndecl), cookie);
> +      }
> +      break;
> +    case GSS_EH_ELSE:
> +      {
> +       gimple_statement_eh_else*stmt
> +         = static_cast <gimple_statement_eh_else *> (gs);
> +       op (&(stmt->n_body), cookie);
> +       op (&(stmt->e_body), cookie);
> +      }
> +      break;
> +    case GSS_PHI:
> +      {
> +       gimple_statement_phi *stmt
> +         = static_cast <gimple_statement_phi *> (gs);
> +       size_t num = (size_t)(stmt->nargs);
> +       op (&(stmt->result), cookie);
> +       for (size_t i = 0; i != num; i++)
> +         op (&(stmt->args[i].def), cookie);
> +      }
> +      break;
> +    case GSS_EH_CTRL:
> +      break;
> +    case GSS_TRY:
> +      {
> +       gimple_statement_try *stmt
> +         = static_cast <gimple_statement_try *> (gs);
> +       op (&(stmt->eval), cookie);
> +       op (&(stmt->cleanup), cookie);
> +      }
> +      break;
> +    case GSS_WCE:
> +      {
> +       gimple_statement_wce *stmt
> +         = static_cast <gimple_statement_wce *> (gs);
> +       op (&(stmt->cleanup), cookie);
> +      }
> +      break;
> +    case GSS_ASM:
> +      {
> +       gimple_statement_asm *stmt
> +         = static_cast <gimple_statement_asm *> (gs);
> +       size_t num = (size_t)(stmt->num_ops);
> +       op (&(stmt->string), cookie);
> +       for (size_t i = 0; i != num; i++)
> +         op (&(stmt->op[i]), cookie);
> +      }
> +      break;
> +    case GSS_OMP_CRITICAL:
> +      {
> +       gimple_statement_omp_critical *stmt
> +         = static_cast <gimple_statement_omp_critical *> (gs);
> +       op (&(stmt->body), cookie);
> +       op (&(stmt->name), cookie);
> +      }
> +      break;
> +    case GSS_OMP_FOR:
> +      {
> +       gimple_statement_omp_for *stmt
> +         = static_cast <gimple_statement_omp_for *> (gs);
> +       size_t num = (size_t)(stmt->collapse);
> +       op (&(stmt->body), cookie);
> +       op (&(stmt->clauses), cookie);
> +       if (stmt->iter != NULL) {
> +         for (size_t i = 0; i != num; i++) {
> +           op (&(stmt->iter[i].index), cookie);
> +           op (&(stmt->iter[i].initial), cookie);
> +           op (&(stmt->iter[i].final), cookie);
> +           op (&(stmt->iter[i].incr), cookie);
> +         }
> +         op (&(stmt->iter), cookie);
> +       }
> +       op (&(stmt->pre_body), cookie);
> +      }
> +      break;
> +    case GSS_OMP_PARALLEL:
> +      {
> +       gimple_statement_omp_parallel *stmt
> +         = static_cast <gimple_statement_omp_parallel *> (gs);
> +       op (&(stmt->body), cookie);
> +       op (&(stmt->clauses), cookie);
> +       op (&(stmt->child_fn), cookie);
> +       op (&(stmt->data_arg), cookie);
> +      }
> +      break;
> +    case GSS_OMP_TASK:
> +      {
> +       gimple_statement_omp_task *stmt
> +         = static_cast <gimple_statement_omp_task *> (gs);
> +       op (&(stmt->body), cookie);
> +       op (&(stmt->clauses), cookie);
> +       op (&(stmt->child_fn), cookie);
> +       op (&(stmt->data_arg), cookie);
> +       op (&(stmt->copy_fn), cookie);
> +       op (&(stmt->arg_size), cookie);
> +       op (&(stmt->arg_align), cookie);
> +      }
> +      break;
> +    case GSS_OMP_SECTIONS:
> +      {
> +       gimple_statement_omp_sections *stmt
> +         = static_cast <gimple_statement_omp_sections *> (gs);
> +       op (&(stmt->body), cookie);
> +       op (&(stmt->clauses), cookie);
> +       op (&(stmt->control), cookie);
> +      }
> +      break;
> +    case GSS_OMP_SINGLE:
> +      {
> +       gimple_statement_omp_single *stmt
> +         = static_cast <gimple_statement_omp_single *> (gs);
> +       op (&(stmt->body), cookie);
> +       op (&(stmt->clauses), cookie);
> +      }
> +      break;
> +    case GSS_OMP_CONTINUE:
> +      {
> +       gimple_statement_omp_continue *stmt
> +         = static_cast <gimple_statement_omp_continue *> (gs);
> +       op (&(stmt->control_def), cookie);
> +       op (&(stmt->control_use), cookie);
> +      }
> +      break;
> +    case GSS_OMP_ATOMIC_LOAD:
> +      {
> +       gimple_statement_omp_atomic_load *stmt
> +         = static_cast <gimple_statement_omp_atomic_load *> (gs);
> +       op (&(stmt->rhs), cookie);
> +       op (&(stmt->lhs), cookie);
> +      }
> +      break;
> +    case GSS_OMP_ATOMIC_STORE:
> +      {
> +       gimple_statement_omp_atomic_store *stmt
> +         = static_cast <gimple_statement_omp_atomic_store *> (gs);
> +       op (&(stmt->val), cookie);
> +      }
> +      break;
> +    case GSS_TRANSACTION:
> +      {
> +       gimple_statement_transaction *stmt
> +         = static_cast <gimple_statement_transaction *> (gs);
> +       op (&(stmt->body), cookie);
> +       op (&(stmt->label), cookie);
> +      }
> +      break;
> +    default:
> +      break;
> +    }
> +}
> +
> +
>  #include "gt-gimple.h"
> diff --git a/gcc/gimple.h b/gcc/gimple.h
> index daab54e..9b428eb 100644
> --- a/gcc/gimple.h
> +++ b/gcc/gimple.h
> @@ -222,6 +222,12 @@ struct GTY((user)) gimple_statement_base {
>    gimple GTY((skip)) prev;
>  };
>
> +/* GTY((user)) hooks for gimple, called once per-traversal.  */
> +void gt_ggc_mx (gimple gs);
> +void gt_pch_nx (gimple gs);
> +void gt_pch_nx (gimple gs, gt_pointer_operator op, void *cookie);
> +
> +
>
>  /* Base structure for tuples with operands.  */
>
> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> index af8685c..185c072 100644
> --- a/gcc/tree-cfg.c
> +++ b/gcc/tree-cfg.c
> @@ -8291,7 +8291,6 @@ make_pass_warn_unused_result (gcc::context *ctxt)
>  /* Garbage collection support for edge_def.  */
>
>  extern void gt_ggc_mx (tree&);
> -extern void gt_ggc_mx (gimple&);
>  extern void gt_ggc_mx (rtx&);
>  extern void gt_ggc_mx (basic_block&);
>
> @@ -8302,7 +8301,7 @@ gt_ggc_mx (edge_def *e)
>    gt_ggc_mx (e->src);
>    gt_ggc_mx (e->dest);
>    if (current_ir_type () == IR_GIMPLE)
> -    gt_ggc_mx (e->insns.g);
> +    gt_ggc_mx_gimple_statement_base (e->insns.g);
>    else
>      gt_ggc_mx (e->insns.r);
>    gt_ggc_mx (block);
> @@ -8311,7 +8310,6 @@ gt_ggc_mx (edge_def *e)
>  /* PCH support for edge_def.  */
>
>  extern void gt_pch_nx (tree&);
> -extern void gt_pch_nx (gimple&);
>  extern void gt_pch_nx (rtx&);
>  extern void gt_pch_nx (basic_block&);
>
> @@ -8322,7 +8320,7 @@ gt_pch_nx (edge_def *e)
>    gt_pch_nx (e->src);
>    gt_pch_nx (e->dest);
>    if (current_ir_type () == IR_GIMPLE)
> -    gt_pch_nx (e->insns.g);
> +    gt_pch_nx_gimple_statement_base (e->insns.g);
>    else
>      gt_pch_nx (e->insns.r);
>    gt_pch_nx (block);
> --
> 1.7.11.7
>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 6/6] Add manual GTY hooks
  2013-08-29 19:50   ` Steven Bosscher
@ 2013-08-30  8:12     ` Richard Biener
  2013-08-31 14:30       ` David Malcolm
  0 siblings, 1 reply; 44+ messages in thread
From: Richard Biener @ 2013-08-30  8:12 UTC (permalink / raw)
  To: Steven Bosscher, Diego Novillo; +Cc: David Malcolm, GCC Patches

On Thu, Aug 29, 2013 at 9:44 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Thu, Aug 29, 2013 at 6:20 PM, David Malcolm <dmalcolm@redhat.com> wrote:
>>         * gimple.c (gt_ggc_mx (gimple)): New, as required by GTY((user)).
>>         (gt_pch_nx (gimple)): Likewise.
>
> GIMPLE isn't supposed to end up in a PCH. Can you please make this
> function simply call gcc_unreachable()?
>
> FWIW 1: I really think all these hand-written markers aren't a good
> idea, we should really figure out a way to have automatic marker
> function generators, something less complex than gengtype, of course.
> But to have all these calls to the type-mangled marker functions
> (gt_pch_n_9tree_node, etc.) is going to be a problem in the long term.
>
> It seems to me that the first step in all these conversions to
> hand-written markers should be to make gengtype spit out the marker
> functions *without* the type name mangling, i.e. all marker functions
> should just be gt_ggc_mx(type) / gt_pch_nx(type).

Yes, the original idea was that gengtype would do that.  For things we like
to optimize the GTY((user)) thing would tell it that we do provide the markers.
Like when you want to look through a non-GCed intermediate object.  Or
for things like GTY((chain)) stuff that doesn't really work if you have multiple
chains (without clever GTY((skip))s...).

The lack of the unmangled overloads is annoying :/  IIRC Diego halfway completed
the transition to unmangled overloads / specializations?

Richard.

> Ciao!
> Steven

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 6/6] Add manual GTY hooks
  2013-08-29 17:04 ` [PATCH 6/6] Add manual GTY hooks David Malcolm
  2013-08-29 19:50   ` Steven Bosscher
  2013-08-30  8:09   ` Richard Biener
@ 2013-08-30  8:43   ` Richard Biener
  2013-08-30 20:25     ` David Malcolm
  2 siblings, 1 reply; 44+ messages in thread
From: Richard Biener @ 2013-08-30  8:43 UTC (permalink / raw)
  To: David Malcolm; +Cc: GCC Patches

On Thu, Aug 29, 2013 at 6:20 PM, David Malcolm <dmalcolm@redhat.com> wrote:
>         * gimple.c (gt_ggc_mx (gimple)): New, as required by GTY((user)).
>         (gt_pch_nx (gimple)): Likewise.
>         (gt_pch_nx (gimple, gt_pointer_operator, void *)): Likewise.
>         * gimple.h  (gt_ggc_mx (gimple)): Declare.
>         (gt_pch_nx (gimple)): Declare.
>         (gt_pch_nx (gimple, gt_pointer_operator, void *)): Declare.
>         * tree-cfg.c (ggc_mx (gimple&)): Remove declaration, as this
>         collides with the function that GTY((user)) expects.
>         (gt_ggc_mx (edge_def *)): Replace call to gt_ggc_mx on the
>         gimple with gt_ggc_mx_gimple_statement_base: in the
>         pre-GTY((user)) world, "gt_ggc_mx" was the function to be called
>         on a possibly NULL pointed to check if needed marking and if so
>         to traverse its fields.  In the GTY((user)) world, "gt_ggc_mx"
>         is the function to be called on non-NULL objects immediately *after*
>         they have been marked: it does not mark the object itself.
>         (gt_pch_nx (gimple&)): Remove declaration.
>         (gt_pch_nx (edge_def *)): Update as per the mx hook.

Btw, this shows that gimple isn't a true C++ hierarchy - because of GTY
you can only ever use 'gimple' pointers, not more specialized ones
like gimple_phi as you are missing the GTY machinery for them.

I'm not 100% convinced that we should do all this at this point without
getting a better hand on the gengtype issues (it's partial transition to
the C++ world of GCC)

> ---
>  gcc/gimple.c   | 743 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  gcc/gimple.h   |   6 +
>  gcc/tree-cfg.c |   6 +-
>  3 files changed, 751 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/gimple.c b/gcc/gimple.c
> index 1ad36d1..dd99cda 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -4338,4 +4338,747 @@ build_type_cast (tree to_type, gimple op, enum ssa_mode mode)
>    return build_type_cast (to_type, gimple_assign_lhs (op), mode);
>  }
>
> +void
> +gt_ggc_mx (gimple gs)
> +{
> +  gimple x = gs;
> +  /* Emulation of the "chain_next" GTY attribute.
> +
> +     gs has already been marked.
> +     Iterate the chain of next statements, marking until we reach one that
> +     has already been marked, or NULL.   */
> +  gimple xlimit = gs->next;
> +  while (ggc_test_and_set_mark (xlimit))
> +    xlimit = xlimit->next;
> +
> +  /* All of the statements within the half-open interval [x..xlimit) have
> +     just been marked.  Iterate through the list, visiting their fields.  */
> +  while (x != xlimit)
> +    {
> +      gt_ggc_m_15basic_block_def (x->bb);
> +      switch (gimple_statement_structure (&((*x))))
> +       {
> +       case GSS_BASE:
> +         break;
> +       case GSS_WITH_OPS:
> +         {
> +           gimple_statement_with_ops *stmt
> +             = static_cast <gimple_statement_with_ops *> (x);
> +           size_t num = (size_t)(stmt->num_ops);
> +           for (size_t i = 0; i != num; i++)
> +             gt_ggc_m_9tree_node (stmt->op[i]);
> +         }
> +         break;
> +       case GSS_WITH_MEM_OPS_BASE:
> +         break;
> +       case GSS_WITH_MEM_OPS:
> +         {
> +           gimple_statement_with_memory_ops *stmt
> +             = static_cast <gimple_statement_with_memory_ops *> (x);
> +           size_t num = (size_t)(stmt->num_ops);
> +           for (size_t i = 0; i != num; i++)
> +             gt_ggc_m_9tree_node (stmt->op[i]);
> +         }
> +         break;
> +       case GSS_CALL:
> +         {
> +           gimple_statement_call *stmt
> +             = static_cast <gimple_statement_call *> (x);
> +           gt_ggc_m_15bitmap_head_def (stmt->call_used.vars);
> +           gt_ggc_m_15bitmap_head_def (stmt->call_clobbered.vars);
> +           switch (stmt->subcode & GF_CALL_INTERNAL)
> +             {
> +             case 0:
> +               gt_ggc_m_9tree_node (stmt->u.fntype);
> +               break;
> +             case GF_CALL_INTERNAL:
> +               break;
> +             default:
> +               break;
> +             }
> +           size_t num = (size_t)(stmt->num_ops);
> +           for (size_t i = 0; i != num; i++)
> +             gt_ggc_m_9tree_node (stmt->op[i]);
> +         }
> +         break;
> +       case GSS_OMP:
> +         {
> +           gimple_statement_omp *stmt
> +             = static_cast <gimple_statement_omp *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +         }
> +         break;
> +       case GSS_BIND:
> +         {
> +            gimple_statement_bind *stmt
> +             = static_cast <gimple_statement_bind *> (x);
> +           gt_ggc_m_9tree_node (stmt->vars);
> +           gt_ggc_m_9tree_node (stmt->block);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +         }
> +         break;
> +       case GSS_CATCH:
> +         {
> +           gimple_statement_catch *stmt
> +             = static_cast <gimple_statement_catch *> (x);
> +           gt_ggc_m_9tree_node (stmt->types);
> +           gt_ggc_mx_gimple_statement_base (stmt->handler);
> +         }
> +         break;
> +       case GSS_EH_FILTER:
> +         {
> +           gimple_statement_eh_filter *stmt
> +             = static_cast <gimple_statement_eh_filter *> (x);
> +           gt_ggc_m_9tree_node (stmt->types);
> +           gt_ggc_mx_gimple_statement_base (stmt->failure);
> +         }
> +         break;
> +       case GSS_EH_MNT:
> +         {
> +           gimple_statement_eh_mnt *stmt
> +             = static_cast <gimple_statement_eh_mnt *> (x);
> +           gt_ggc_m_9tree_node (stmt->fndecl);
> +         }
> +         break;
> +       case GSS_EH_ELSE:
> +         {
> +           gimple_statement_eh_else*stmt
> +             = static_cast <gimple_statement_eh_else *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->n_body);
> +           gt_ggc_mx_gimple_statement_base (stmt->e_body);
> +         }
> +         break;
> +       case GSS_PHI:
> +         {
> +           gimple_statement_phi *stmt
> +             = static_cast <gimple_statement_phi *> (x);
> +           size_t num = (size_t)(stmt->nargs);
> +           gt_ggc_m_9tree_node (stmt->result);
> +           for (size_t i = 0; i != num; i++)
> +             gt_ggc_m_9tree_node (stmt->args[i].def);
> +         }
> +         break;
> +       case GSS_EH_CTRL:
> +         break;
> +       case GSS_TRY:
> +         {
> +           gimple_statement_try *stmt
> +             = static_cast <gimple_statement_try *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->eval);
> +           gt_ggc_mx_gimple_statement_base (stmt->cleanup);
> +         }
> +         break;
> +       case GSS_WCE:
> +         {
> +           gimple_statement_wce *stmt
> +             = static_cast <gimple_statement_wce *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->cleanup);
> +         }
> +         break;
> +       case GSS_ASM:
> +         {
> +           gimple_statement_asm *stmt
> +             = static_cast <gimple_statement_asm *> (x);
> +           size_t num = (size_t)(stmt->num_ops);
> +           gt_ggc_m_S (stmt->string);
> +           for (size_t i = 0; i != num; i++)
> +             gt_ggc_m_9tree_node (stmt->op[i]);
> +         }
> +         break;
> +       case GSS_OMP_CRITICAL:
> +         {
> +           gimple_statement_omp_critical *stmt
> +             = static_cast <gimple_statement_omp_critical *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +           gt_ggc_m_9tree_node (stmt->name);
> +         }
> +         break;
> +       case GSS_OMP_FOR:
> +         {
> +           gimple_statement_omp_for *stmt
> +             = static_cast <gimple_statement_omp_for *> (x);
> +           size_t num = (size_t)(stmt->collapse);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +           gt_ggc_m_9tree_node (stmt->clauses);
> +           if (stmt->iter != NULL) {
> +             for (size_t i = 0; i != num; i++) {
> +               gt_ggc_m_9tree_node (stmt->iter[i].index);
> +               gt_ggc_m_9tree_node (stmt->iter[i].initial);
> +               gt_ggc_m_9tree_node (stmt->iter[i].final);
> +               gt_ggc_m_9tree_node (stmt->iter[i].incr);
> +             }
> +             ggc_mark (stmt->iter);
> +           }
> +           gt_ggc_mx_gimple_statement_base (stmt->pre_body);
> +         }
> +         break;
> +       case GSS_OMP_PARALLEL:
> +         {
> +           gimple_statement_omp_parallel *stmt
> +             = static_cast <gimple_statement_omp_parallel *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +           gt_ggc_m_9tree_node (stmt->clauses);
> +           gt_ggc_m_9tree_node (stmt->child_fn);
> +           gt_ggc_m_9tree_node (stmt->data_arg);
> +         }
> +         break;
> +       case GSS_OMP_TASK:
> +         {
> +           gimple_statement_omp_task *stmt
> +             = static_cast <gimple_statement_omp_task *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +           gt_ggc_m_9tree_node (stmt->clauses);
> +           gt_ggc_m_9tree_node (stmt->child_fn);
> +           gt_ggc_m_9tree_node (stmt->data_arg);
> +           gt_ggc_m_9tree_node (stmt->copy_fn);
> +           gt_ggc_m_9tree_node (stmt->arg_size);
> +           gt_ggc_m_9tree_node (stmt->arg_align);
> +         }
> +         break;
> +       case GSS_OMP_SECTIONS:
> +         {
> +           gimple_statement_omp_sections *stmt
> +             = static_cast <gimple_statement_omp_sections *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +           gt_ggc_m_9tree_node (stmt->clauses);
> +           gt_ggc_m_9tree_node (stmt->control);
> +         }
> +         break;
> +       case GSS_OMP_SINGLE:
> +         {
> +           gimple_statement_omp_single *stmt
> +             = static_cast <gimple_statement_omp_single *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +           gt_ggc_m_9tree_node (stmt->clauses);
> +         }
> +         break;
> +       case GSS_OMP_CONTINUE:
> +         {
> +           gimple_statement_omp_continue *stmt
> +             = static_cast <gimple_statement_omp_continue *> (x);
> +           gt_ggc_m_9tree_node (stmt->control_def);
> +           gt_ggc_m_9tree_node (stmt->control_use);
> +         }
> +         break;
> +       case GSS_OMP_ATOMIC_LOAD:
> +         {
> +           gimple_statement_omp_atomic_load *stmt
> +             = static_cast <gimple_statement_omp_atomic_load *> (x);
> +           gt_ggc_m_9tree_node (stmt->rhs);
> +           gt_ggc_m_9tree_node (stmt->lhs);
> +         }
> +         break;
> +       case GSS_OMP_ATOMIC_STORE:
> +         {
> +           gimple_statement_omp_atomic_store *stmt
> +             = static_cast <gimple_statement_omp_atomic_store *> (x);
> +           gt_ggc_m_9tree_node (stmt->val);
> +         }
> +         break;
> +       case GSS_TRANSACTION:
> +         {
> +           gimple_statement_transaction *stmt
> +             = static_cast <gimple_statement_transaction *> (x);
> +           gt_ggc_mx_gimple_statement_base (stmt->body);
> +           gt_ggc_m_9tree_node (stmt->label);
> +         }
> +         break;
> +       default:
> +         break;
> +       }
> +      x = x->next;
> +    }
> +}
> +
> +void
> +gt_pch_nx (gimple gs)
> +{
> +  gimple x = gs;
> +  /* Emulation of the "chain_next" GTY attribute.
> +
> +     gs has already been marked.
> +     Iterate the chain of next statements, marking until we reach one that
> +     has already been marked, or NULL.   */
> +  gimple xlimit = gs->next;
> +  while (gt_pch_note_object (xlimit, xlimit, gt_pch_p_21gimple_statement_base))
> +    xlimit = xlimit->next;
> +
> +  /* All of the statements within the half-open interval [x..xlimit) have
> +     just been marked.  Iterate through the list, visiting their fields.  */
> +  while (x != xlimit)
> +    {
> +      gt_pch_n_15basic_block_def (x->bb);
> +      switch (gimple_statement_structure (&((*x))))
> +       {
> +       case GSS_BASE:
> +         break;
> +       case GSS_WITH_OPS:
> +         {
> +           gimple_statement_with_ops *stmt
> +             = static_cast <gimple_statement_with_ops *> (x);
> +           size_t num = (size_t)(stmt->num_ops);
> +           for (size_t i = 0; i != num; i++)
> +             gt_pch_n_9tree_node (stmt->op[i]);
> +         }
> +         break;
> +       case GSS_WITH_MEM_OPS_BASE:
> +         break;
> +       case GSS_WITH_MEM_OPS:
> +         {
> +           gimple_statement_with_memory_ops *stmt
> +             = static_cast <gimple_statement_with_memory_ops *> (x);
> +           size_t num = (size_t)(stmt->num_ops);
> +           for (size_t i = 0; i != num; i++)
> +             gt_pch_n_9tree_node (stmt->op[i]);
> +         }
> +         break;
> +       case GSS_CALL:
> +         {
> +           gimple_statement_call *stmt
> +             = static_cast <gimple_statement_call *> (x);
> +           gt_pch_n_15bitmap_head_def (stmt->call_used.vars);
> +           gt_pch_n_15bitmap_head_def (stmt->call_clobbered.vars);
> +           switch (stmt->subcode & GF_CALL_INTERNAL)
> +             {
> +             case 0:
> +               gt_pch_n_9tree_node (stmt->u.fntype);
> +               break;
> +             case GF_CALL_INTERNAL:
> +               break;
> +             default:
> +               break;
> +             }
> +           size_t num = (size_t)(stmt->num_ops);
> +           for (size_t i = 0; i != num; i++)
> +             gt_pch_n_9tree_node (stmt->op[i]);
> +         }
> +         break;
> +       case GSS_OMP:
> +         {
> +           gimple_statement_omp *stmt
> +             = static_cast <gimple_statement_omp *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +         }
> +         break;
> +       case GSS_BIND:
> +         {
> +            gimple_statement_bind *stmt
> +             = static_cast <gimple_statement_bind *> (x);
> +           gt_pch_n_9tree_node (stmt->vars);
> +           gt_pch_n_9tree_node (stmt->block);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +         }
> +         break;
> +       case GSS_CATCH:
> +         {
> +           gimple_statement_catch *stmt
> +             = static_cast <gimple_statement_catch *> (x);
> +           gt_pch_n_9tree_node (stmt->types);
> +           gt_pch_nx_gimple_statement_base (stmt->handler);
> +         }
> +         break;
> +       case GSS_EH_FILTER:
> +         {
> +           gimple_statement_eh_filter *stmt
> +             = static_cast <gimple_statement_eh_filter *> (x);
> +           gt_pch_n_9tree_node (stmt->types);
> +           gt_pch_nx_gimple_statement_base (stmt->failure);
> +         }
> +         break;
> +       case GSS_EH_MNT:
> +         {
> +           gimple_statement_eh_mnt *stmt
> +             = static_cast <gimple_statement_eh_mnt *> (x);
> +           gt_pch_n_9tree_node (stmt->fndecl);
> +         }
> +         break;
> +       case GSS_EH_ELSE:
> +         {
> +           gimple_statement_eh_else*stmt
> +             = static_cast <gimple_statement_eh_else *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->n_body);
> +           gt_pch_nx_gimple_statement_base (stmt->e_body);
> +         }
> +         break;
> +       case GSS_PHI:
> +         {
> +           gimple_statement_phi *stmt
> +             = static_cast <gimple_statement_phi *> (x);
> +           size_t num = (size_t)(stmt->nargs);
> +           gt_pch_n_9tree_node (stmt->result);
> +           for (size_t i = 0; i != num; i++)
> +             gt_pch_n_9tree_node (stmt->args[i].def);
> +         }
> +         break;
> +       case GSS_EH_CTRL:
> +         break;
> +       case GSS_TRY:
> +         {
> +           gimple_statement_try *stmt
> +             = static_cast <gimple_statement_try *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->eval);
> +           gt_pch_nx_gimple_statement_base (stmt->cleanup);
> +         }
> +         break;
> +       case GSS_WCE:
> +         {
> +           gimple_statement_wce *stmt
> +             = static_cast <gimple_statement_wce *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->cleanup);
> +         }
> +         break;
> +       case GSS_ASM:
> +         {
> +           gimple_statement_asm *stmt
> +             = static_cast <gimple_statement_asm *> (x);
> +           size_t num = (size_t)(stmt->num_ops);
> +           gt_pch_n_S (stmt->string);
> +           for (size_t i = 0; i != num; i++)
> +             gt_pch_n_9tree_node (stmt->op[i]);
> +         }
> +         break;
> +       case GSS_OMP_CRITICAL:
> +         {
> +           gimple_statement_omp_critical *stmt
> +             = static_cast <gimple_statement_omp_critical *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +           gt_pch_n_9tree_node (stmt->name);
> +         }
> +         break;
> +       case GSS_OMP_FOR:
> +         {
> +           gimple_statement_omp_for *stmt
> +             = static_cast <gimple_statement_omp_for *> (x);
> +           size_t num = (size_t)(stmt->collapse);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +           gt_pch_n_9tree_node (stmt->clauses);
> +           if (stmt->iter != NULL) {
> +             for (size_t i = 0; i != num; i++) {
> +               gt_pch_n_9tree_node (stmt->iter[i].index);
> +               gt_pch_n_9tree_node (stmt->iter[i].initial);
> +               gt_pch_n_9tree_node (stmt->iter[i].final);
> +               gt_pch_n_9tree_node (stmt->iter[i].incr);
> +             }
> +             gt_pch_note_object (stmt->iter, x,
> +                                 gt_pch_p_21gimple_statement_base);
> +           }
> +           gt_pch_nx_gimple_statement_base (stmt->pre_body);
> +         }
> +         break;
> +       case GSS_OMP_PARALLEL:
> +         {
> +           gimple_statement_omp_parallel *stmt
> +             = static_cast <gimple_statement_omp_parallel *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +           gt_pch_n_9tree_node (stmt->clauses);
> +           gt_pch_n_9tree_node (stmt->child_fn);
> +           gt_pch_n_9tree_node (stmt->data_arg);
> +         }
> +         break;
> +       case GSS_OMP_TASK:
> +         {
> +           gimple_statement_omp_task *stmt
> +             = static_cast <gimple_statement_omp_task *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +           gt_pch_n_9tree_node (stmt->clauses);
> +           gt_pch_n_9tree_node (stmt->child_fn);
> +           gt_pch_n_9tree_node (stmt->data_arg);
> +           gt_pch_n_9tree_node (stmt->copy_fn);
> +           gt_pch_n_9tree_node (stmt->arg_size);
> +           gt_pch_n_9tree_node (stmt->arg_align);
> +         }
> +         break;
> +       case GSS_OMP_SECTIONS:
> +         {
> +           gimple_statement_omp_sections *stmt
> +             = static_cast <gimple_statement_omp_sections *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +           gt_pch_n_9tree_node (stmt->clauses);
> +           gt_pch_n_9tree_node (stmt->control);
> +         }
> +         break;
> +       case GSS_OMP_SINGLE:
> +         {
> +           gimple_statement_omp_single *stmt
> +             = static_cast <gimple_statement_omp_single *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +           gt_pch_n_9tree_node (stmt->clauses);
> +         }
> +         break;
> +       case GSS_OMP_CONTINUE:
> +         {
> +           gimple_statement_omp_continue *stmt
> +             = static_cast <gimple_statement_omp_continue *> (x);
> +           gt_pch_n_9tree_node (stmt->control_def);
> +           gt_pch_n_9tree_node (stmt->control_use);
> +         }
> +         break;
> +       case GSS_OMP_ATOMIC_LOAD:
> +         {
> +           gimple_statement_omp_atomic_load *stmt
> +             = static_cast <gimple_statement_omp_atomic_load *> (x);
> +           gt_pch_n_9tree_node (stmt->rhs);
> +           gt_pch_n_9tree_node (stmt->lhs);
> +         }
> +         break;
> +       case GSS_OMP_ATOMIC_STORE:
> +         {
> +           gimple_statement_omp_atomic_store *stmt
> +             = static_cast <gimple_statement_omp_atomic_store *> (x);
> +           gt_pch_n_9tree_node (stmt->val);
> +         }
> +         break;
> +       case GSS_TRANSACTION:
> +         {
> +           gimple_statement_transaction *stmt
> +             = static_cast <gimple_statement_transaction *> (x);
> +           gt_pch_nx_gimple_statement_base (stmt->body);
> +           gt_pch_n_9tree_node (stmt->label);
> +         }
> +         break;
> +       default:
> +         break;
> +       }
> +      x = x->next;
> +    }
> +}
> +
> +void
> +gt_pch_nx (gimple gs, gt_pointer_operator op, void *cookie)
> +{
> +  op (&(gs->bb), cookie);
> +  op (&(gs->next), cookie);
> +  switch (gimple_statement_structure (gs))
> +    {
> +    case GSS_BASE:
> +      break;
> +    case GSS_WITH_OPS:
> +      {
> +       gimple_statement_with_ops *stmt
> +         = static_cast <gimple_statement_with_ops *> (gs);
> +       size_t num = (size_t)(stmt->num_ops);
> +       for (size_t i = 0; i != num; i++)
> +         op (&(stmt->op[i]), cookie);
> +      }
> +      break;
> +    case GSS_WITH_MEM_OPS_BASE:
> +      break;
> +    case GSS_WITH_MEM_OPS:
> +      {
> +       gimple_statement_with_memory_ops *stmt
> +         = static_cast <gimple_statement_with_memory_ops *> (gs);
> +       size_t num = (size_t)(stmt->num_ops);
> +       for (size_t i = 0; i != num; i++)
> +         op (&(stmt->op[i]), cookie);
> +      }
> +      break;
> +    case GSS_CALL:
> +      {
> +       gimple_statement_call *stmt
> +         = static_cast <gimple_statement_call *> (gs);
> +       size_t num = (size_t)(stmt->num_ops);
> +       op (&(stmt->call_used.vars), cookie);
> +       op (&(stmt->call_clobbered.vars), cookie);
> +       switch (stmt->subcode & GF_CALL_INTERNAL)
> +         {
> +         case 0:
> +           op (&(stmt->u.fntype), cookie);
> +           break;
> +         case GF_CALL_INTERNAL:
> +           break;
> +         default:
> +           break;
> +         }
> +       for (size_t i = 0; i != num; i++)
> +         op (&(stmt->op[i]), cookie);
> +      }
> +      break;
> +    case GSS_OMP:
> +      {
> +       gimple_statement_omp *stmt
> +         = static_cast <gimple_statement_omp *> (gs);
> +       op (&(stmt->body), cookie);
> +      }
> +      break;
> +    case GSS_BIND:
> +      {
> +       gimple_statement_bind *stmt
> +         = static_cast <gimple_statement_bind *> (gs);
> +       op (&(stmt->vars), cookie);
> +       op (&(stmt->block), cookie);
> +       op (&(stmt->body), cookie);
> +      }
> +      break;
> +    case GSS_CATCH:
> +      {
> +       gimple_statement_catch *stmt
> +         = static_cast <gimple_statement_catch *> (gs);
> +       op (&(stmt->types), cookie);
> +       op (&(stmt->handler), cookie);
> +      }
> +      break;
> +    case GSS_EH_FILTER:
> +      {
> +       gimple_statement_eh_filter *stmt
> +         = static_cast <gimple_statement_eh_filter *> (gs);
> +       op (&(stmt->types), cookie);
> +       op (&(stmt->failure), cookie);
> +      }
> +      break;
> +    case GSS_EH_MNT:
> +      {
> +       gimple_statement_eh_mnt *stmt
> +         = static_cast <gimple_statement_eh_mnt *> (gs);
> +       op (&(stmt->fndecl), cookie);
> +      }
> +      break;
> +    case GSS_EH_ELSE:
> +      {
> +       gimple_statement_eh_else*stmt
> +         = static_cast <gimple_statement_eh_else *> (gs);
> +       op (&(stmt->n_body), cookie);
> +       op (&(stmt->e_body), cookie);
> +      }
> +      break;
> +    case GSS_PHI:
> +      {
> +       gimple_statement_phi *stmt
> +         = static_cast <gimple_statement_phi *> (gs);
> +       size_t num = (size_t)(stmt->nargs);
> +       op (&(stmt->result), cookie);
> +       for (size_t i = 0; i != num; i++)
> +         op (&(stmt->args[i].def), cookie);
> +      }
> +      break;
> +    case GSS_EH_CTRL:
> +      break;
> +    case GSS_TRY:
> +      {
> +       gimple_statement_try *stmt
> +         = static_cast <gimple_statement_try *> (gs);
> +       op (&(stmt->eval), cookie);
> +       op (&(stmt->cleanup), cookie);
> +      }
> +      break;
> +    case GSS_WCE:
> +      {
> +       gimple_statement_wce *stmt
> +         = static_cast <gimple_statement_wce *> (gs);
> +       op (&(stmt->cleanup), cookie);
> +      }
> +      break;
> +    case GSS_ASM:
> +      {
> +       gimple_statement_asm *stmt
> +         = static_cast <gimple_statement_asm *> (gs);
> +       size_t num = (size_t)(stmt->num_ops);
> +       op (&(stmt->string), cookie);
> +       for (size_t i = 0; i != num; i++)
> +         op (&(stmt->op[i]), cookie);
> +      }
> +      break;
> +    case GSS_OMP_CRITICAL:
> +      {
> +       gimple_statement_omp_critical *stmt
> +         = static_cast <gimple_statement_omp_critical *> (gs);
> +       op (&(stmt->body), cookie);
> +       op (&(stmt->name), cookie);
> +      }
> +      break;
> +    case GSS_OMP_FOR:
> +      {
> +       gimple_statement_omp_for *stmt
> +         = static_cast <gimple_statement_omp_for *> (gs);
> +       size_t num = (size_t)(stmt->collapse);
> +       op (&(stmt->body), cookie);
> +       op (&(stmt->clauses), cookie);
> +       if (stmt->iter != NULL) {
> +         for (size_t i = 0; i != num; i++) {
> +           op (&(stmt->iter[i].index), cookie);
> +           op (&(stmt->iter[i].initial), cookie);
> +           op (&(stmt->iter[i].final), cookie);
> +           op (&(stmt->iter[i].incr), cookie);
> +         }
> +         op (&(stmt->iter), cookie);
> +       }
> +       op (&(stmt->pre_body), cookie);
> +      }
> +      break;
> +    case GSS_OMP_PARALLEL:
> +      {
> +       gimple_statement_omp_parallel *stmt
> +         = static_cast <gimple_statement_omp_parallel *> (gs);
> +       op (&(stmt->body), cookie);
> +       op (&(stmt->clauses), cookie);
> +       op (&(stmt->child_fn), cookie);
> +       op (&(stmt->data_arg), cookie);
> +      }
> +      break;
> +    case GSS_OMP_TASK:
> +      {
> +       gimple_statement_omp_task *stmt
> +         = static_cast <gimple_statement_omp_task *> (gs);
> +       op (&(stmt->body), cookie);
> +       op (&(stmt->clauses), cookie);
> +       op (&(stmt->child_fn), cookie);
> +       op (&(stmt->data_arg), cookie);
> +       op (&(stmt->copy_fn), cookie);
> +       op (&(stmt->arg_size), cookie);
> +       op (&(stmt->arg_align), cookie);
> +      }
> +      break;
> +    case GSS_OMP_SECTIONS:
> +      {
> +       gimple_statement_omp_sections *stmt
> +         = static_cast <gimple_statement_omp_sections *> (gs);
> +       op (&(stmt->body), cookie);
> +       op (&(stmt->clauses), cookie);
> +       op (&(stmt->control), cookie);
> +      }
> +      break;
> +    case GSS_OMP_SINGLE:
> +      {
> +       gimple_statement_omp_single *stmt
> +         = static_cast <gimple_statement_omp_single *> (gs);
> +       op (&(stmt->body), cookie);
> +       op (&(stmt->clauses), cookie);
> +      }
> +      break;
> +    case GSS_OMP_CONTINUE:
> +      {
> +       gimple_statement_omp_continue *stmt
> +         = static_cast <gimple_statement_omp_continue *> (gs);
> +       op (&(stmt->control_def), cookie);
> +       op (&(stmt->control_use), cookie);
> +      }
> +      break;
> +    case GSS_OMP_ATOMIC_LOAD:
> +      {
> +       gimple_statement_omp_atomic_load *stmt
> +         = static_cast <gimple_statement_omp_atomic_load *> (gs);
> +       op (&(stmt->rhs), cookie);
> +       op (&(stmt->lhs), cookie);
> +      }
> +      break;
> +    case GSS_OMP_ATOMIC_STORE:
> +      {
> +       gimple_statement_omp_atomic_store *stmt
> +         = static_cast <gimple_statement_omp_atomic_store *> (gs);
> +       op (&(stmt->val), cookie);
> +      }
> +      break;
> +    case GSS_TRANSACTION:
> +      {
> +       gimple_statement_transaction *stmt
> +         = static_cast <gimple_statement_transaction *> (gs);
> +       op (&(stmt->body), cookie);
> +       op (&(stmt->label), cookie);
> +      }
> +      break;
> +    default:
> +      break;
> +    }
> +}
> +
> +
>  #include "gt-gimple.h"
> diff --git a/gcc/gimple.h b/gcc/gimple.h
> index daab54e..9b428eb 100644
> --- a/gcc/gimple.h
> +++ b/gcc/gimple.h
> @@ -222,6 +222,12 @@ struct GTY((user)) gimple_statement_base {
>    gimple GTY((skip)) prev;
>  };
>
> +/* GTY((user)) hooks for gimple, called once per-traversal.  */
> +void gt_ggc_mx (gimple gs);
> +void gt_pch_nx (gimple gs);
> +void gt_pch_nx (gimple gs, gt_pointer_operator op, void *cookie);
> +
> +
>
>  /* Base structure for tuples with operands.  */
>
> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> index af8685c..185c072 100644
> --- a/gcc/tree-cfg.c
> +++ b/gcc/tree-cfg.c
> @@ -8291,7 +8291,6 @@ make_pass_warn_unused_result (gcc::context *ctxt)
>  /* Garbage collection support for edge_def.  */
>
>  extern void gt_ggc_mx (tree&);
> -extern void gt_ggc_mx (gimple&);
>  extern void gt_ggc_mx (rtx&);
>  extern void gt_ggc_mx (basic_block&);
>
> @@ -8302,7 +8301,7 @@ gt_ggc_mx (edge_def *e)
>    gt_ggc_mx (e->src);
>    gt_ggc_mx (e->dest);
>    if (current_ir_type () == IR_GIMPLE)
> -    gt_ggc_mx (e->insns.g);
> +    gt_ggc_mx_gimple_statement_base (e->insns.g);
>    else
>      gt_ggc_mx (e->insns.r);
>    gt_ggc_mx (block);
> @@ -8311,7 +8310,6 @@ gt_ggc_mx (edge_def *e)
>  /* PCH support for edge_def.  */
>
>  extern void gt_pch_nx (tree&);
> -extern void gt_pch_nx (gimple&);
>  extern void gt_pch_nx (rtx&);
>  extern void gt_pch_nx (basic_block&);
>
> @@ -8322,7 +8320,7 @@ gt_pch_nx (edge_def *e)
>    gt_pch_nx (e->src);
>    gt_pch_nx (e->dest);
>    if (current_ir_type () == IR_GIMPLE)
> -    gt_pch_nx (e->insns.g);
> +    gt_pch_nx_gimple_statement_base (e->insns.g);
>    else
>      gt_pch_nx (e->insns.r);
>    gt_pch_nx (block);
> --
> 1.7.11.7
>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-29 16:20 [PATCH 0/6] Convert gimple to a C++ class hierarchy David Malcolm
                   ` (5 preceding siblings ...)
  2013-08-29 17:04 ` [PATCH 6/6] Add manual GTY hooks David Malcolm
@ 2013-08-30 13:58 ` Michael Matz
  2013-08-30 14:02   ` Gabriel Dos Reis
  2013-08-30 19:38   ` David Malcolm
  2013-08-30 21:51 ` David Malcolm
  7 siblings, 2 replies; 44+ messages in thread
From: Michael Matz @ 2013-08-30 13:58 UTC (permalink / raw)
  To: David Malcolm; +Cc: gcc-patches

Hi,

On Thu, 29 Aug 2013, David Malcolm wrote:

> Successfully bootstrapped and tested on x86_64-unknown-linux-gnu: all
> testcases show the same results as an unpatched build (relative to
> r202029).

I'd like to see some statistics for cc1{,plus} codesize and for compile 
time of something reasonably large (needing say 60 seconds to 
compile normally), before/after patch series.

And the manual GTY markers are so not maintainable in the long run, 
gengtype or something else really needs to be taught to create them 
automatically.


Ciao,
Michael.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-30 13:58 ` [PATCH 0/6] Convert gimple to a C++ class hierarchy Michael Matz
@ 2013-08-30 14:02   ` Gabriel Dos Reis
  2013-08-30 14:03     ` Jakub Jelinek
  2013-08-30 19:38   ` David Malcolm
  1 sibling, 1 reply; 44+ messages in thread
From: Gabriel Dos Reis @ 2013-08-30 14:02 UTC (permalink / raw)
  To: Michael Matz; +Cc: David Malcolm, gcc-patches

On Fri, Aug 30, 2013 at 8:44 AM, Michael Matz <matz@suse.de> wrote:

> And the manual GTY markers are so not maintainable in the long run,
> gengtype or something else really needs to be taught to create them
> automatically.

I thought the principle that was acquired was that gengtype shouldn't
be improved to support more than what it does now….

-- Gaby

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-30 14:02   ` Gabriel Dos Reis
@ 2013-08-30 14:03     ` Jakub Jelinek
  2013-08-30 14:26       ` Gabriel Dos Reis
  0 siblings, 1 reply; 44+ messages in thread
From: Jakub Jelinek @ 2013-08-30 14:03 UTC (permalink / raw)
  To: Gabriel Dos Reis; +Cc: Michael Matz, David Malcolm, gcc-patches

On Fri, Aug 30, 2013 at 08:58:43AM -0500, Gabriel Dos Reis wrote:
> On Fri, Aug 30, 2013 at 8:44 AM, Michael Matz <matz@suse.de> wrote:
> 
> > And the manual GTY markers are so not maintainable in the long run,
> > gengtype or something else really needs to be taught to create them
> > automatically.
> 
> I thought the principle that was acquired was that gengtype shouldn't
> be improved to support more than what it does now….

If it means that we'll need to write and maintain tons of hand written code
that could otherwise be generated and maintained by a tool for us, that
principle doesn't look very good.

	Jakub

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-30 14:03     ` Jakub Jelinek
@ 2013-08-30 14:26       ` Gabriel Dos Reis
  2013-08-30 14:49         ` David Malcolm
  2013-08-30 15:21         ` Michael Matz
  0 siblings, 2 replies; 44+ messages in thread
From: Gabriel Dos Reis @ 2013-08-30 14:26 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Michael Matz, David Malcolm, gcc-patches

On Fri, Aug 30, 2013 at 9:02 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Aug 30, 2013 at 08:58:43AM -0500, Gabriel Dos Reis wrote:
>> On Fri, Aug 30, 2013 at 8:44 AM, Michael Matz <matz@suse.de> wrote:
>>
>> > And the manual GTY markers are so not maintainable in the long run,
>> > gengtype or something else really needs to be taught to create them
>> > automatically.
>>
>> I thought the principle that was acquired was that gengtype shouldn't
>> be improved to support more than what it does now….
>
> If it means that we'll need to write and maintain tons of hand written code
> that could otherwise be generated and maintained by a tool for us, that
> principle doesn't look very good.
>
>         Jakub

Back in March 2013, I asked about gengtype support for inheritance.

   http://gcc.gnu.org/ml/gcc/2013-03/msg00273.html

This

   http://gcc.gnu.org/ml/gcc/2013-03/msg00295.html

was the definitive answer that appeared to be the consensus.

-- Gaby

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-30 14:26       ` Gabriel Dos Reis
@ 2013-08-30 14:49         ` David Malcolm
  2013-08-30 15:21         ` Michael Matz
  1 sibling, 0 replies; 44+ messages in thread
From: David Malcolm @ 2013-08-30 14:49 UTC (permalink / raw)
  To: Gabriel Dos Reis; +Cc: Jakub Jelinek, Michael Matz, gcc-patches, Diego Novillo

On Fri, 2013-08-30 at 09:12 -0500, Gabriel Dos Reis wrote:
> On Fri, Aug 30, 2013 at 9:02 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Fri, Aug 30, 2013 at 08:58:43AM -0500, Gabriel Dos Reis wrote:
> >> On Fri, Aug 30, 2013 at 8:44 AM, Michael Matz <matz@suse.de> wrote:
> >>
> >> > And the manual GTY markers are so not maintainable in the long run,
> >> > gengtype or something else really needs to be taught to create them
> >> > automatically.
> >>
> >> I thought the principle that was acquired was that gengtype shouldn't
> >> be improved to support more than what it does now….
> >
> > If it means that we'll need to write and maintain tons of hand written code
> > that could otherwise be generated and maintained by a tool for us, that
> > principle doesn't look very good.
> >
> >         Jakub
> 
> Back in March 2013, I asked about gengtype support for inheritance.
> 
>    http://gcc.gnu.org/ml/gcc/2013-03/msg00273.html
> 
> This
> 
>    http://gcc.gnu.org/ml/gcc/2013-03/msg00295.html
> 
> was the definitive answer that appeared to be the consensus.

[adding Diego to the CC]

I get the impression from the responses so far to this and to the symtab
patches [1] that people would prefer that gengtype be somehow expanded
to cope with single-inheritance.

Handwaving over syntax (borrowing from the union-marking syntax),
perhaps something like this:

struct
  GTY((chain_next ("%h.next")),
       desc ("gimple_statement_structure (&%h)"))
  gimple_statement_base {
   /* as before */
};

struct GTY((subclass_tag ("GSS_PHI"))) gimple_statement_phi : public
gimple_statement_base {
}

and then have gengtype recognize the inheritance hierarchy below
gimple_statement_base, and "do the right thing" (again I'm handwaving).

Also, perhaps we could use a new GTY flag:  "never_in_pch" or somesuch,
so that we can mark e.g. gimple and rtx as never appearing in pch, and
thus only the GC hooks need to exist; the PCH either don't need to be
generated, or could be stubs containing just gcc_unreachable?   That
would be an independent feature from the subclass support, of course.

That said, for this particular patch series, for the hand-maintained
route, there would just be one big GTY-related function to maintain,
rather than 3, given the lack of need for PCH support.

Dave

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-30 14:26       ` Gabriel Dos Reis
  2013-08-30 14:49         ` David Malcolm
@ 2013-08-30 15:21         ` Michael Matz
  2013-08-30 15:26           ` Gabriel Dos Reis
  2013-08-30 15:31           ` Diego Novillo
  1 sibling, 2 replies; 44+ messages in thread
From: Michael Matz @ 2013-08-30 15:21 UTC (permalink / raw)
  To: Gabriel Dos Reis; +Cc: Jakub Jelinek, David Malcolm, gcc-patches

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1023 bytes --]

Hi,

On Fri, 30 Aug 2013, Gabriel Dos Reis wrote:

> On Fri, Aug 30, 2013 at 9:02 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> >> I thought the principle that was acquired was that gengtype shouldn't
> >> be improved to support more than what it does nowÂ….
> >
> > If it means that we'll need to write and maintain tons of hand written code
> > that could otherwise be generated and maintained by a tool for us, that
> > principle doesn't look very good.

Exactly.

> Back in March 2013, I asked about gengtype support for inheritance.
>    http://gcc.gnu.org/ml/gcc/2013-03/msg00273.html
> This
>    http://gcc.gnu.org/ml/gcc/2013-03/msg00295.html
> was the definitive answer that appeared to be the consensus.

Well, it was a wrong decision then.  For some smaller types writing manual 
marker might be a sensible thing, or for some extra complicated 
constructs.  But here we're talking about the most simple struct hierarchy 
imaginable.  Having to write manual markers for that one is absurd IMO.


Ciao,
Michael.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-30 15:21         ` Michael Matz
@ 2013-08-30 15:26           ` Gabriel Dos Reis
  2013-08-30 15:31           ` Diego Novillo
  1 sibling, 0 replies; 44+ messages in thread
From: Gabriel Dos Reis @ 2013-08-30 15:26 UTC (permalink / raw)
  To: Michael Matz; +Cc: Jakub Jelinek, David Malcolm, gcc-patches

On Fri, Aug 30, 2013 at 10:21 AM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> On Fri, 30 Aug 2013, Gabriel Dos Reis wrote:
>
>> On Fri, Aug 30, 2013 at 9:02 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>>
>> >> I thought the principle that was acquired was that gengtype shouldn't
>> >> be improved to support more than what it does now….
>> >
>> > If it means that we'll need to write and maintain tons of hand written code
>> > that could otherwise be generated and maintained by a tool for us, that
>> > principle doesn't look very good.
>
> Exactly.
>
>> Back in March 2013, I asked about gengtype support for inheritance.
>>    http://gcc.gnu.org/ml/gcc/2013-03/msg00273.html
>> This
>>    http://gcc.gnu.org/ml/gcc/2013-03/msg00295.html
>> was the definitive answer that appeared to be the consensus.
>
> Well, it was a wrong decision then.  For some smaller types writing manual
> marker might be a sensible thing, or for some extra complicated
> constructs.  But here we're talking about the most simple struct hierarchy
> imaginable.  Having to write manual markers for that one is absurd IMO.

We can reserve the emotional strong words for later.  For now, the focus
should be for us to ensure we are being consistent and making design
decisions that carry consensus, hence my original note.

-- Gaby

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-30 15:21         ` Michael Matz
  2013-08-30 15:26           ` Gabriel Dos Reis
@ 2013-08-30 15:31           ` Diego Novillo
  2013-08-30 15:40             ` Jakub Jelinek
  1 sibling, 1 reply; 44+ messages in thread
From: Diego Novillo @ 2013-08-30 15:31 UTC (permalink / raw)
  To: Michael Matz; +Cc: Gabriel Dos Reis, Jakub Jelinek, David Malcolm, gcc-patches

On Fri, Aug 30, 2013 at 11:21 AM, Michael Matz <matz@suse.de> wrote:
>
> Hi,
>
> On Fri, 30 Aug 2013, Gabriel Dos Reis wrote:
>
> > On Fri, Aug 30, 2013 at 9:02 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> >
> > >> I thought the principle that was acquired was that gengtype shouldn't
> > >> be improved to support more than what it does now….
> > >
> > > If it means that we'll need to write and maintain tons of hand written code
> > > that could otherwise be generated and maintained by a tool for us, that
> > > principle doesn't look very good.
>
> Exactly.
>
> > Back in March 2013, I asked about gengtype support for inheritance.
> >    http://gcc.gnu.org/ml/gcc/2013-03/msg00273.html
> > This
> >    http://gcc.gnu.org/ml/gcc/2013-03/msg00295.html
> > was the definitive answer that appeared to be the consensus.
>
> Well, it was a wrong decision then.  For some smaller types writing manual
> marker might be a sensible thing, or for some extra complicated
> constructs.  But here we're talking about the most simple struct hierarchy
> imaginable.  Having to write manual markers for that one is absurd IMO.

I want to discourage extending gengtype more than necessary. Long
term, I would like to see memory pools replacing GC. However, that is
likely a long road so we should find an interim solution.

I vaguely remember thinking about what would be needed to have
gengtype deal with inheritance. It needed some pretty ugly
annotations. This made gengtype even more magic.  That's very bad for
maintenance.

One thing I liked is a suggestion that went something along the lines
of creating some base templates that could be used to facilitate
writing the manual markers.

Perhaps we could minimally extend gengtype to generate those. But I
think we can take advantage of C++ template features to facilitate
this.


Diego.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-30 15:31           ` Diego Novillo
@ 2013-08-30 15:40             ` Jakub Jelinek
  2013-08-30 15:50               ` Diego Novillo
                                 ` (2 more replies)
  0 siblings, 3 replies; 44+ messages in thread
From: Jakub Jelinek @ 2013-08-30 15:40 UTC (permalink / raw)
  To: Diego Novillo; +Cc: Michael Matz, Gabriel Dos Reis, David Malcolm, gcc-patches

On Fri, Aug 30, 2013 at 11:28:34AM -0400, Diego Novillo wrote:
> > Well, it was a wrong decision then.  For some smaller types writing manual
> > marker might be a sensible thing, or for some extra complicated
> > constructs.  But here we're talking about the most simple struct hierarchy
> > imaginable.  Having to write manual markers for that one is absurd IMO.
> 
> I want to discourage extending gengtype more than necessary. Long
> term, I would like to see memory pools replacing GC. However, that is
> likely a long road so we should find an interim solution.

I have doubts that, still somewhat remember the obstack era and memory pools
would again bring all the issues back, but let's put that aside.

> I vaguely remember thinking about what would be needed to have
> gengtype deal with inheritance. It needed some pretty ugly
> annotations. This made gengtype even more magic.  That's very bad for
> maintenance.

Teaching the gengtype parser about
{struct,class} name : public {struct,class} someothername { ... }
as opposed to current
{struct,class} name { ... }
shouldn't be that hard.  And, if the complaint is that we'd need to write
whole C++ parser for it, then the response could be that we already have
one C++ parser (and, even have plugin support and/or emit dwarf etc.).
So, gengtype could very well use a g++ plugin to emit the stuff it needs,
or parse DWARF, etc.  And, we even could not require everybody actually
emitting those themselves, we could check some text form of that
(gengtype.state?) into the tree, so only people actually changing the
compiler would need to run the plugin.

> One thing I liked is a suggestion that went something along the lines
> of creating some base templates that could be used to facilitate
> writing the manual markers.

Even if you have some stuff that helps you writing those, still it will be a
big source of bugs (very hard to debug) and a maintainance nightmare.

	Jakub

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-30 15:40             ` Jakub Jelinek
@ 2013-08-30 15:50               ` Diego Novillo
  2013-08-31 10:53                 ` Richard Biener
  2013-08-30 15:54               ` Gabriel Dos Reis
  2013-08-31  9:56               ` Richard Biener
  2 siblings, 1 reply; 44+ messages in thread
From: Diego Novillo @ 2013-08-30 15:50 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Michael Matz, Gabriel Dos Reis, David Malcolm, gcc-patches

On Fri, Aug 30, 2013 at 11:37 AM, Jakub Jelinek <jakub@redhat.com> wrote:

> Teaching the gengtype parser about
> {struct,class} name : public {struct,class} someothername { ... }
> as opposed to current
> {struct,class} name { ... }
> shouldn't be that hard.  And, if the complaint is that we'd need to write
> whole C++ parser for it, then the response could be that we already have
> one C++ parser (and, even have plugin support and/or emit dwarf etc.).

It isn't.  It's annoying and a duplication of effort.

> So, gengtype could very well use a g++ plugin to emit the stuff it needs,
> or parse DWARF, etc.  And, we even could not require everybody actually
> emitting those themselves, we could check some text form of that
> (gengtype.state?) into the tree, so only people actually changing the
> compiler would need to run the plugin.

Yes.  Lawrence and I thought about moving gengtype inside g++.  That
seemed like a promising approach.

> Even if you have some stuff that helps you writing those, still it will be a
> big source of bugs (very hard to debug) and a maintainance nightmare.

Debugging gengtype is much harder.  It is magic code that is not visible.


Diego.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-30 15:40             ` Jakub Jelinek
  2013-08-30 15:50               ` Diego Novillo
@ 2013-08-30 15:54               ` Gabriel Dos Reis
  2013-08-31  9:56               ` Richard Biener
  2 siblings, 0 replies; 44+ messages in thread
From: Gabriel Dos Reis @ 2013-08-30 15:54 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Diego Novillo, Michael Matz, David Malcolm, gcc-patches

On Fri, Aug 30, 2013 at 10:37 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Aug 30, 2013 at 11:28:34AM -0400, Diego Novillo wrote:
>> > Well, it was a wrong decision then.  For some smaller types writing manual
>> > marker might be a sensible thing, or for some extra complicated
>> > constructs.  But here we're talking about the most simple struct hierarchy
>> > imaginable.  Having to write manual markers for that one is absurd IMO.
>>
>> I want to discourage extending gengtype more than necessary. Long
>> term, I would like to see memory pools replacing GC. However, that is
>> likely a long road so we should find an interim solution.
>
> I have doubts that, still somewhat remember the obstack era and memory pools
> would again bring all the issues back, but let's put that aside.

We didn't have the automation of RAII with obstacle, back then.
We do have evidence of widely use industrial
C and C++ compilers that are built around the idea of memory pools.

>> I vaguely remember thinking about what would be needed to have
>> gengtype deal with inheritance. It needed some pretty ugly
>> annotations. This made gengtype even more magic.  That's very bad for
>> maintenance.
>
> Teaching the gengtype parser about
> {struct,class} name : public {struct,class} someothername { … }

I would suggest NOT to allow the {struct, class} part in the base-class clause.

> as opposed to current
> {struct,class} name { ... }
> shouldn't be that hard.  And, if the complaint is that we'd need to write
> whole C++ parser for it, then the response could be that we already have
> one C++ parser (and, even have plugin support and/or emit dwarf etc.).

We should not need one.  At most the base classes would have the form

   optional-typename optional-qualifying-scope  identifier
optional-template-argument-list

that should cover most of what we want.

> So, gengtype could very well use a g++ plugin to emit the stuff it needs,
> or parse DWARF, etc.  And, we even could not require everybody actually
> emitting those themselves, we could check some text form of that
> (gengtype.state?) into the tree, so only people actually changing the
> compiler would need to run the plugin.
>
>> One thing I liked is a suggestion that went something along the lines
>> of creating some base templates that could be used to facilitate
>> writing the manual markers.
>
> Even if you have some stuff that helps you writing those, still it will be a
> big source of bugs (very hard to debug) and a maintainance nightmare.
>
>         Jakub

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-30 13:58 ` [PATCH 0/6] Convert gimple to a C++ class hierarchy Michael Matz
  2013-08-30 14:02   ` Gabriel Dos Reis
@ 2013-08-30 19:38   ` David Malcolm
  2013-09-02 11:45     ` Michael Matz
  2013-09-02 12:35     ` Martin Jambor
  1 sibling, 2 replies; 44+ messages in thread
From: David Malcolm @ 2013-08-30 19:38 UTC (permalink / raw)
  To: Michael Matz; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2568 bytes --]

On Fri, 2013-08-30 at 15:44 +0200, Michael Matz wrote:
> Hi,
> 
> On Thu, 29 Aug 2013, David Malcolm wrote:
> 
> > Successfully bootstrapped and tested on x86_64-unknown-linux-gnu: all
> > testcases show the same results as an unpatched build (relative to
> > r202029).
> 
> I'd like to see some statistics for cc1{,plus} codesize and for compile 
> time of something reasonably large (needing say 60 seconds to 
> compile normally), before/after patch series.

Here's the result of a pair of builds of r202029 without and with the
patches, configured with --enable-checking=release, running "make", then
stripping debuginfo [1]

# ll */build/gcc/cc1
-rwxr-xr-x. 1 root root 13230048 Aug 30 15:00 control/build/gcc/cc1
-rwxr-xr-x. 1 root root 13230144 Aug 30 15:00 experiment/build/gcc/cc1
(98 bytes difference)

# ll */build/gcc/cc1obj
-rwxr-xr-x. 1 root root 13426336 Aug 30 15:00 control/build/gcc/cc1obj
-rwxr-xr-x. 1 root root 13426432 Aug 30 15:00 experiment/build/gcc/cc1obj
(96 bytes diff)

# ll */build/gcc/cc1plus
-rwxr-xr-x. 1 root root 14328480 Aug 29 13:59 control/build/gcc/cc1plus
-rwxr-xr-x. 1 root root 14328608 Aug 29 13:59 experiment/build/gcc/cc1plus
(128 bytes diff)

# ll */build/gcc/f951
-rwxr-xr-x. 1 root root 13960728 Aug 30 15:05 control/build/gcc/f951
-rwxr-xr-x. 1 root root 13960856 Aug 30 15:05 experiment/build/gcc/f951
(128 bytes diff)

# ll */build/gcc/jc1
-rwxr-xr-x. 1 root root 12607704 Aug 30 15:17 control/build/gcc/jc1
-rwxr-xr-x. 1 root root 12607704 Aug 30 15:17 experiment/build/gcc/jc1
(the same size)

So the overall sizes of such binaries are essentially unchanged.

To dig a bit deeper, I extended my asmdiff tool [2] to compare sizes of
functions; I'm attaching the results of comparing cc1plus before/after.

Any suggestions on what to compile to compare performance?  By 60
seconds, do you mean 60s for one TU, or do you mean a large build e.g.
the linux kernel?

> And the manual GTY markers are so not maintainable in the long run, 
> gengtype or something else really needs to be taught to create them 
> automatically.

Apart from the GTY aspect, how do people feel about the patch series?
FWIW I have vague thoughts about doing something similar for tree -
doing so *might* give an easier route to the type vs expression
separation that Andrew spoke about at the Cauldron rearchitecture BoF.
(I started looking at doing a similar C++-ification of rtx, but...
gahhhhh)

Dave

[1] yes, I built as root; this was done on a throwaway provisioning of a
RHEL 6.4 x86_64 box.
[2] https://github.com/davidmalcolm/asmdiff

[-- Attachment #2: diff.txt --]
[-- Type: text/plain, Size: 12971 bytes --]

Old: test/control/build/gcc/cc1plus
New: test/experiment/build/gcc/cc1plus
  Function removed: Function('_start-0x1a684')
  Function removed: Function('gt_pch_p_18gimple_statement_d(void*, void*, void (*)(void*, void*), void*)')
  Function removed: Function('gt_ggc_mx_gimple_statement_d(void*)')
  Function removed: Function('gt_pch_nx_gimple_statement_d(void*)')
  Function removed: Function('vec<constraint_expr, va_heap, vl_ptr>::safe_push(constraint_expr const&)')
  Function added: Function('_start-0x1a6ac')
  Function added: Function('gt_ggc_mx(gimple_statement_base*)')
  Function added: Function('gt_pch_nx(gimple_statement_base*)')
  Function added: Function('gt_pch_nx(gimple_statement_base*, void (*)(void*, void*), void*)')
  Function added: Function('gt_pch_p_21gimple_statement_base(void*, void*, void (*)(void*, void*), void*)')
  Function added: Function('gt_ggc_mx_gimple_statement_base(void*)')
  Function added: Function('gt_pch_nx_gimple_statement_base(void*)')
  Function hash_table<oecount_hasher, xcallocator>::find_slot_with_hash(void const*, unsigned int, insert_option) changed size from 5984 to 5872 bytes
  Function same_succ_def::equal(same_succ_def const*, same_succ_def const*) changed size from 480 to 512 bytes
  Function maybe_remove_unreachable_handlers() changed size from 112 to 128 bytes
  Function verify_ssa_operands(gimple_statement_d*) changed size from 927 to 960 bytes
    (renamed to verify_ssa_operands(gimple_statement_base*))
  Function vect_analyze_data_ref_accesses(_loop_vec_info*, _bb_vec_info*) changed size from 3920 to 3872 bytes
  Function make_pass_ipa_pta(gcc::context*) changed size from 36880 to 16464 bytes
  Function inline_merge_summary(cgraph_edge*) changed size from 17904 to 17920 bytes
  Function release_defs_bitset(bitmap_head_def*) changed size from 2464 to 2304 bytes
  Function gt_pch_nx(loop*&) changed size from 144 to 32 bytes
  Function make_pass_fold_builtins(gcc::context*) changed size from 5711 to 5727 bytes
  Function dump_value_range(_IO_FILE*, value_range_d*) changed size from 32880 to 32800 bytes
  Function extract_true_false_edges_from_block(basic_block_def*, edge_def**, edge_def**) changed size from 20208 to 20224 bytes
  Function tree_ssa_lim() changed size from 4847 to 4831 bytes
  Function gt_ggc_mx_control_flow_graph(void*) changed size from 96 to 192 bytes
  Function vectorize_loops() changed size from 3999 to 4191 bytes
  Function dump_node(tree_node const*, int, _IO_FILE*) changed size from 10784 to 10928 bytes
  Function stmt_can_throw_external(gimple_statement_d*) changed size from 6399 to 6431 bytes
    (renamed to stmt_can_throw_external(gimple_statement_base*))
  Function remove_phi_args(edge_def*) changed size from 272 to 288 bytes
  Function degenerate_phi_result(gimple_statement_d*) changed size from 6608 to 6592 bytes
    (renamed to degenerate_phi_result(gimple_statement_base*))
  Function make_pass_fre(gcc::context*) changed size from 32815 to 33055 bytes
  Function make_pass_strlen(gcc::context*) changed size from 16815 to 16207 bytes
  Function build_type_cast(tree_node*, gimple_statement_d*, ssa_mode) changed size from 1376 to 47 bytes
    (renamed to build_type_cast(tree_node*, gimple_statement_base*, ssa_mode))
  Function replace_uses_by(tree_node*, tree_node*) changed size from 5600 to 5632 bytes
  Function update_stmt_operands(gimple_statement_d*) changed size from 3968 to 4000 bytes
    (renamed to update_stmt_operands(gimple_statement_base*))
  Function gimplify_and_update_call_from_tree(gimple_stmt_iterator_d*, tree_node*) changed size from 1232 to 1200 bytes
  Function hash_table<expr_elt_hasher, xcallocator>::find_slot_with_hash(expr_hash_elt const*, unsigned int, insert_option) changed size from 7392 to 7359 bytes
  Function split_constant_offset(tree_node*, tree_node**, tree_node**) changed size from 1440 to 1392 bytes
  Function hash_table<iv_inv_expr_hasher, xcallocator>::find_slot_with_hash(iv_inv_expr_ent const*, unsigned int, insert_option) changed size from 1328 to 1312 bytes
  Function dump_dfa_stats(_IO_FILE*) changed size from 1791 to 1824 bytes
  Function copy_decl_no_change(tree_node*, copy_body_data*) changed size from 4384 to 4352 bytes
  Function _start changed size from 956 to 948 bytes
  Function vn_nary_op_insert_stmt(gimple_statement_d*, tree_node*) changed size from 8896 to 9824 bytes
    (renamed to vn_nary_op_insert_stmt(gimple_statement_base*, tree_node*))
  Function vect_finish_stmt_generation(gimple_statement_d*, gimple_statement_d*, gimple_stmt_iterator_d*) changed size from 1184 to 1200 bytes
    (renamed to vect_finish_stmt_generation(gimple_statement_base*, gimple_statement_base*, gimple_stmt_iterator_d*))
  Function hash_table<ssa_names_hasher, xcallocator>::find_slot_with_hash(name_to_bb const*, unsigned int, insert_option) changed size from 7248 to 7216 bytes
  Function loop_niter_by_eval(loop*, edge_def*) changed size from 848 to 864 bytes
  Function move_ssa_defining_stmt_for_defs(gimple_statement_d*, gimple_statement_d*) changed size from 848 to 832 bytes
    (renamed to move_ssa_defining_stmt_for_defs(gimple_statement_base*, gimple_statement_base*))
  Function update_call_from_tree(gimple_stmt_iterator_d*, tree_node*) changed size from 943 to 928 bytes
  Function reset_debug_uses(gimple_statement_d*) changed size from 2352 to 2304 bytes
    (renamed to reset_debug_uses(gimple_statement_base*))
  Function void va_stack::reserve<adjust_info>(vec<adjust_info, va_stack, vl_embed>*&, unsigned int, bool) changed size from 17296 to 17088 bytes
  Function hash_table<simduid_to_vf, xcallocator>::expand() changed size from 50672 to 51088 bytes
  Function gimple_stmt_may_fallthru(gimple_statement_d*) changed size from 287 to 272 bytes
    (renamed to gimple_stmt_may_fallthru(gimple_statement_base*))
  Function gimple_call_copy_skip_args(gimple_statement_d*, bitmap_head_def*) changed size from 672 to 688 bytes
    (renamed to gimple_call_copy_skip_args(gimple_statement_base*, bitmap_head_def*))
  Function make_pass_diagnose_omp_blocks(gcc::context*) changed size from 14128 to 14144 bytes
  Function mark_virtual_phi_result_for_renaming(gimple_statement_d*) changed size from 8800 to 8736 bytes
    (renamed to mark_virtual_phi_result_for_renaming(gimple_statement_base*))
  Function make_pass_early_ipa_sra(gcc::context*) changed size from 21151 to 21167 bytes
  Function vect_pattern_recog(_loop_vec_info*, _bb_vec_info*) changed size from 10095 to 10112 bytes
  Function input_bb(lto_input_block*, LTO_tags, data_in*, function*, int) changed size from 2912 to 2975 bytes
  Function gimple_find_values_to_profile(vec<histogram_value_t*, va_heap, vl_ptr>*) changed size from 15888 to 15856 bytes
  Function walk_gimple_op(gimple_statement_d*, tree_node* (*)(tree_node**, int*, void*), walk_stmt_info*) changed size from 2064 to 2000 bytes
    (renamed to walk_gimple_op(gimple_statement_base*, tree_node* (*)(tree_node**, int*, void*), walk_stmt_info*))
  Function make_pass_ch(gcc::context*) changed size from 22672 to 22704 bytes
  Function vect_can_advance_ivs_p(_loop_vec_info*) changed size from 6880 to 7472 bytes
  Function lto_end_uncompression(lto_compression_stream*) changed size from 31600 to 31536 bytes
  Function gimple_copy(gimple_statement_d*) changed size from 928 to 895 bytes
    (renamed to gimple_copy(gimple_statement_base*))
  Function coalesce_ssa_name() changed size from 8911 to 8895 bytes
  Function fixup_noreturn_call(gimple_statement_d*) changed size from 2544 to 2560 bytes
    (renamed to fixup_noreturn_call(gimple_statement_base*))
  Function make_pass_strength_reduction(gcc::context*) changed size from 8239 to 8223 bytes
  Function hash_table<vn_phi_hasher, xcallocator>::find_slot_with_hash(vn_phi_s const*, unsigned int, insert_option) changed size from 5904 to 5728 bytes
  Function copy_reference_ops_from_call(gimple_statement_d*, vec<vn_reference_op_struct, va_heap, vl_ptr>*) changed size from 911 to 928 bytes
    (renamed to copy_reference_ops_from_call(gimple_statement_base*, vec<vn_reference_op_struct, va_heap, vl_ptr>*))
  Function vec<data_dependence_relation*, va_heap, vl_ptr>::reserve(unsigned int, bool) changed size from 3664 to 3648 bytes
  Function hash_table<mem_ref_hasher, xcallocator>::find_slot_with_hash(tree_node const*, unsigned int, insert_option) changed size from 2848 to 2832 bytes
  Function vec<vec<tree_node*, va_heap, vl_ptr>, va_heap, vl_ptr>::reserve(unsigned int, bool) changed size from 3216 to 3200 bytes
  Function execute_update_addresses_taken() changed size from 2784 to 2768 bytes
  Function vectorizable_condition(gimple_statement_d*, gimple_stmt_iterator_d*, gimple_statement_d**, tree_node*, int, _slp_tree*) changed size from 17648 to 17696 bytes
    (renamed to vectorizable_condition(gimple_statement_base*, gimple_stmt_iterator_d*, gimple_statement_base**, tree_node*, int, _slp_tree*))
  Function supportable_narrowing_operation(tree_code, tree_node*, tree_node*, tree_code*, int*, vec<tree_node*, va_heap, vl_ptr>*) changed size from 19984 to 19968 bytes
  Function vect_record_grouped_load_vectors(gimple_statement_d*, vec<tree_node*, va_heap, vl_ptr>) changed size from 384 to 400 bytes
    (renamed to vect_record_grouped_load_vectors(gimple_statement_base*, vec<tree_node*, va_heap, vl_ptr>))
  Function vect_enhance_data_refs_alignment(_loop_vec_info*) changed size from 5328 to 5312 bytes
  Function expr_last(tree_node*) changed size from 16848 to 16816 bytes
  Function make_pass_dse(gcc::context*) changed size from 39600 to 39424 bytes
  Function stmt_could_throw_p(gimple_statement_d*) changed size from 768 to 752 bytes
    (renamed to stmt_could_throw_p(gimple_statement_base*))
  Function void va_stack::release<tree_node*>(vec<tree_node*, va_stack, vl_embed>*&) changed size from 22096 to 22192 bytes
  Function substitute_and_fold(tree_node* (*)(tree_node*), bool (*)(gimple_stmt_iterator_d*), bool) changed size from 3728 to 3696 bytes
  Function void va_gc::reserve<gimple_statement_d*, va_gc>(vec<gimple_statement_d*, va_gc, vl_embed>*&, unsigned int, bool) changed size from 27552 to 27808 bytes
    (renamed to void va_gc::reserve<gimple_statement_base*, va_gc>(vec<gimple_statement_base*, va_gc, vl_embed>*&, unsigned int, bool))
  Function blocks_in_phiopt_order() changed size from 11568 to 11536 bytes
  Function debug_currdefs() changed size from 11408 to 11456 bytes
  Function make_pass_ipa_lower_emutls(gcc::context*) changed size from 11888 to 11920 bytes
  Function vn_nary_op_insert(tree_node*, tree_node*) changed size from 2128 to 2144 bytes
  Function make_pass_phiprop(gcc::context*) changed size from 13184 to 13168 bytes
  Function voidify_wrapper_expr(tree_node*, tree_node*) changed size from 3408 to 1376 bytes
  Function canonicalize_loop_ivs(loop*, tree_node**, bool) changed size from 1856 to 1904 bytes
  Function multiplier_allowed_in_address_p(long, machine_mode, unsigned char) changed size from 11296 to 11280 bytes
  Function gt_ggc_mx(loop*&) changed size from 128 to 32 bytes
  Function cgraph_redirect_edge_call_stmt_to_callee(cgraph_edge*) changed size from 1696 to 1664 bytes
  Function operation_could_trap_helper_p(tree_code, bool, bool, bool, bool, tree_node*, bool*) changed size from 208 to 224 bytes
  Function compute_may_aliases() changed size from 8624 to 8639 bytes
  Function make_pass_lower_vector_ssa(gcc::context*) changed size from 2768 to 2783 bytes
  Function dump_all_asserts(_IO_FILE*) changed size from 6640 to 6688 bytes
  Function find_replaceable_exprs(_var_map*) changed size from 5328 to 5263 bytes
  Function record_vars_into(tree_node*, tree_node*) changed size from 4320 to 4336 bytes
  Function make_pass_postreload_cse(gcc::context*) changed size from 9632 to 9616 bytes
  Function expr_invariant_in_loop_p(loop*, tree_node*) changed size from 3200 to 3232 bytes
  Function hash_table<stridxlist_hasher, xcallocator>::expand() changed size from 14784 to 35232 bytes
  Function make_pass_tree_loop_done(gcc::context*) changed size from 31904 to 31952 bytes
  Function omp_firstprivatize_variable(gimplify_omp_ctx*, tree_node*) changed size from 1023 to 3056 bytes
  Function make_pass_phi_only_cprop(gcc::context*) changed size from 12511 to 12495 bytes
  Function gimplify_self_mod_expr(tree_node**, gimple_statement_d**, gimple_statement_d**, bool, tree_node*) changed size from 19632 to 19600 bytes
    (renamed to gimplify_self_mod_expr(tree_node**, gimple_statement_base**, gimple_statement_base**, bool, tree_node*))
  Function make_pass_rename_ssa_copies(gcc::context*) changed size from 9840 to 9776 bytes
  Function update_ssa(unsigned int) changed size from 15328 to 15344 bytes
  Function gt_pch_nx_control_flow_graph(void*) changed size from 96 to 208 bytes
  Function hash_table<instantiate_cache_entry_hasher, xcallocator>::expand() changed size from 11664 to 11680 bytes
  Function build_duplicate_type(tree_node*) changed size from 11888 to 11872 bytes

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 6/6] Add manual GTY hooks
  2013-08-30  8:43   ` [PATCH 6/6] " Richard Biener
@ 2013-08-30 20:25     ` David Malcolm
  0 siblings, 0 replies; 44+ messages in thread
From: David Malcolm @ 2013-08-30 20:25 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

On Fri, 2013-08-30 at 10:40 +0200, Richard Biener wrote:
> On Thu, Aug 29, 2013 at 6:20 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> >         * gimple.c (gt_ggc_mx (gimple)): New, as required by GTY((user)).
> >         (gt_pch_nx (gimple)): Likewise.
> >         (gt_pch_nx (gimple, gt_pointer_operator, void *)): Likewise.
> >         * gimple.h  (gt_ggc_mx (gimple)): Declare.
> >         (gt_pch_nx (gimple)): Declare.
> >         (gt_pch_nx (gimple, gt_pointer_operator, void *)): Declare.
> >         * tree-cfg.c (ggc_mx (gimple&)): Remove declaration, as this
> >         collides with the function that GTY((user)) expects.
> >         (gt_ggc_mx (edge_def *)): Replace call to gt_ggc_mx on the
> >         gimple with gt_ggc_mx_gimple_statement_base: in the
> >         pre-GTY((user)) world, "gt_ggc_mx" was the function to be called
> >         on a possibly NULL pointed to check if needed marking and if so
> >         to traverse its fields.  In the GTY((user)) world, "gt_ggc_mx"
> >         is the function to be called on non-NULL objects immediately *after*
> >         they have been marked: it does not mark the object itself.
> >         (gt_pch_nx (gimple&)): Remove declaration.
> >         (gt_pch_nx (edge_def *)): Update as per the mx hook.
> 
> Btw, this shows that gimple isn't a true C++ hierarchy - because of GTY
> you can only ever use 'gimple' pointers, not more specialized ones
> like gimple_phi as you are missing the GTY machinery for them.

One can definitely use pointers to subclasses on the *stack*, indeed,
the patches use this throughout; IMHO it's an improvement.

FWIW I did a quick test and was also able to add dummy bss subclass ptrs
e.g.:

  static GTY(()) gimple_statement_phi *dummy_phi;

This leads to gengtype creating code like this in gtype-desc.c:

void
gt_ggc_mx_gimple_statement_phi (void *x_p)
{
  gimple_statement_phi * const x = (gimple_statement_phi *)x_p;
  if (ggc_test_and_set_mark (x))
    {
      gt_ggc_mx (x);
    }
}

Note how the "gt_ggc_mx (x);" is resolved by using the base-class
implementation of gt_ggc_mx i.e.: gt_ggc_mx (gimple )
since a gimple_statement_phi * is-a gimple_statement_base *.

Anything more complicated than that (e.g. containers) and we're hitting
up against the limitations of gengtype, I guess.

> I'm not 100% convinced that we should do all this at this point without
> getting a better hand on the gengtype issues (it's partial transition to
> the C++ world of GCC)

[...]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-29 16:20 [PATCH 0/6] Convert gimple to a C++ class hierarchy David Malcolm
                   ` (6 preceding siblings ...)
  2013-08-30 13:58 ` [PATCH 0/6] Convert gimple to a C++ class hierarchy Michael Matz
@ 2013-08-30 21:51 ` David Malcolm
  7 siblings, 0 replies; 44+ messages in thread
From: David Malcolm @ 2013-08-30 21:51 UTC (permalink / raw)
  To: gcc-patches

On Thu, 2013-08-29 at 12:20 -0400, David Malcolm wrote:
[...]

> Successfully bootstrapped and tested on x86_64-unknown-linux-gnu: all
> testcases show the same results as an unpatched build (relative to
> r202029).

I messed up the testing for this by accidentally configuring the builds
with --enable-checking=release, thus not running the run-time checks.

Upon rechecking with a plain "./configure" (and thus enabling the
run-time checks), the patches break 20 test cases involving OpenMP.

Sorry about this.

I'm working on a fix [1]; I'll also add gcc_unreachable to the
PCH-related hooks for gimple (and thus reduce the size of the
handwritten GTY code by a factor of 3; not sure if it will make it any
more popular...).

Dave

[1] I believe the issue is this overzealous generated is_a_helper test:

template <>
template <>
inline bool
is_a_helper <gimple_statement_omp_parallel>::test (gimple gs)
{
  return gs->code == GIMPLE_OMP_PARALLEL;
}

which should clearly also permit gs->code to be GIMPLE_OMP_TASK; this
bug leads to various failures in omp-lowering for obvious reasons.


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-30 15:40             ` Jakub Jelinek
  2013-08-30 15:50               ` Diego Novillo
  2013-08-30 15:54               ` Gabriel Dos Reis
@ 2013-08-31  9:56               ` Richard Biener
  2 siblings, 0 replies; 44+ messages in thread
From: Richard Biener @ 2013-08-31  9:56 UTC (permalink / raw)
  To: Jakub Jelinek, Jakub Jelinek, Diego Novillo
  Cc: Michael Matz, Gabriel Dos Reis, David Malcolm, gcc-patches

Jakub Jelinek <jakub@redhat.com> wrote:
>On Fri, Aug 30, 2013 at 11:28:34AM -0400, Diego Novillo wrote:
>> > Well, it was a wrong decision then.  For some smaller types writing
>manual
>> > marker might be a sensible thing, or for some extra complicated
>> > constructs.  But here we're talking about the most simple struct
>hierarchy
>> > imaginable.  Having to write manual markers for that one is absurd
>IMO.
>> 
>> I want to discourage extending gengtype more than necessary. Long
>> term, I would like to see memory pools replacing GC. However, that is
>> likely a long road so we should find an interim solution.
>
>I have doubts that, still somewhat remember the obstack era and memory
>pools
>would again bring all the issues back, but let's put that aside.
>
>> I vaguely remember thinking about what would be needed to have
>> gengtype deal with inheritance. It needed some pretty ugly
>> annotations. This made gengtype even more magic.  That's very bad for
>> maintenance.
>
>Teaching the gengtype parser about
>{struct,class} name : public {struct,class} someothername { ... }
>as opposed to current
>{struct,class} name { ... }
>shouldn't be that hard.  And, if the complaint is that we'd need to
>write
>whole C++ parser for it, then the response could be that we already
>have
>one C++ parser (and, even have plugin support and/or emit dwarf etc.).

Note that explicit markings was also about simply writing GTY((derivesfrom(xxx))) instead of extending the parser.  The result could be as simple as gengtype emitting

 ggc_mx ((xxx) *this)

In the marker routine.

Richard.

>So, gengtype could very well use a g++ plugin to emit the stuff it
>needs,
>or parse DWARF, etc.  And, we even could not require everybody actually
>emitting those themselves, we could check some text form of that
>(gengtype.state?) into the tree, so only people actually changing the
>compiler would need to run the plugin.
>
>> One thing I liked is a suggestion that went something along the lines
>> of creating some base templates that could be used to facilitate
>> writing the manual markers.
>
>Even if you have some stuff that helps you writing those, still it will
>be a
>big source of bugs (very hard to debug) and a maintainance nightmare.
>
>	Jakub


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-30 15:50               ` Diego Novillo
@ 2013-08-31 10:53                 ` Richard Biener
  2013-08-31 13:34                   ` Basile Starynkevitch
  2013-08-31 14:04                   ` Diego Novillo
  0 siblings, 2 replies; 44+ messages in thread
From: Richard Biener @ 2013-08-31 10:53 UTC (permalink / raw)
  To: Diego Novillo, Jakub Jelinek
  Cc: Michael Matz, Gabriel Dos Reis, David Malcolm, gcc-patches

Diego Novillo <dnovillo@google.com> wrote:
>On Fri, Aug 30, 2013 at 11:37 AM, Jakub Jelinek <jakub@redhat.com>
>wrote:
>
>> Teaching the gengtype parser about
>> {struct,class} name : public {struct,class} someothername { ... }
>> as opposed to current
>> {struct,class} name { ... }
>> shouldn't be that hard.  And, if the complaint is that we'd need to
>write
>> whole C++ parser for it, then the response could be that we already
>have
>> one C++ parser (and, even have plugin support and/or emit dwarf
>etc.).
>
>It isn't.  It's annoying and a duplication of effort.
>
>> So, gengtype could very well use a g++ plugin to emit the stuff it
>needs,
>> or parse DWARF, etc.  And, we even could not require everybody
>actually
>> emitting those themselves, we could check some text form of that
>> (gengtype.state?) into the tree, so only people actually changing the
>> compiler would need to run the plugin.
>
>Yes.  Lawrence and I thought about moving gengtype inside g++.  That
>seemed like a promising approach.


What do you do during stage1?  Have a collector that never collects?

Richard.

>> Even if you have some stuff that helps you writing those, still it
>will be a
>> big source of bugs (very hard to debug) and a maintainance nightmare.
>
>Debugging gengtype is much harder.  It is magic code that is not
>visible.
>
>
>Diego.


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-31 10:53                 ` Richard Biener
@ 2013-08-31 13:34                   ` Basile Starynkevitch
  2013-08-31 14:04                   ` Diego Novillo
  1 sibling, 0 replies; 44+ messages in thread
From: Basile Starynkevitch @ 2013-08-31 13:34 UTC (permalink / raw)
  To: gcc
  Cc: Diego Novillo, Jakub Jelinek, Michael Matz, Gabriel Dos Reis,
	David Malcolm, gcc-patches

On Sat, 2013-08-31 at 11:57 +0200, Richard Biener wrote:
> Diego Novillo <dnovillo@google.com> wrote:
> >
> >Yes.  Lawrence and I thought about moving gengtype inside g++.  That
> >seemed like a promising approach.
> 
> 
> What do you do during stage1?  Have a collector that never collects?

We could imagine that the successor of gengtype would be some GCC plugin
(which would generate C++ code for marking and GC and PCH purposes,
perhaps using ad-hoc attributes and pragmas)

Then for bootstrapping purposes, we could put the generated C++ code in
the source repository (like we already do for configure, or
fixincludes/fixincl.x etc...). Hence stage1 would be buildable with the
generated C++ code in the repository.

A more difficult issue is that the set of GTY-ed types is target
specific and depends upon the .../configure argument at build time.

Perhaps we could consider processing all of it  (i.e. every GTY-ed class
declaration), and have our gengtype successor plugin emit appropriate
#if in the generated C++ code.  

Of course having gengtype replaced by a plugin requires such a plugin to
be developed and GCC maintainers to have access to some gcc...

Cheers
-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***


^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH 3/6 v2] Autogenerated conversion of gimple to C++
  2013-08-29 16:20 ` [PATCH 3/6] Autogenerated conversion of gimple to C++ David Malcolm
@ 2013-08-31 13:40   ` David Malcolm
  0 siblings, 0 replies; 44+ messages in thread
From: David Malcolm @ 2013-08-31 13:40 UTC (permalink / raw)
  To: gcc-patches

On Thu, 2013-08-29 at 12:20 -0400, David Malcolm wrote:
> This patch is 110KB in size, so to avoid mailing-list size limits I've uploaded it to:
> 
> http://dmalcolm.fedorapeople.org/gcc/large-patches/a89d361b4f95dd216e1d29cb44fbaf90372c48b8-0003-Autogenerated-conversion-of-gimple-to-C.patch
> 
> The ChangeLog entry and diffstat follow:
> 
> gcc/
> 
> 	Patch autogenerated by refactor_gimple.py from
> 	https://github.com/davidmalcolm/gcc-refactoring-scripts
> 	revision 26aabff11750d29e6129930a426242a564538d1a

I've fixed the bug with OMP_TASK mentioned in 
  http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01934.html
by regenerating this patch with a fixed refactor_gimple.py [1]

Again, it's 110KB in size, exceeding the list limit, so I've uploaded it
here:
http://dmalcolm.fedorapeople.org/gcc/large-patches/733a06e20496464e373c1f48388cedf0b6935a4e-0003-Autogenerated-conversion-of-gimple-to-C.patch

The only effective difference between the patches is this:

 +inline bool
 +is_a_helper <gimple_statement_omp_parallel>::test (gimple gs)
 +{
-+  return gs->code == GIMPLE_OMP_PARALLEL;
++  return gs->code == GIMPLE_OMP_PARALLEL || gs->code == GIMPLE_OMP_TASK;
 +}
 +

(and the const equivalent).

Successfully bootstrapped on x86_64-unknown-linux-gnu with plain
"configure" and thus with checking enabled; all tests show same results
as a unpatched control build (of r202029), fixing the omp ICEs described
above.

Dave

[1] Specifically, this commit to my refactoring scripts:
https://github.com/davidmalcolm/gcc-refactoring-scripts/commit/d55c7a3e3d1227bad36284ffdb67c83fb089b272


^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH 6/6 v2] Add manual GTY hooks
  2013-08-30  8:09   ` Richard Biener
@ 2013-08-31 13:48     ` David Malcolm
  0 siblings, 0 replies; 44+ messages in thread
From: David Malcolm @ 2013-08-31 13:48 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 985 bytes --]

On Fri, 2013-08-30 at 10:04 +0200, Richard Biener wrote:
> On Thu, Aug 29, 2013 at 6:20 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> >         * gimple.c (gt_ggc_mx (gimple)): New, as required by GTY((user)).
> >         (gt_pch_nx (gimple)): Likewise.
> >         (gt_pch_nx (gimple, gt_pointer_operator, void *)): Likewise.
> >         * gimple.h  (gt_ggc_mx (gimple)): Declare.
> >         (gt_pch_nx (gimple)): Declare.
> >         (gt_pch_nx (gimple, gt_pointer_operator, void *)): Declare.
> 
> No GIMPLE should reside in PCHs so you should be able to just put
> gcc_unreachable () in them ... (if dropping them does not work)

Thanks.  I'm attaching a revised version of the patch which does this,
bringing the length of the hand-written GTY code for this down from 741
lines to 267 lines.

Successfully bootstrapped on x86_64-unknown-linux-gnu (with plain
"configure" and thus with checking enabled); all tests show same results
as a unpatched control build (of r202029).


[-- Attachment #2: 0006-Add-manual-GTY-hooks.patch --]
[-- Type: text/x-patch, Size: 10957 bytes --]

From 4edea4c28c624d4d308d8be731f4415d22e10f14 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Mon, 26 Aug 2013 20:41:04 -0400
Subject: [PATCH 6/6] Add manual GTY hooks

	* gimple.c (gt_ggc_mx (gimple)): New, as required by GTY((user)).
	(gt_pch_nx (gimple)): New, stub implementation.
	(gt_pch_nx (gimple, gt_pointer_operator, void *)): Likewise.
	* gimple.h  (gt_ggc_mx (gimple)): Declare.
	(gt_pch_nx (gimple)): Declare.
	(gt_pch_nx (gimple, gt_pointer_operator, void *)): Declare.
	* tree-cfg.c (ggc_mx (gimple&)): Remove declaration, as this
	collides with the function that GTY((user)) expects.
	(gt_ggc_mx (edge_def *)): Replace call to gt_ggc_mx on the
	gimple with gt_ggc_mx_gimple_statement_base: in the
	pre-GTY((user)) world, "gt_ggc_mx" was the function to be called
	on a possibly NULL pointed to check if needed marking and if so
	to traverse its fields.  In the GTY((user)) world, "gt_ggc_mx"
	is the function to be called on non-NULL objects immediately *after*
	they have been marked: it does not mark the object itself.
	(gt_pch_nx (gimple&)): Remove declaration.
	(gt_pch_nx (edge_def *)): Update as per the mx hook.
---
 gcc/gimple.c   | 269 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gcc/gimple.h   |   6 ++
 gcc/tree-cfg.c |   6 +-
 3 files changed, 277 insertions(+), 4 deletions(-)

diff --git a/gcc/gimple.c b/gcc/gimple.c
index 1ad36d1..a5b4799 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -4338,4 +4338,273 @@ build_type_cast (tree to_type, gimple op, enum ssa_mode mode)
   return build_type_cast (to_type, gimple_assign_lhs (op), mode);
 }
 
+void
+gt_ggc_mx (gimple gs)
+{
+  gimple x = gs;
+  /* Emulation of the "chain_next" GTY attribute.
+
+     gs has already been marked.
+     Iterate the chain of next statements, marking until we reach one that
+     has already been marked, or NULL.   */
+  gimple xlimit = gs->next;
+  while (ggc_test_and_set_mark (xlimit))
+    xlimit = xlimit->next;
+
+  /* All of the statements within the half-open interval [x..xlimit) have
+     just been marked.  Iterate through the list, visiting their fields.  */
+  while (x != xlimit)
+    {
+      gt_ggc_m_15basic_block_def (x->bb);
+      switch (gimple_statement_structure (&((*x))))
+	{
+	case GSS_BASE:
+	  break;
+	case GSS_WITH_OPS:
+	  {
+	    gimple_statement_with_ops *stmt
+	      = static_cast <gimple_statement_with_ops *> (x);
+	    size_t num = (size_t)(stmt->num_ops);
+	    for (size_t i = 0; i != num; i++)
+	      gt_ggc_m_9tree_node (stmt->op[i]);
+	  }
+	  break;
+	case GSS_WITH_MEM_OPS_BASE:
+	  break;
+	case GSS_WITH_MEM_OPS:
+	  {
+	    gimple_statement_with_memory_ops *stmt
+	      = static_cast <gimple_statement_with_memory_ops *> (x);
+	    size_t num = (size_t)(stmt->num_ops);
+	    for (size_t i = 0; i != num; i++)
+	      gt_ggc_m_9tree_node (stmt->op[i]);
+	  }
+	  break;
+	case GSS_CALL:
+	  {
+	    gimple_statement_call *stmt
+	      = static_cast <gimple_statement_call *> (x);
+	    gt_ggc_m_15bitmap_head_def (stmt->call_used.vars);
+	    gt_ggc_m_15bitmap_head_def (stmt->call_clobbered.vars);
+	    switch (stmt->subcode & GF_CALL_INTERNAL)
+	      {
+	      case 0:
+		gt_ggc_m_9tree_node (stmt->u.fntype);
+		break;
+	      case GF_CALL_INTERNAL:
+		break;
+	      default:
+		break;
+	      }
+	    size_t num = (size_t)(stmt->num_ops);
+	    for (size_t i = 0; i != num; i++)
+	      gt_ggc_m_9tree_node (stmt->op[i]);
+	  }
+	  break;
+	case GSS_OMP:
+	  {
+	    gimple_statement_omp *stmt
+	      = static_cast <gimple_statement_omp *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	  }
+	  break;
+	case GSS_BIND:
+	  {
+	     gimple_statement_bind *stmt
+	      = static_cast <gimple_statement_bind *> (x);
+	    gt_ggc_m_9tree_node (stmt->vars);
+	    gt_ggc_m_9tree_node (stmt->block);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	  }
+	  break;
+	case GSS_CATCH:
+	  {
+	    gimple_statement_catch *stmt
+	      = static_cast <gimple_statement_catch *> (x);
+	    gt_ggc_m_9tree_node (stmt->types);
+	    gt_ggc_mx_gimple_statement_base (stmt->handler);
+	  }
+	  break;
+	case GSS_EH_FILTER:
+	  {
+	    gimple_statement_eh_filter *stmt
+	      = static_cast <gimple_statement_eh_filter *> (x);
+	    gt_ggc_m_9tree_node (stmt->types);
+	    gt_ggc_mx_gimple_statement_base (stmt->failure);
+	  }
+	  break;
+	case GSS_EH_MNT:
+	  {
+	    gimple_statement_eh_mnt *stmt
+	      = static_cast <gimple_statement_eh_mnt *> (x);
+	    gt_ggc_m_9tree_node (stmt->fndecl);
+	  }
+	  break;
+	case GSS_EH_ELSE:
+	  {
+	    gimple_statement_eh_else*stmt
+	      = static_cast <gimple_statement_eh_else *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->n_body);
+	    gt_ggc_mx_gimple_statement_base (stmt->e_body);
+	  }
+	  break;
+	case GSS_PHI:
+	  {
+	    gimple_statement_phi *stmt
+	      = static_cast <gimple_statement_phi *> (x);
+	    size_t num = (size_t)(stmt->nargs);
+	    gt_ggc_m_9tree_node (stmt->result);
+	    for (size_t i = 0; i != num; i++)
+	      gt_ggc_m_9tree_node (stmt->args[i].def);
+	  }
+	  break;
+	case GSS_EH_CTRL:
+	  break;
+	case GSS_TRY:
+	  {
+	    gimple_statement_try *stmt
+	      = static_cast <gimple_statement_try *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->eval);
+	    gt_ggc_mx_gimple_statement_base (stmt->cleanup);
+	  }
+	  break;
+	case GSS_WCE:
+	  {
+	    gimple_statement_wce *stmt
+	      = static_cast <gimple_statement_wce *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->cleanup);
+	  }
+	  break;
+	case GSS_ASM:
+	  {
+	    gimple_statement_asm *stmt
+	      = static_cast <gimple_statement_asm *> (x);
+	    size_t num = (size_t)(stmt->num_ops);
+	    gt_ggc_m_S (stmt->string);
+	    for (size_t i = 0; i != num; i++)
+	      gt_ggc_m_9tree_node (stmt->op[i]);
+	  }
+	  break;
+	case GSS_OMP_CRITICAL:
+	  {
+	    gimple_statement_omp_critical *stmt
+	      = static_cast <gimple_statement_omp_critical *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	    gt_ggc_m_9tree_node (stmt->name);
+	  }
+	  break;
+	case GSS_OMP_FOR:
+	  {
+	    gimple_statement_omp_for *stmt
+	      = static_cast <gimple_statement_omp_for *> (x);
+	    size_t num = (size_t)(stmt->collapse);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	    gt_ggc_m_9tree_node (stmt->clauses);
+	    if (stmt->iter != NULL) {
+	      for (size_t i = 0; i != num; i++) {
+		gt_ggc_m_9tree_node (stmt->iter[i].index);
+		gt_ggc_m_9tree_node (stmt->iter[i].initial);
+		gt_ggc_m_9tree_node (stmt->iter[i].final);
+		gt_ggc_m_9tree_node (stmt->iter[i].incr);
+	      }
+	      ggc_mark (stmt->iter);
+	    }
+	    gt_ggc_mx_gimple_statement_base (stmt->pre_body);
+	  }
+	  break;
+	case GSS_OMP_PARALLEL:
+	  {
+	    gimple_statement_omp_parallel *stmt
+	      = static_cast <gimple_statement_omp_parallel *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	    gt_ggc_m_9tree_node (stmt->clauses);
+	    gt_ggc_m_9tree_node (stmt->child_fn);
+	    gt_ggc_m_9tree_node (stmt->data_arg);
+	  }
+	  break;
+	case GSS_OMP_TASK:
+	  {
+	    gimple_statement_omp_task *stmt
+	      = static_cast <gimple_statement_omp_task *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	    gt_ggc_m_9tree_node (stmt->clauses);
+	    gt_ggc_m_9tree_node (stmt->child_fn);
+	    gt_ggc_m_9tree_node (stmt->data_arg);
+	    gt_ggc_m_9tree_node (stmt->copy_fn);
+	    gt_ggc_m_9tree_node (stmt->arg_size);
+	    gt_ggc_m_9tree_node (stmt->arg_align);
+	  }
+	  break;
+	case GSS_OMP_SECTIONS:
+	  {
+	    gimple_statement_omp_sections *stmt
+	      = static_cast <gimple_statement_omp_sections *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	    gt_ggc_m_9tree_node (stmt->clauses);
+	    gt_ggc_m_9tree_node (stmt->control);
+	  }
+	  break;
+	case GSS_OMP_SINGLE:
+	  {
+	    gimple_statement_omp_single *stmt
+	      = static_cast <gimple_statement_omp_single *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	    gt_ggc_m_9tree_node (stmt->clauses);
+	  }
+	  break;
+	case GSS_OMP_CONTINUE:
+	  {
+	    gimple_statement_omp_continue *stmt
+	      = static_cast <gimple_statement_omp_continue *> (x);
+	    gt_ggc_m_9tree_node (stmt->control_def);
+	    gt_ggc_m_9tree_node (stmt->control_use);
+	  }
+	  break;
+	case GSS_OMP_ATOMIC_LOAD:
+	  {
+	    gimple_statement_omp_atomic_load *stmt
+	      = static_cast <gimple_statement_omp_atomic_load *> (x);
+	    gt_ggc_m_9tree_node (stmt->rhs);
+	    gt_ggc_m_9tree_node (stmt->lhs);
+	  }
+	  break;
+	case GSS_OMP_ATOMIC_STORE:
+	  {
+	    gimple_statement_omp_atomic_store *stmt
+	      = static_cast <gimple_statement_omp_atomic_store *> (x);
+	    gt_ggc_m_9tree_node (stmt->val);
+	  }
+	  break;
+	case GSS_TRANSACTION:
+	  {
+	    gimple_statement_transaction *stmt
+	      = static_cast <gimple_statement_transaction *> (x);
+	    gt_ggc_mx_gimple_statement_base (stmt->body);
+	    gt_ggc_m_9tree_node (stmt->label);
+	  }
+	  break;
+	default:
+	  break;
+	}
+      x = x->next;
+    }
+}
+
+void
+gt_pch_nx (gimple gs ATTRIBUTE_UNUSED)
+{
+  /* gimple should not be present in PCH files.  */
+  gcc_unreachable ();
+}
+
+void
+gt_pch_nx (gimple gs ATTRIBUTE_UNUSED,
+	   gt_pointer_operator op ATTRIBUTE_UNUSED,
+	   void *cookie ATTRIBUTE_UNUSED)
+{
+  /* gimple should not be present in PCH files.  */
+  gcc_unreachable ();
+}
+
+
 #include "gt-gimple.h"
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 0f6eb77..2e865e9 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -222,6 +222,12 @@ struct GTY((user)) gimple_statement_base {
   gimple GTY((skip)) prev;
 };
 
+/* GTY((user)) hooks for gimple, called once per-traversal.  */
+void gt_ggc_mx (gimple gs);
+void gt_pch_nx (gimple gs);
+void gt_pch_nx (gimple gs, gt_pointer_operator op, void *cookie);
+
+
 
 /* Base structure for tuples with operands.  */
 
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index af8685c..185c072 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -8291,7 +8291,6 @@ make_pass_warn_unused_result (gcc::context *ctxt)
 /* Garbage collection support for edge_def.  */
 
 extern void gt_ggc_mx (tree&);
-extern void gt_ggc_mx (gimple&);
 extern void gt_ggc_mx (rtx&);
 extern void gt_ggc_mx (basic_block&);
 
@@ -8302,7 +8301,7 @@ gt_ggc_mx (edge_def *e)
   gt_ggc_mx (e->src);
   gt_ggc_mx (e->dest);
   if (current_ir_type () == IR_GIMPLE)
-    gt_ggc_mx (e->insns.g);
+    gt_ggc_mx_gimple_statement_base (e->insns.g);
   else
     gt_ggc_mx (e->insns.r);
   gt_ggc_mx (block);
@@ -8311,7 +8310,6 @@ gt_ggc_mx (edge_def *e)
 /* PCH support for edge_def.  */
 
 extern void gt_pch_nx (tree&);
-extern void gt_pch_nx (gimple&);
 extern void gt_pch_nx (rtx&);
 extern void gt_pch_nx (basic_block&);
 
@@ -8322,7 +8320,7 @@ gt_pch_nx (edge_def *e)
   gt_pch_nx (e->src);
   gt_pch_nx (e->dest);
   if (current_ir_type () == IR_GIMPLE)
-    gt_pch_nx (e->insns.g);
+    gt_pch_nx_gimple_statement_base (e->insns.g);
   else
     gt_pch_nx (e->insns.r);
   gt_pch_nx (block);
-- 
1.7.11.7


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-31 10:53                 ` Richard Biener
  2013-08-31 13:34                   ` Basile Starynkevitch
@ 2013-08-31 14:04                   ` Diego Novillo
  1 sibling, 0 replies; 44+ messages in thread
From: Diego Novillo @ 2013-08-31 14:04 UTC (permalink / raw)
  To: Richard Biener
  Cc: Jakub Jelinek, Michael Matz, Gabriel Dos Reis, David Malcolm,
	gcc-patches

On Sat, Aug 31, 2013 at 5:57 AM, Richard Biener
<richard.guenther@gmail.com> wrote:

> What do you do during stage1?  Have a collector that never collects?

Yes.  That was the pebble in the shoe.  The cc1plus built for the
purposes of gengtype does not need to look at a lot of code, so
turning off collection may not be a big problem.  We also considered
using a retargetable parser like Clang, or even plugins.

All these approaches meant keeping GC.  Neither of us is very fond of
GC, so we did not explore these alternatives very deeply.


Diego.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 6/6] Add manual GTY hooks
  2013-08-30  8:12     ` Richard Biener
@ 2013-08-31 14:30       ` David Malcolm
  2013-08-31 19:52         ` Richard Biener
  0 siblings, 1 reply; 44+ messages in thread
From: David Malcolm @ 2013-08-31 14:30 UTC (permalink / raw)
  To: Richard Biener; +Cc: Steven Bosscher, Diego Novillo, GCC Patches

On Fri, 2013-08-30 at 10:09 +0200, Richard Biener wrote:
> On Thu, Aug 29, 2013 at 9:44 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> > On Thu, Aug 29, 2013 at 6:20 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> >>         * gimple.c (gt_ggc_mx (gimple)): New, as required by GTY((user)).
> >>         (gt_pch_nx (gimple)): Likewise.
> >
> > GIMPLE isn't supposed to end up in a PCH. Can you please make this
> > function simply call gcc_unreachable()?
> >
> > FWIW 1: I really think all these hand-written markers aren't a good
> > idea, we should really figure out a way to have automatic marker
> > function generators, something less complex than gengtype, of course.
> > But to have all these calls to the type-mangled marker functions
> > (gt_pch_n_9tree_node, etc.) is going to be a problem in the long term.
> >
> > It seems to me that the first step in all these conversions to
> > hand-written markers should be to make gengtype spit out the marker
> > functions *without* the type name mangling, i.e. all marker functions
> > should just be gt_ggc_mx(type) / gt_pch_nx(type).
> 
> Yes, the original idea was that gengtype would do that.  For things we like
> to optimize the GTY((user)) thing would tell it that we do provide the markers.
> Like when you want to look through a non-GCed intermediate object.  Or
> for things like GTY((chain)) stuff that doesn't really work if you have multiple
> chains (without clever GTY((skip))s...).
> 
> The lack of the unmangled overloads is annoying :/  IIRC Diego halfway completed
> the transition to unmangled overloads / specializations?

How would that work, and is that something that it would be productive
for me to work on?

Currently AIUI gtype-desc.h contains mangled macros and decls e.g.:
  extern void gt_ggc_mx_rtvec_def (void *);
  #define gt_ggc_m_9rtvec_def(X) do { \
    if (X != NULL) gt_ggc_mx_rtvec_def (X);\
    } while (0)

and the corresponding functions in gtype-desc.c contain:

void
gt_ggc_mx_rtvec_def (void *x_p)
{
  struct rtvec_def * const x = (struct rtvec_def *)x_p;
  if (ggc_test_and_set_mark (x))
    {
	/* visit fields of x, invoking the macros */
    }
}

Is the goal for the field-visiting code to all be able to simply do:
   gt_ggc_mx (field)
and have overloading resolve it all? (and handle e.g. chain_next etc
etc)

Presumably if this were implemented, then hand-written GTY functions
would be that much easier to maintain, and perhaps this gimple
conversion idea would be more acceptable?  (or, in other words, should I
work on this?)

Thanks
Dave

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 6/6] Add manual GTY hooks
  2013-08-31 14:30       ` David Malcolm
@ 2013-08-31 19:52         ` Richard Biener
  2013-09-04 15:40           ` David Malcolm
  0 siblings, 1 reply; 44+ messages in thread
From: Richard Biener @ 2013-08-31 19:52 UTC (permalink / raw)
  To: David Malcolm; +Cc: Steven Bosscher, Diego Novillo, GCC Patches

David Malcolm <dmalcolm@redhat.com> wrote:
>On Fri, 2013-08-30 at 10:09 +0200, Richard Biener wrote:
>> On Thu, Aug 29, 2013 at 9:44 PM, Steven Bosscher
><stevenb.gcc@gmail.com> wrote:
>> > On Thu, Aug 29, 2013 at 6:20 PM, David Malcolm
><dmalcolm@redhat.com> wrote:
>> >>         * gimple.c (gt_ggc_mx (gimple)): New, as required by
>GTY((user)).
>> >>         (gt_pch_nx (gimple)): Likewise.
>> >
>> > GIMPLE isn't supposed to end up in a PCH. Can you please make this
>> > function simply call gcc_unreachable()?
>> >
>> > FWIW 1: I really think all these hand-written markers aren't a good
>> > idea, we should really figure out a way to have automatic marker
>> > function generators, something less complex than gengtype, of
>course.
>> > But to have all these calls to the type-mangled marker functions
>> > (gt_pch_n_9tree_node, etc.) is going to be a problem in the long
>term.
>> >
>> > It seems to me that the first step in all these conversions to
>> > hand-written markers should be to make gengtype spit out the marker
>> > functions *without* the type name mangling, i.e. all marker
>functions
>> > should just be gt_ggc_mx(type) / gt_pch_nx(type).
>> 
>> Yes, the original idea was that gengtype would do that.  For things
>we like
>> to optimize the GTY((user)) thing would tell it that we do provide
>the markers.
>> Like when you want to look through a non-GCed intermediate object. 
>Or
>> for things like GTY((chain)) stuff that doesn't really work if you
>have multiple
>> chains (without clever GTY((skip))s...).
>> 
>> The lack of the unmangled overloads is annoying :/  IIRC Diego
>halfway completed
>> the transition to unmangled overloads / specializations?
>
>How would that work, and is that something that it would be productive
>for me to work on?
>
>Currently AIUI gtype-desc.h contains mangled macros and decls e.g.:
>  extern void gt_ggc_mx_rtvec_def (void *);
>  #define gt_ggc_m_9rtvec_def(X) do { \
>    if (X != NULL) gt_ggc_mx_rtvec_def (X);\
>    } while (0)
>
>and the corresponding functions in gtype-desc.c contain:
>
>void
>gt_ggc_mx_rtvec_def (void *x_p)
>{
>  struct rtvec_def * const x = (struct rtvec_def *)x_p;
>  if (ggc_test_and_set_mark (x))
>    {
>	/* visit fields of x, invoking the macros */
>    }
>}
>
>Is the goal for the field-visiting code to all be able to simply do:
>   gt_ggc_mx (field)

Yes. The advantage is that gengtype this way only needs to parse field names and not types which is a lot easier.

>and have overloading resolve it all? (and handle e.g. chain_next etc
>etc)

Yes. All bits that would require more complex parsing should instead be telled explicit via a GTY annotation.

>Presumably if this were implemented, then hand-written GTY functions
>would be that much easier to maintain, and perhaps this gimple
>conversion idea would be more acceptable?  (or, in other words, should
>I
>work on this?)

That would be very nice! IIRC Diego had issues with making all declarations visible to make this work. Diego?

Richard.

>Thanks
>Dave


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-30 19:38   ` David Malcolm
@ 2013-09-02 11:45     ` Michael Matz
  2013-09-04  1:08       ` David Malcolm
  2013-09-04  1:12       ` David Malcolm
  2013-09-02 12:35     ` Martin Jambor
  1 sibling, 2 replies; 44+ messages in thread
From: Michael Matz @ 2013-09-02 11:45 UTC (permalink / raw)
  To: David Malcolm; +Cc: gcc-patches

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2375 bytes --]

Hi,

On Fri, 30 Aug 2013, David Malcolm wrote:

> Here's the result of a pair of builds of r202029 without and with the
> patches, configured with --enable-checking=release, running "make", then
> stripping debuginfo [1]
> 
> So the overall sizes of such binaries are essentially unchanged.

Yep, cool.

> Any suggestions on what to compile to compare performance?  By 60 
> seconds, do you mean 60s for one TU, or do you mean a large build e.g. 
> the linux kernel?

For one TU, so as to not measure any significant execve, as or IO 
overhead.  Some people compile e.g preprocessed variants of some GCC 
source file, myself I'm measuring such speeds since some years with 
unchanged versions of the attached (big-code.c, several large functions 
with arithmetics and one-deep loops) which can be customized 
by hand to produce different largeness and kdecore.cc [1] (real world C++ 
library code from 2009).

You can customize big-code.c to match some compile time goal by 
commenting out some FUNC invocation for some types, or by changing the 
body size of the functions by fiddling in the FUNC macro itself to invoke 
more of the L2's or even L3's, but that's really slow.

> > And the manual GTY markers are so not maintainable in the long run, 
> > gengtype or something else really needs to be taught to create them 
> > automatically.
> 
> Apart from the GTY aspect, how do people feel about the patch series?

Generally I like it, the parts I don't like are common to all the 
C++ification patches [2].  The manual GTY parts give me the creeps, but I 
think I expressed that already :)


Ciao,
Michael.
[1] http://frakked.de/~matz/kdecore.cc.gz 
[2] In this series it's the is_a/has_a/dyn_cast uglification,
    -  gcc_gimple_checking_assert (gimple_has_mem_ops (g));
    -  g->gsmembase.vuse = vuse;
    +  gimple_statement_with_memory_ops *mem_ops_stmt =
    +    as_a <gimple_statement_with_memory_ops> (g);
    +  mem_ops_stmt->vuse = vuse;

    I can't really say I find this shorter, easier to read, more
    expressive or even safer than what was there before.  And the 
    repetition for adding the helpers for const and non-const types
    all the time doesn't make it better.

    (Btw: something for your helper script: it's customary to put the
    '=' to the next line; I know there is precedent for your variant,
    but '=' on next line is prevalent)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: TEXT/X-C++SRC; NAME=big-code.c, Size: 1576 bytes --]

extern int get_i(void);
typedef signed char schar;
typedef unsigned char uchar;
typedef unsigned short ushort;
typedef unsigned int uint;
typedef unsigned long ulong;
typedef long long llong;
typedef unsigned long long ullong;
typedef long double ldouble;

#define BODY(T) \
for (i = get_i(); i; i--) { \
  accum += do_1_ ## T(accum, x, y, z); \
  switch (get_i()) { \
    case 1: case 3: case 34: case 123: \
      if (z*x > y) \
        x = do_2_ ## T (x * y + z); \
      else \
        y = do_3_ ## T (x / y - z); \
      break; \
    case 6: case 43: case -2: \
      z = do_4_ ## T ((int)z); \
      break; \
  }\
  for (j = 1; j < get_i (); j++) { \
    a[i+j] = accum * b[i+j*get_i()] / c[j]; \
    b[j] = b[j] - a[j-1]*x; \
    x += b[j-1] + b[j+1]; \
    y *= c[j*i] += a[0] + b[0] * c[(int)x]; \
  } \
  for (j = 0; j < 8192; j++) { \
    a[j] = a[j] + b[j] * c[j]; \
  } \
}

#define L1(T) BODY(T) BODY(T) BODY(T) BODY(T) BODY(T) BODY(T) BODY(T) BODY(T)
#define L2(T) L1(T) L1(T) L1(T) L1(T) L1(T) L1(T) L1(T) L1(T)
#define L3(T) L2(T) L2(T) L2(T) L2(T) L2(T) L2(T) L2(T) L2(T)

#define FUNC(T) \
extern T do_1_ ## T(T, T, T, int); \
extern T do_2_ ## T(T); \
extern T do_3_ ## T(T); \
extern T do_4_ ## T(T); \
T func_ ## T (T x, T y, T z, T *a, T *b, T *c) \
{ \
  T accum = 0; \
  int i, j; \
  L2(T) \
  /*L2(T)*/ \
  return accum; \
}

FUNC(schar)
FUNC(uchar)
FUNC(short)
FUNC(ushort)
FUNC(int)
FUNC(uint)
FUNC(long)
FUNC(ulong)
FUNC(llong)
FUNC(ullong)
FUNC(float)
FUNC(double)
FUNC(ldouble)

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-08-30 19:38   ` David Malcolm
  2013-09-02 11:45     ` Michael Matz
@ 2013-09-02 12:35     ` Martin Jambor
  2013-09-04  1:20       ` David Malcolm
  1 sibling, 1 reply; 44+ messages in thread
From: Martin Jambor @ 2013-09-02 12:35 UTC (permalink / raw)
  To: David Malcolm; +Cc: gcc-patches

Hi,

On Fri, Aug 30, 2013 at 03:21:22PM -0400, David Malcolm wrote:
> Apart from the GTY aspect, how do people feel about the patch series?
> FWIW I have vague thoughts about doing something similar for tree -
> doing so *might* give an easier route to the type vs expression
> separation that Andrew spoke about at the Cauldron rearchitecture BoF.
> (I started looking at doing a similar C++-ification of rtx, but...
> gahhhhh)
> 

I like it but before you start looking at the biger things, could you
perhpas proceed with the symtab?  It has much fewer classes, will
probably affect private development of fewer people, the accessor
macros/functions of symtab are less developed so it will immediately
really make code nicer, Honza has approved it and I'm really looking
forward to it.  Also, perhaps it will show us at much saller scale
potential problems with the general scheme.

I'm only writing this because the development there seems a bit
stalled and it it a shame.  Of course, you ay want to simplify the
manual markings first.  I'd perfectly understand that.

Thanks,

Martin

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-09-02 11:45     ` Michael Matz
@ 2013-09-04  1:08       ` David Malcolm
  2013-09-04  1:12       ` David Malcolm
  1 sibling, 0 replies; 44+ messages in thread
From: David Malcolm @ 2013-09-04  1:08 UTC (permalink / raw)
  To: Michael Matz; +Cc: gcc-patches

On Mon, 2013-09-02 at 13:44 +0200, Michael Matz wrote:
> Hi,
> 
> On Fri, 30 Aug 2013, David Malcolm wrote:
> 
> > Here's the result of a pair of builds of r202029 without and with the
> > patches, configured with --enable-checking=release, running "make", then
> > stripping debuginfo [1]
> > 
> > So the overall sizes of such binaries are essentially unchanged.
> 
> Yep, cool.
> 
> > Any suggestions on what to compile to compare performance?  By 60 
> > seconds, do you mean 60s for one TU, or do you mean a large build e.g. 
> > the linux kernel?
> 
> For one TU, so as to not measure any significant execve, as or IO 
> overhead.  Some people compile e.g preprocessed variants of some GCC 
> source file, myself I'm measuring such speeds since some years with 
> unchanged versions of the attached (big-code.c, several large functions 
> with arithmetics and one-deep loops) which can be customized 
> by hand to produce different largeness and kdecore.cc [1] (real world C++ 
> library code from 2009).
> 
> You can customize big-code.c to match some compile time goal by 
> commenting out some FUNC invocation for some types, or by changing the 
> body size of the functions by fiddling in the FUNC macro itself to invoke 
> more of the L2's or even L3's, but that's really slow.

Thanks.  Everyone seems to have their own benchmarking test files - I'd
like to gather those that I can into a common repository.  What license
is big-code.c under?  Presumably kdecore.cc is from a compile of KDE and
thus covered by the license of that (plus of all the headers that were
included, I guess).

I scripted compilation of each of big-code.c and kdecore.cc at -O3,
using the cc1plus from each of the builds described above, 10 times
(i.e. the build of r202029 without (control) and with the patches
(experiment), both configured with --enable-checking=release, running
"make", then stripping debuginfo).

I wrote a script to parse the "TOTAL" timevar data from the logs,
extracting the total timings for user, sys, and wallclock, and the ggc
memory total. I used some code from Python's benchmarking suite to
determine whether the observed difference is "significant", and to draw
comparative graphs of the data (actually, to generate URLs to Google's
Chart API).

There were no significant differences for these data between the cc1plus
with the patch and the cc1plus without.  The script uses Student's
two-tailed T test on the benchmark results at the 95% confidence level
to determine significance, but it also has a cutoff in which when the
averages are within 1% of each other they are treated as
"insignificant".  If I remove the cutoff, then the kdecore.cc wallclock
time is reported as *faster* with the patch (t=2.41) - but by a tiny
amount.

FWIW this benchmarking code can be seen at:
https://github.com/davidmalcolm/gcc-build/commit/4581f080be2ed92179bfc1bc12e0ba9e923adaae

Data and URLs to graphs follow:

kdecore.cc -O3: usr
      control: [63.01, 63.11, 62.9, 63.14, 63.22, 63.0, 63.21, 63.03, 63.07, 63.09]
   experiment: [62.94, 63.05, 62.99, 63.03, 63.25, 62.91, 63.14, 62.96, 63.0, 63.04]
Min: 62.900000 -> 62.910000: 1.00x slower
Avg: 63.078000 -> 63.031000: 1.00x faster
Not significant
Stddev: 0.09852 -> 0.10049: 1.0200x larger
Timeline: http://goo.gl/8P9mFr

kdecore.cc -O3: sys
      control: [5.85, 5.74, 5.91, 5.79, 5.79, 5.85, 5.81, 5.9, 5.83, 5.8]
   experiment: [5.8, 5.81, 5.88, 5.88, 5.77, 5.87, 5.69, 5.8, 5.75, 5.84]
Min: 5.740000 -> 5.690000: 1.01x faster
Avg: 5.827000 -> 5.809000: 1.00x faster
Not significant
Stddev: 0.05229 -> 0.06154: 1.1769x larger
Timeline: http://goo.gl/xfWoUL

kdecore.cc -O3: wall
      control: [69.01, 69.01, 68.94, 69.03, 69.13, 68.99, 69.18, 69.07, 69.01, 69.01]
   experiment: [68.88, 68.97, 68.99, 69.03, 69.12, 68.88, 68.96, 68.86, 68.85, 68.99]
Min: 68.940000 -> 68.850000: 1.00x faster
Avg: 69.038000 -> 68.953000: 1.00x faster
Not significant
Stddev: 0.07052 -> 0.08616: 1.2217x larger
Timeline: http://goo.gl/Pf9BL8

kdecore.cc -O3: ggc
      control: [988589.0, 988591.0, 988585.0, 988582.0, 988584.0, 988589.0, 988585.0, 988582.0, 988584.0, 988586.0]
   experiment: [988593.0, 988585.0, 988589.0, 988585.0, 988591.0, 988588.0, 988591.0, 988585.0, 988589.0, 988590.0]
Mem max: 988591.000 -> 988593.000: 1.0000x larger
Usage over time: http://goo.gl/4pxB8Y

big-code.c -O3: usr
      control: [60.06, 60.31, 60.33, 60.29, 60.28, 60.26, 60.41, 60.28, 60.29, 60.29]
   experiment: [59.9, 60.14, 60.27, 60.33, 60.22, 60.33, 60.31, 60.33, 60.16, 60.26]
Min: 60.060000 -> 59.900000: 1.00x faster
Avg: 60.280000 -> 60.225000: 1.00x faster
Not significant
Stddev: 0.08781 -> 0.13360: 1.5215x larger
Timeline: http://goo.gl/gkFbgi

big-code.c -O3: sys
      control: [1.32, 1.37, 1.34, 1.41, 1.35, 1.34, 1.29, 1.36, 1.33, 1.35]
   experiment: [1.38, 1.35, 1.32, 1.32, 1.33, 1.33, 1.34, 1.32, 1.41, 1.35]
Min: 1.290000 -> 1.320000: 1.02x slower
Avg: 1.346000 -> 1.345000: 1.00x faster
Not significant
Stddev: 0.03169 -> 0.02953: 1.0731x smaller
Timeline: http://goo.gl/DPijei

big-code.c -O3: wall
      control: [61.48, 61.78, 61.78, 61.8, 61.74, 61.71, 61.81, 61.73, 61.73, 61.74]
   experiment: [61.39, 61.58, 61.7, 61.75, 61.65, 61.76, 61.76, 61.76, 61.67, 61.72]
Min: 61.480000 -> 61.390000: 1.00x faster
Avg: 61.730000 -> 61.674000: 1.00x faster
Not significant
Stddev: 0.09393 -> 0.11587: 1.2337x larger
Timeline: http://goo.gl/0WcMNl

big-code.c -O3: ggc
      control: [575294.0, 575294.0, 575294.0, 575294.0, 575294.0, 575294.0, 575294.0, 575294.0, 575294.0, 575294.0]
   experiment: [575294.0, 575294.0, 575294.0, 575294.0, 575294.0, 575294.0, 575294.0, 575294.0, 575294.0, 575294.0]
Mem max: 575294.000 -> 575294.000: no change
Usage over time: http://goo.gl/jO0Enh


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-09-02 11:45     ` Michael Matz
  2013-09-04  1:08       ` David Malcolm
@ 2013-09-04  1:12       ` David Malcolm
  2013-09-04 13:22         ` Michael Matz
  1 sibling, 1 reply; 44+ messages in thread
From: David Malcolm @ 2013-09-04  1:12 UTC (permalink / raw)
  To: Michael Matz; +Cc: gcc-patches

On Mon, 2013-09-02 at 13:44 +0200, Michael Matz wrote:
> Hi,
> 
> On Fri, 30 Aug 2013, David Malcolm wrote:
> 
[...]
> > > And the manual GTY markers are so not maintainable in the long run, 
> > > gengtype or something else really needs to be taught to create them 
> > > automatically.
> > 
> > Apart from the GTY aspect, how do people feel about the patch series?
> 
> Generally I like it, the parts I don't like are common to all the 
> C++ification patches [2].  The manual GTY parts give me the creeps, but I 
> think I expressed that already :)
> 
> 
> Ciao,
> Michael.
> [1] http://frakked.de/~matz/kdecore.cc.gz 
> [2] In this series it's the is_a/has_a/dyn_cast uglification,
>     -  gcc_gimple_checking_assert (gimple_has_mem_ops (g));
>     -  g->gsmembase.vuse = vuse;
>     +  gimple_statement_with_memory_ops *mem_ops_stmt =
>     +    as_a <gimple_statement_with_memory_ops> (g);
>     +  mem_ops_stmt->vuse = vuse;
> 
>     I can't really say I find this shorter, easier to read, more
>     expressive or even safer than what was there before.  And the 
>     repetition for adding the helpers for const and non-const types
>     all the time doesn't make it better.
Part of this is the verbose struct names.  I mentioned getting rid of
the "_statement" part of the typenames, I think I'll do that.

The other part is that the accessor functions become redundant, and that
you'd be able to do the cast once, and then use all of the various
fields of a gimple_whatever, bypassing the getters/setters.

>     (Btw: something for your helper script: it's customary to put the
>     '=' to the next line; I know there is precedent for your variant,
>     but '=' on next line is prevalent)
Thanks; I'll fix that.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-09-02 12:35     ` Martin Jambor
@ 2013-09-04  1:20       ` David Malcolm
  2013-09-04 14:09         ` Jan Hubicka
  0 siblings, 1 reply; 44+ messages in thread
From: David Malcolm @ 2013-09-04  1:20 UTC (permalink / raw)
  To: Martin Jambor, Jan Hubicka; +Cc: gcc-patches

On Mon, 2013-09-02 at 14:35 +0200, Martin Jambor wrote:
> Hi,
> 
> On Fri, Aug 30, 2013 at 03:21:22PM -0400, David Malcolm wrote:
> > Apart from the GTY aspect, how do people feel about the patch series?
> > FWIW I have vague thoughts about doing something similar for tree -
> > doing so *might* give an easier route to the type vs expression
> > separation that Andrew spoke about at the Cauldron rearchitecture BoF.
> > (I started looking at doing a similar C++-ification of rtx, but...
> > gahhhhh)
> > 
> 
> I like it but before you start looking at the biger things, could you
> perhpas proceed with the symtab?  It has much fewer classes, will
> probably affect private development of fewer people, the accessor
> macros/functions of symtab are less developed so it will immediately
> really make code nicer, Honza has approved it and I'm really looking
> forward to it.  Also, perhaps it will show us at much saller scale
> potential problems with the general scheme.

Sorry about the delay.  I wasn't aware that it had been approved; there
seemed to be a lot of caveats and objections in that thread.  On
re-reading, http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01147.html
could be seen as approval, but I guess I was making a conservative
reading of that post.  I hope to refresh the patches and
reboostrap/repost them at some point this week.

> I'm only writing this because the development there seems a bit
> stalled and it it a shame.  Of course, you ay want to simplify the
> manual markings first.  I'd perfectly understand that.

I've been poking at gengtype (and running benchmarks; see other post),
which would affect the symtab patch, though it's something of a
quagmire...

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-09-04  1:12       ` David Malcolm
@ 2013-09-04 13:22         ` Michael Matz
  0 siblings, 0 replies; 44+ messages in thread
From: Michael Matz @ 2013-09-04 13:22 UTC (permalink / raw)
  To: David Malcolm; +Cc: gcc-patches

Hi,

On Tue, 3 Sep 2013, David Malcolm wrote:

> >     I can't really say I find this shorter, easier to read, more
> >     expressive or even safer than what was there before.  And the 
> >     repetition for adding the helpers for const and non-const types
> >     all the time doesn't make it better.
> Part of this is the verbose struct names.  I mentioned getting rid of
> the "_statement" part of the typenames, I think I'll do that.

Yep, IMO makes sense.

> The other part is that the accessor functions become redundant, and that 
> you'd be able to do the cast once, and then use all of the various 
> fields of a gimple_whatever, bypassing the getters/setters.

Well, you can do that today with unions too, it's just not prevalent 
style; but you could do:

if (gimple_has_mem_ops (g))
  {
    struct gimple_statement_with_memory_ops_base *gm = &g->gsmembase;
    gm->vuse = ...;
  }

Obviously the naming of the struct here also is a bit excessive.  Using 
accessors has one large advantage over accessing the fields directly, you 
can change the semantics of them.  One reason why the above style isn't 
used.  But if that is true one of your reasons doing the change (downcast 
once, access fields directly, obsoleting the accessors) becomes moot, 
because we don't _want_ to access the fields directly.  That is, until you 
add member functions doing the accesses, which has its own problems (of 
stylistic nature, because then we'd have a very weird and clumsy mix in 
GCC sources of some data structures having member function accessors and 
others using the traditional C style).

Hmm.  After some nights sleeping over this, I'm oscillating again between 
not liking the change and being indifferent; as in, I do see some of the 
advantages you mentioned but I don't regard them outweighing the 
disadvantages.


Ciao,
Michael.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-09-04  1:20       ` David Malcolm
@ 2013-09-04 14:09         ` Jan Hubicka
  2013-09-04 14:49           ` Mike Stump
  0 siblings, 1 reply; 44+ messages in thread
From: Jan Hubicka @ 2013-09-04 14:09 UTC (permalink / raw)
  To: David Malcolm; +Cc: Martin Jambor, Jan Hubicka, gcc-patches

> On Mon, 2013-09-02 at 14:35 +0200, Martin Jambor wrote:
> > Hi,
> > 
> > On Fri, Aug 30, 2013 at 03:21:22PM -0400, David Malcolm wrote:
> > > Apart from the GTY aspect, how do people feel about the patch series?
> > > FWIW I have vague thoughts about doing something similar for tree -
> > > doing so *might* give an easier route to the type vs expression
> > > separation that Andrew spoke about at the Cauldron rearchitecture BoF.
> > > (I started looking at doing a similar C++-ification of rtx, but...
> > > gahhhhh)
> > > 
> > 
> > I like it but before you start looking at the biger things, could you
> > perhpas proceed with the symtab?  It has much fewer classes, will
> > probably affect private development of fewer people, the accessor
> > macros/functions of symtab are less developed so it will immediately
> > really make code nicer, Honza has approved it and I'm really looking
> > forward to it.  Also, perhaps it will show us at much saller scale
> > potential problems with the general scheme.
> 
> Sorry about the delay.  I wasn't aware that it had been approved; there
> seemed to be a lot of caveats and objections in that thread.  On
> re-reading, http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01147.html
> could be seen as approval, but I guess I was making a conservative
> reading of that post.  I hope to refresh the patches and
> reboostrap/repost them at some point this week.

I think the patch was generally in right direction.  I would welcome
if you got rid of symtab_node_base (and made symtab_node the basetype)
and possibly also did the suggested grand renaming.  But the second
can be probably done incrementally.
> 
> > I'm only writing this because the development there seems a bit
> > stalled and it it a shame.  Of course, you ay want to simplify the
> > manual markings first.  I'd perfectly understand that.
> 
> I've been poking at gengtype (and running benchmarks; see other post),
> which would affect the symtab patch, though it's something of a
> quagmire...

Making gengtype to generate ggc_mark for each type would make hand writting
easier - you can use C++ overloading instead of trying to guess the funny
names gengtype uses right now.
But that is independent of this change.  I am slowly getting used to the
world of hand written gengtype markings.

Honza

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy
  2013-09-04 14:09         ` Jan Hubicka
@ 2013-09-04 14:49           ` Mike Stump
  0 siblings, 0 replies; 44+ messages in thread
From: Mike Stump @ 2013-09-04 14:49 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: David Malcolm, Martin Jambor, gcc-patches

On Sep 4, 2013, at 7:09 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Making gengtype to generate ggc_mark for each type would make hand writting
> easier - you can use C++ overloading instead of trying to guess the funny
> names gengtype uses right now.
> But that is independent of this change.  I am slowly getting used to the
> world of hand written gengtype markings.

I'd rather plugin generated code that applies general rules to the transitive closure of all types needed.  The markings would be for exceptions to those rules, which should be very, very rare.  The load for someone writing and maintaining code is then reduced and the fear of GTY doesn't play a role in activities such as refactoring, as there would be nothing to do.  I know it, and I have a healthy fear of it.  :-)

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 6/6] Add manual GTY hooks
  2013-08-31 19:52         ` Richard Biener
@ 2013-09-04 15:40           ` David Malcolm
  2013-09-05  9:08             ` Richard Biener
  2013-09-05 10:49             ` Diego Novillo
  0 siblings, 2 replies; 44+ messages in thread
From: David Malcolm @ 2013-09-04 15:40 UTC (permalink / raw)
  To: Richard Biener; +Cc: Steven Bosscher, Diego Novillo, GCC Patches

On Sat, 2013-08-31 at 19:27 +0200, Richard Biener wrote:
> David Malcolm <dmalcolm@redhat.com> wrote:
> >On Fri, 2013-08-30 at 10:09 +0200, Richard Biener wrote:
> >> On Thu, Aug 29, 2013 at 9:44 PM, Steven Bosscher
> ><stevenb.gcc@gmail.com> wrote:
> >> > On Thu, Aug 29, 2013 at 6:20 PM, David Malcolm
> ><dmalcolm@redhat.com> wrote:
> >> >>         * gimple.c (gt_ggc_mx (gimple)): New, as required by
> >GTY((user)).
> >> >>         (gt_pch_nx (gimple)): Likewise.
> >> >
> >> > GIMPLE isn't supposed to end up in a PCH. Can you please make this
> >> > function simply call gcc_unreachable()?
> >> >
> >> > FWIW 1: I really think all these hand-written markers aren't a good
> >> > idea, we should really figure out a way to have automatic marker
> >> > function generators, something less complex than gengtype, of
> >course.
> >> > But to have all these calls to the type-mangled marker functions
> >> > (gt_pch_n_9tree_node, etc.) is going to be a problem in the long
> >term.
> >> >
> >> > It seems to me that the first step in all these conversions to
> >> > hand-written markers should be to make gengtype spit out the marker
> >> > functions *without* the type name mangling, i.e. all marker
> >functions
> >> > should just be gt_ggc_mx(type) / gt_pch_nx(type).
> >> 
> >> Yes, the original idea was that gengtype would do that.  For things
> >we like
> >> to optimize the GTY((user)) thing would tell it that we do provide
> >the markers.
> >> Like when you want to look through a non-GCed intermediate object. 
> >Or
> >> for things like GTY((chain)) stuff that doesn't really work if you
> >have multiple
> >> chains (without clever GTY((skip))s...).
> >> 
> >> The lack of the unmangled overloads is annoying :/  IIRC Diego
> >halfway completed
> >> the transition to unmangled overloads / specializations?
> >
> >How would that work, and is that something that it would be productive
> >for me to work on?
> >
> >Currently AIUI gtype-desc.h contains mangled macros and decls e.g.:
> >  extern void gt_ggc_mx_rtvec_def (void *);
> >  #define gt_ggc_m_9rtvec_def(X) do { \
> >    if (X != NULL) gt_ggc_mx_rtvec_def (X);\
> >    } while (0)
> >
> >and the corresponding functions in gtype-desc.c contain:
> >
> >void
> >gt_ggc_mx_rtvec_def (void *x_p)
> >{
> >  struct rtvec_def * const x = (struct rtvec_def *)x_p;
> >  if (ggc_test_and_set_mark (x))
> >    {
> >	/* visit fields of x, invoking the macros */
> >    }
> >}
> >
> >Is the goal for the field-visiting code to all be able to simply do:
> >   gt_ggc_mx (field)
> 
> Yes. The advantage is that gengtype this way only needs to parse field names and not types which is a lot easier.
> 
> >and have overloading resolve it all? (and handle e.g. chain_next etc
> >etc)
> 
> Yes. All bits that would require more complex parsing should instead be telled explicit via a GTY annotation.
> 
> >Presumably if this were implemented, then hand-written GTY functions
> >would be that much easier to maintain, and perhaps this gimple
> >conversion idea would be more acceptable?  (or, in other words, should
> >I
> >work on this?)
> 
> That would be very nice! IIRC Diego had issues with making all declarations visible to make this work. Diego?

An update: I've been trying this, but I'm running into what may be a
blocker: types need to be visible in the declaration of the marker
function.  Doing this in gtype-desc.h runs into a tangle of dependency
issues.  For example:

  ./gtype-desc.h:2842:28: error: ‘ivarref_entry’ was not declared in this scope
  extern void gt_ggc_mx (vec<ivarref_entry,va_gc> *p);
                      ^
In the extreme case, the underlying types in question might be only
declared in one specific .c file.  In other cases, there's likely to be
some set of header files needed to get at the types.  Also, vec.h uses
ggc.h, which uses gtype-desc.h, and so we have a loop...

AIUI, the mangled names of types in the function names are doing the job
of hiding all the types in gtype-desc.h, so that they are only needed in
the various gt-*.h files and in gtype-desc.c.  By changing things so
that the marker function declarations refer to the types directly, all
types need to be known to users of such functions.  I'm not sure that
that's possible - or, at least, not if the declarations are in a shared
header file.

Perhaps it could work with some scheme in which the types are
partitioned by .h/.c file, and copies of forward declarations are added
only as-needed to the appropriate gt-*.h/gtype-desc.c files?

I'm trying this now.

BTW, what is the etymology of "gt_ggc_mx" and "gt_pch_nx", and how do
people pronounce them? (I call them the "object-marking" and
"object-noting" hooks" fwiw).  As far as I can see, the "m" and and the
"n" come from "mark" and "note"; it's not clear to me where the trailing
"x" came from.

Dave

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 6/6] Add manual GTY hooks
  2013-09-04 15:40           ` David Malcolm
@ 2013-09-05  9:08             ` Richard Biener
  2013-09-05 10:49             ` Diego Novillo
  1 sibling, 0 replies; 44+ messages in thread
From: Richard Biener @ 2013-09-05  9:08 UTC (permalink / raw)
  To: David Malcolm; +Cc: Steven Bosscher, Diego Novillo, GCC Patches

On Wed, Sep 4, 2013 at 5:40 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Sat, 2013-08-31 at 19:27 +0200, Richard Biener wrote:
>> David Malcolm <dmalcolm@redhat.com> wrote:
>> >On Fri, 2013-08-30 at 10:09 +0200, Richard Biener wrote:
>> >> On Thu, Aug 29, 2013 at 9:44 PM, Steven Bosscher
>> ><stevenb.gcc@gmail.com> wrote:
>> >> > On Thu, Aug 29, 2013 at 6:20 PM, David Malcolm
>> ><dmalcolm@redhat.com> wrote:
>> >> >>         * gimple.c (gt_ggc_mx (gimple)): New, as required by
>> >GTY((user)).
>> >> >>         (gt_pch_nx (gimple)): Likewise.
>> >> >
>> >> > GIMPLE isn't supposed to end up in a PCH. Can you please make this
>> >> > function simply call gcc_unreachable()?
>> >> >
>> >> > FWIW 1: I really think all these hand-written markers aren't a good
>> >> > idea, we should really figure out a way to have automatic marker
>> >> > function generators, something less complex than gengtype, of
>> >course.
>> >> > But to have all these calls to the type-mangled marker functions
>> >> > (gt_pch_n_9tree_node, etc.) is going to be a problem in the long
>> >term.
>> >> >
>> >> > It seems to me that the first step in all these conversions to
>> >> > hand-written markers should be to make gengtype spit out the marker
>> >> > functions *without* the type name mangling, i.e. all marker
>> >functions
>> >> > should just be gt_ggc_mx(type) / gt_pch_nx(type).
>> >>
>> >> Yes, the original idea was that gengtype would do that.  For things
>> >we like
>> >> to optimize the GTY((user)) thing would tell it that we do provide
>> >the markers.
>> >> Like when you want to look through a non-GCed intermediate object.
>> >Or
>> >> for things like GTY((chain)) stuff that doesn't really work if you
>> >have multiple
>> >> chains (without clever GTY((skip))s...).
>> >>
>> >> The lack of the unmangled overloads is annoying :/  IIRC Diego
>> >halfway completed
>> >> the transition to unmangled overloads / specializations?
>> >
>> >How would that work, and is that something that it would be productive
>> >for me to work on?
>> >
>> >Currently AIUI gtype-desc.h contains mangled macros and decls e.g.:
>> >  extern void gt_ggc_mx_rtvec_def (void *);
>> >  #define gt_ggc_m_9rtvec_def(X) do { \
>> >    if (X != NULL) gt_ggc_mx_rtvec_def (X);\
>> >    } while (0)
>> >
>> >and the corresponding functions in gtype-desc.c contain:
>> >
>> >void
>> >gt_ggc_mx_rtvec_def (void *x_p)
>> >{
>> >  struct rtvec_def * const x = (struct rtvec_def *)x_p;
>> >  if (ggc_test_and_set_mark (x))
>> >    {
>> >     /* visit fields of x, invoking the macros */
>> >    }
>> >}
>> >
>> >Is the goal for the field-visiting code to all be able to simply do:
>> >   gt_ggc_mx (field)
>>
>> Yes. The advantage is that gengtype this way only needs to parse field names and not types which is a lot easier.
>>
>> >and have overloading resolve it all? (and handle e.g. chain_next etc
>> >etc)
>>
>> Yes. All bits that would require more complex parsing should instead be telled explicit via a GTY annotation.
>>
>> >Presumably if this were implemented, then hand-written GTY functions
>> >would be that much easier to maintain, and perhaps this gimple
>> >conversion idea would be more acceptable?  (or, in other words, should
>> >I
>> >work on this?)
>>
>> That would be very nice! IIRC Diego had issues with making all declarations visible to make this work. Diego?
>
> An update: I've been trying this, but I'm running into what may be a
> blocker: types need to be visible in the declaration of the marker
> function.  Doing this in gtype-desc.h runs into a tangle of dependency
> issues.  For example:
>
>   ./gtype-desc.h:2842:28: error: ‘ivarref_entry’ was not declared in this scope
>   extern void gt_ggc_mx (vec<ivarref_entry,va_gc> *p);
>                       ^
> In the extreme case, the underlying types in question might be only
> declared in one specific .c file.  In other cases, there's likely to be
> some set of header files needed to get at the types.  Also, vec.h uses
> ggc.h, which uses gtype-desc.h, and so we have a loop...
>
> AIUI, the mangled names of types in the function names are doing the job
> of hiding all the types in gtype-desc.h, so that they are only needed in
> the various gt-*.h files and in gtype-desc.c.  By changing things so
> that the marker function declarations refer to the types directly, all
> types need to be known to users of such functions.  I'm not sure that
> that's possible - or, at least, not if the declarations are in a shared
> header file.

I think that's the issue Diego ran into.  The theory with the idea was
that at the point of the struct declaration we want to generate the
marker for all those types are properly declared so their marking
prototypes would work.  Which means that the marker declarations
should go alongside with struct definitions.

One idea would be to simply require explicit declarations of them
after their structs declaration.

> Perhaps it could work with some scheme in which the types are
> partitioned by .h/.c file, and copies of forward declarations are added
> only as-needed to the appropriate gt-*.h/gtype-desc.c files?

Not sure if forward declarations would work - at least autogenerated ones.
That would require gengtype parsing field types again.

Richard.

> I'm trying this now.
>
> BTW, what is the etymology of "gt_ggc_mx" and "gt_pch_nx", and how do
> people pronounce them? (I call them the "object-marking" and
> "object-noting" hooks" fwiw).  As far as I can see, the "m" and and the
> "n" come from "mark" and "note"; it's not clear to me where the trailing
> "x" came from.
>
> Dave
>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 6/6] Add manual GTY hooks
  2013-09-04 15:40           ` David Malcolm
  2013-09-05  9:08             ` Richard Biener
@ 2013-09-05 10:49             ` Diego Novillo
  1 sibling, 0 replies; 44+ messages in thread
From: Diego Novillo @ 2013-09-05 10:49 UTC (permalink / raw)
  To: David Malcolm; +Cc: Richard Biener, Steven Bosscher, GCC Patches

On Wed, Sep 4, 2013 at 11:40 AM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Sat, 2013-08-31 at 19:27 +0200, Richard Biener wrote:
>> David Malcolm <dmalcolm@redhat.com> wrote:
>> >On Fri, 2013-08-30 at 10:09 +0200, Richard Biener wrote:
>> >> On Thu, Aug 29, 2013 at 9:44 PM, Steven Bosscher
>> ><stevenb.gcc@gmail.com> wrote:
>> >> > On Thu, Aug 29, 2013 at 6:20 PM, David Malcolm
>> ><dmalcolm@redhat.com> wrote:
>> >> >>         * gimple.c (gt_ggc_mx (gimple)): New, as required by
>> >GTY((user)).
>> >> >>         (gt_pch_nx (gimple)): Likewise.
>> >> >
>> >> > GIMPLE isn't supposed to end up in a PCH. Can you please make this
>> >> > function simply call gcc_unreachable()?
>> >> >
>> >> > FWIW 1: I really think all these hand-written markers aren't a good
>> >> > idea, we should really figure out a way to have automatic marker
>> >> > function generators, something less complex than gengtype, of
>> >course.
>> >> > But to have all these calls to the type-mangled marker functions
>> >> > (gt_pch_n_9tree_node, etc.) is going to be a problem in the long
>> >term.
>> >> >
>> >> > It seems to me that the first step in all these conversions to
>> >> > hand-written markers should be to make gengtype spit out the marker
>> >> > functions *without* the type name mangling, i.e. all marker
>> >functions
>> >> > should just be gt_ggc_mx(type) / gt_pch_nx(type).
>> >>
>> >> Yes, the original idea was that gengtype would do that.  For things
>> >we like
>> >> to optimize the GTY((user)) thing would tell it that we do provide
>> >the markers.
>> >> Like when you want to look through a non-GCed intermediate object.
>> >Or
>> >> for things like GTY((chain)) stuff that doesn't really work if you
>> >have multiple
>> >> chains (without clever GTY((skip))s...).
>> >>
>> >> The lack of the unmangled overloads is annoying :/  IIRC Diego
>> >halfway completed
>> >> the transition to unmangled overloads / specializations?
>> >
>> >How would that work, and is that something that it would be productive
>> >for me to work on?
>> >
>> >Currently AIUI gtype-desc.h contains mangled macros and decls e.g.:
>> >  extern void gt_ggc_mx_rtvec_def (void *);
>> >  #define gt_ggc_m_9rtvec_def(X) do { \
>> >    if (X != NULL) gt_ggc_mx_rtvec_def (X);\
>> >    } while (0)
>> >
>> >and the corresponding functions in gtype-desc.c contain:
>> >
>> >void
>> >gt_ggc_mx_rtvec_def (void *x_p)
>> >{
>> >  struct rtvec_def * const x = (struct rtvec_def *)x_p;
>> >  if (ggc_test_and_set_mark (x))
>> >    {
>> >     /* visit fields of x, invoking the macros */
>> >    }
>> >}
>> >
>> >Is the goal for the field-visiting code to all be able to simply do:
>> >   gt_ggc_mx (field)
>>
>> Yes. The advantage is that gengtype this way only needs to parse field names and not types which is a lot easier.
>>
>> >and have overloading resolve it all? (and handle e.g. chain_next etc
>> >etc)
>>
>> Yes. All bits that would require more complex parsing should instead be telled explicit via a GTY annotation.
>>
>> >Presumably if this were implemented, then hand-written GTY functions
>> >would be that much easier to maintain, and perhaps this gimple
>> >conversion idea would be more acceptable?  (or, in other words, should
>> >I
>> >work on this?)
>>
>> That would be very nice! IIRC Diego had issues with making all declarations visible to make this work. Diego?

Yes.  I ran into the same problem that David just described.  When
trying to unmangle all the markers, you now require gengtype to have
actual visibility into all the headers, so that the compiler can see
the actual type definitions to resolve the overloads.

> An update: I've been trying this, but I'm running into what may be a
> blocker: types need to be visible in the declaration of the marker
> function.  Doing this in gtype-desc.h runs into a tangle of dependency
> issues.  For example:
>
>   ./gtype-desc.h:2842:28: error: ‘ivarref_entry’ was not declared in this scope
>   extern void gt_ggc_mx (vec<ivarref_entry,va_gc> *p);

This kind of issue.  Yes.

Perhaps this could be solved by not having a single header generated
for the markers.  Instead, markers are generated in separate header
files that are #included by the headers that have the actual type
definitions needed for the overloads to work.  Ditto for gtype-desc.c.

> BTW, what is the etymology of "gt_ggc_mx" and "gt_pch_nx", and how do
> people pronounce them? (I call them the "object-marking" and
> "object-noting" hooks" fwiw).  As far as I can see, the "m" and and the
> "n" come from "mark" and "note"; it's not clear to me where the trailing
> "x" came from.

That's how I think of them too.  I'm not sure what the 'x' denotes.


Diego.

^ permalink raw reply	[flat|nested] 44+ messages in thread

end of thread, other threads:[~2013-09-05 10:49 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-29 16:20 [PATCH 0/6] Convert gimple to a C++ class hierarchy David Malcolm
2013-08-29 16:20 ` [PATCH 2/6] Hand-written port of various accessors within gimple.h David Malcolm
2013-08-29 16:20 ` [PATCH 4/6] Implement is_a_helper <>::test specializations for various gimple types David Malcolm
2013-08-29 16:20 ` [PATCH 5/6] Port various places from union access to subclass access David Malcolm
2013-08-29 16:20 ` [PATCH 1/6] Convert gimple types from a union to a C++ class hierarchy David Malcolm
2013-08-29 16:20 ` [PATCH 3/6] Autogenerated conversion of gimple to C++ David Malcolm
2013-08-31 13:40   ` [PATCH 3/6 v2] " David Malcolm
2013-08-29 17:04 ` [PATCH 6/6] Add manual GTY hooks David Malcolm
2013-08-29 19:50   ` Steven Bosscher
2013-08-30  8:12     ` Richard Biener
2013-08-31 14:30       ` David Malcolm
2013-08-31 19:52         ` Richard Biener
2013-09-04 15:40           ` David Malcolm
2013-09-05  9:08             ` Richard Biener
2013-09-05 10:49             ` Diego Novillo
2013-08-30  8:09   ` Richard Biener
2013-08-31 13:48     ` [PATCH 6/6 v2] " David Malcolm
2013-08-30  8:43   ` [PATCH 6/6] " Richard Biener
2013-08-30 20:25     ` David Malcolm
2013-08-30 13:58 ` [PATCH 0/6] Convert gimple to a C++ class hierarchy Michael Matz
2013-08-30 14:02   ` Gabriel Dos Reis
2013-08-30 14:03     ` Jakub Jelinek
2013-08-30 14:26       ` Gabriel Dos Reis
2013-08-30 14:49         ` David Malcolm
2013-08-30 15:21         ` Michael Matz
2013-08-30 15:26           ` Gabriel Dos Reis
2013-08-30 15:31           ` Diego Novillo
2013-08-30 15:40             ` Jakub Jelinek
2013-08-30 15:50               ` Diego Novillo
2013-08-31 10:53                 ` Richard Biener
2013-08-31 13:34                   ` Basile Starynkevitch
2013-08-31 14:04                   ` Diego Novillo
2013-08-30 15:54               ` Gabriel Dos Reis
2013-08-31  9:56               ` Richard Biener
2013-08-30 19:38   ` David Malcolm
2013-09-02 11:45     ` Michael Matz
2013-09-04  1:08       ` David Malcolm
2013-09-04  1:12       ` David Malcolm
2013-09-04 13:22         ` Michael Matz
2013-09-02 12:35     ` Martin Jambor
2013-09-04  1:20       ` David Malcolm
2013-09-04 14:09         ` Jan Hubicka
2013-09-04 14:49           ` Mike Stump
2013-08-30 21:51 ` David Malcolm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).