Patchwork [3/3] Convert symtab, cgraph and varpool nodes into a real class hierarchy

login
register
mail settings
Submitter David Malcolm
Date Sept. 20, 2013, 2:05 p.m.
Message ID <1379685919-9437-4-git-send-email-dmalcolm@redhat.com>
Download mbox | patch
Permalink /patch/276479/
State New
Headers show

Comments

David Malcolm - Sept. 20, 2013, 2:05 p.m.
This patch is the handwritten part of the conversion of these types
to C++; it requires the followup patch, which is autogenerated.

It converts:
  struct symtab_node_base
to:
  class symtab_node_base

and converts:
  struct cgraph_node
to:
  struct cgraph_node : public symtab_node_base
and:
  struct varpool_node
to:
  class varpool_node : public symtab_node_base

dropping the symtab_node_def union.

This patch is essentially the same as older versions of this patch [1],
but drops the hand-written GTY markers and ugly dummy roots needed as
workarounds for bugs in how GTY((user)) works.

Instead, this version of the patch makes uses of the gengtype
inheritance support introduced earlier in the series.

The above types are actually:
  class GTY((desc ("%h.type"), tag ("SYMTAB_SYMBOL"))) symtab_node_base
i.e. "use type as the discriminator for this class hierarchy" and
"for the base class symtab_node_base, type==SYMTAB_SYMBOL"
along with:
  struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node : public symtab_node_base
and:
  class GTY((tag ("SYMTAB_VARIABLE"))) varpool_node : public symtab_node_base

The new gengtype is able to correctly generate marker functions for
the base class:

  void
  gt_ggc_mx_symtab_node_base (void *x_p);

  void
  gt_pch_nx_symtab_node_base (void *x_p);

  void
  gt_pch_p_16symtab_node_base (ATTRIBUTE_UNUSED void *this_obj,
	void *x_p,
	ATTRIBUTE_UNUSED gt_pointer_operator op,
	ATTRIBUTE_UNUSED void *cookie);

and uses these for any of the types within the inheritance hierarchy.

[1] http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00672.html

gcc/

	* cgraph.h (symtab_node_base): Convert to a class;
	add GTY((desc ("%h.type"), tag ("SYMTAB_SYMBOL"))).
	(cgraph_node): Inherit from symtab_node; add GTY option
	tag ("SYMTAB_FUNCTION").
	(varpool_node): Inherit from symtab_node; add GTY option
	tag ("SYMTAB_VARIABLE").
	(symtab_node_def): Remove.
	(is_a_helper <cgraph_node>::test (symtab_node_def *)): Convert to...
	(is_a_helper <cgraph_node>::test (symtab_node_base *)): ...this.
	(is_a_helper <varpool_node>::test (symtab_node_def *)): Convert to...
	(is_a_helper <varpool_node>::test (symtab_node_base *)): ...this.

	* ipa-ref.h (symtab_node_def): Drop.
	(symtab_node): Change underlying type from symtab_node_def to
	symtab_node_base.
	(const_symtab_node): Likwise.

	* is-a.h: Update examples in comment.

	* symtab.c (symtab_hash): Change symtab_node_def to symtab_node_base.
	(assembler_name_hash): Likewise.
---
 gcc/cgraph.h  | 38 +++++++++++---------------------------
 gcc/ipa-ref.h |  6 +++---
 gcc/is-a.h    |  6 +++---
 gcc/symtab.c  |  4 ++--
 4 files changed, 19 insertions(+), 35 deletions(-)
Jan Hubicka - Sept. 20, 2013, 3:33 p.m.
> This patch is the handwritten part of the conversion of these types
> to C++; it requires the followup patch, which is autogenerated.
> 
> It converts:
>   struct symtab_node_base
> to:
>   class symtab_node_base
> 
> and converts:
>   struct cgraph_node
> to:
>   struct cgraph_node : public symtab_node_base
> and:
>   struct varpool_node
> to:
>   class varpool_node : public symtab_node_base
> 
> dropping the symtab_node_def union.

Yep, incrementally we should continue with the grand renaming 
retiring symtab_node_base and getting things symmetric.
> 
> 	* cgraph.h (symtab_node_base): Convert to a class;
> 	add GTY((desc ("%h.type"), tag ("SYMTAB_SYMBOL"))).
> 	(cgraph_node): Inherit from symtab_node; add GTY option
> 	tag ("SYMTAB_FUNCTION").
> 	(varpool_node): Inherit from symtab_node; add GTY option
> 	tag ("SYMTAB_VARIABLE").
> 	(symtab_node_def): Remove.
> 	(is_a_helper <cgraph_node>::test (symtab_node_def *)): Convert to...
> 	(is_a_helper <cgraph_node>::test (symtab_node_base *)): ...this.
> 	(is_a_helper <varpool_node>::test (symtab_node_def *)): Convert to...
> 	(is_a_helper <varpool_node>::test (symtab_node_base *)): ...this.
> 
> 	* ipa-ref.h (symtab_node_def): Drop.
> 	(symtab_node): Change underlying type from symtab_node_def to
> 	symtab_node_base.
> 	(const_symtab_node): Likwise.
> 
> 	* is-a.h: Update examples in comment.
> 
> 	* symtab.c (symtab_hash): Change symtab_node_def to symtab_node_base.
> 	(assembler_name_hash): Likewise.

This patch is OK.  Thanks for working on this!

Honza

Patch

diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 50e8743..615067c 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -40,8 +40,9 @@  enum symtab_type
 
 /* Base of all entries in the symbol table.
    The symtab_node is inherited by cgraph and varpol nodes.  */
-struct GTY(()) symtab_node_base
+class GTY((desc ("%h.type"), tag ("SYMTAB_SYMBOL"))) symtab_node_base
 {
+public:
   /* Type of the symbol.  */
   ENUM_BITFIELD (symtab_type) type : 8;
 
@@ -252,25 +253,19 @@  struct GTY(()) cgraph_clone_info
 /* The cgraph data structure.
    Each function decl has assigned cgraph_node listing callees and callers.  */
 
-struct GTY(()) cgraph_node {
-  struct symtab_node_base symbol;
+struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node : public symtab_node_base {
+public:
   struct cgraph_edge *callees;
   struct cgraph_edge *callers;
   /* List of edges representing indirect calls with a yet undetermined
      callee.  */
   struct cgraph_edge *indirect_calls;
   /* For nested functions points to function the node is nested in.  */
-  struct cgraph_node *
-    GTY ((nested_ptr (union symtab_node_def, "(struct cgraph_node *)(%h)", "(symtab_node)%h")))
-    origin;
+  struct cgraph_node *origin;
   /* Points to first nested function, if any.  */
-  struct cgraph_node *
-    GTY ((nested_ptr (union symtab_node_def, "(struct cgraph_node *)(%h)", "(symtab_node)%h")))
-    nested;
+  struct cgraph_node *nested;
   /* Pointer to the next function with same origin, if any.  */
-  struct cgraph_node *
-    GTY ((nested_ptr (union symtab_node_def, "(struct cgraph_node *)(%h)", "(symtab_node)%h")))
-    next_nested;
+  struct cgraph_node *next_nested;
   /* Pointer to the next clone.  */
   struct cgraph_node *next_sibling_clone;
   struct cgraph_node *prev_sibling_clone;
@@ -518,9 +513,8 @@  typedef struct cgraph_edge *cgraph_edge_p;
 /* The varpool data structure.
    Each static variable decl has assigned varpool_node.  */
 
-struct GTY(()) varpool_node {
-  struct symtab_node_base symbol;
-
+class GTY((tag ("SYMTAB_VARIABLE"))) varpool_node : public symtab_node_base {
+public:
   /* Set when variable is scheduled to be assembled.  */
   unsigned output : 1;
 };
@@ -536,22 +530,12 @@  struct GTY(()) asm_node {
   int order;
 };
 
-/* Symbol table entry.  */
-union GTY((desc ("%h.symbol.type"), chain_next ("%h.symbol.next"),
-	   chain_prev ("%h.symbol.previous"))) symtab_node_def {
-  struct symtab_node_base GTY ((tag ("SYMTAB_SYMBOL"))) symbol;
-  /* To access the following fields,
-     use the use dyn_cast or as_a to obtain the concrete type.  */
-  struct cgraph_node GTY ((tag ("SYMTAB_FUNCTION"))) x_function;
-  struct varpool_node GTY ((tag ("SYMTAB_VARIABLE"))) x_variable;
-};
-
 /* Report whether or not THIS symtab node is a function, aka cgraph_node.  */
 
 template <>
 template <>
 inline bool
-is_a_helper <cgraph_node>::test (symtab_node_def *p)
+is_a_helper <cgraph_node>::test (symtab_node_base *p)
 {
   return p->symbol.type == SYMTAB_FUNCTION;
 }
@@ -561,7 +545,7 @@  is_a_helper <cgraph_node>::test (symtab_node_def *p)
 template <>
 template <>
 inline bool
-is_a_helper <varpool_node>::test (symtab_node_def *p)
+is_a_helper <varpool_node>::test (symtab_node_base *p)
 {
   return p->symbol.type == SYMTAB_VARIABLE;
 }
diff --git a/gcc/ipa-ref.h b/gcc/ipa-ref.h
index e0553bb..dc6e238 100644
--- a/gcc/ipa-ref.h
+++ b/gcc/ipa-ref.h
@@ -20,9 +20,9 @@  along with GCC; see the file COPYING3.  If not see
 
 struct cgraph_node;
 struct varpool_node;
-union symtab_node_def;
-typedef union symtab_node_def *symtab_node;
-typedef const union symtab_node_def *const_symtab_node;
+class symtab_node_base;
+typedef symtab_node_base *symtab_node;
+typedef const symtab_node_base *const_symtab_node;
 
 
 /* How the reference is done.  */
diff --git a/gcc/is-a.h b/gcc/is-a.h
index b5ee854..ccf12be 100644
--- a/gcc/is-a.h
+++ b/gcc/is-a.h
@@ -31,7 +31,7 @@  bool is_a <TYPE> (pointer)
 
     Tests whether the pointer actually points to a more derived TYPE.
 
-    Suppose you have a symtab_node_def *ptr, AKA symtab_node ptr.  You can test
+    Suppose you have a symtab_node_base *ptr, AKA symtab_node ptr.  You can test
     whether it points to a 'derived' cgraph_node as follows.
 
       if (is_a <cgraph_node> (ptr))
@@ -110,7 +110,7 @@  example,
   template <>
   template <>
   inline bool
-  is_a_helper <cgraph_node>::test (symtab_node_def *p)
+  is_a_helper <cgraph_node>::test (symtab_node_base *p)
   {
     return p->symbol.type == SYMTAB_FUNCTION;
   }
@@ -122,7 +122,7 @@  when needed may result in a crash.  For example,
   template <>
   template <>
   inline bool
-  is_a_helper <cgraph_node>::cast (symtab_node_def *p)
+  is_a_helper <cgraph_node>::cast (symtab_node_base *p)
   {
     return &p->x_function;
   }
diff --git a/gcc/symtab.c b/gcc/symtab.c
index 8dc61d0..adc7c90 100644
--- a/gcc/symtab.c
+++ b/gcc/symtab.c
@@ -48,9 +48,9 @@  const char * const ld_plugin_symbol_resolution_names[]=
 };
 
 /* Hash table used to convert declarations into nodes.  */
-static GTY((param_is (union symtab_node_def))) htab_t symtab_hash;
+static GTY((param_is (symtab_node_base))) htab_t symtab_hash;
 /* Hash table used to convert assembler names into nodes.  */
-static GTY((param_is (union symtab_node_def))) htab_t assembler_name_hash;
+static GTY((param_is (symtab_node_base))) htab_t assembler_name_hash;
 
 /* Linked list of symbol table nodes.  */
 symtab_node symtab_nodes;