On 12/5/19 2:03 PM, Jan Hubicka wrote:
Hi.
As mentioned in the PR, there are classes in cgraph.h that are
not PODs and are initialized with ggc_alloc_cleared. So that I'm suggesting
to use proper constructors. I added ggc_new function that can be used
at different locations as well.
I'm attaching optimized dump file with how ctor expansion looks like.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks for working on this! I filled to PR to not forget to do this
(since it can trigger wrong code if ggc allocation gets inlined)
+ /* Default constructor. */
+ symtab_node (symtab_type t)
+ : type (t), resolution (LDPR_UNKNOWN), definition (0), alias (0),
+ transparent_alias (0), weakref (0), cpp_implicit_alias (0), symver (0),
+ analyzed (0), writeonly (0), refuse_visibility_changes (0),
+ externally_visible (0), no_reorder (0), force_output (0),
+ forced_by_abi (0), unique_name (0), implicit_section (0),
+ body_removed (0), used_from_other_partition (0), in_other_partition (0),
+ address_taken (0), in_init_priority_hash (0), need_lto_streaming (0),
+ offloadable (0), ifunc_resolver (0), order (0), decl (NULL_TREE),
I would use (false) for definition...order since these are flags.
I think there is no need to initialize decl, next, previous since all
allocations immediately put things into the symbol table and these items
are always initialized explicitly.
+ /* Default constructor. */
+ cgraph_node (int uid)
+ : symtab_node (SYMTAB_FUNCTION), callees (NULL), callers (NULL),
+ indirect_calls (NULL), origin (NULL), nested (NULL), next_nested (NULL),
+ next_sibling_clone (NULL), prev_sibling_clone (NULL), clones (NULL),
+ clone_of (NULL), call_site_hash (NULL), former_clone_of (NULL),
+ simdclone (NULL), simd_clones (NULL), ipa_transforms_to_apply (),
+ inlined_to (NULL), rtl (NULL), clone (), thunk (), count (),
Will ipa_transformas_to_appl end up being NULL?
count is initialized to profile_count::uninitialized in
cgraph_node::created that also can eb moved here.
+ count_materialization_scale (0), profile_id (0), unit_id (0),
+ tp_first_run (0), used_as_abstract_origin (0), lowered (0), process (0),
+ frequency (), only_called_at_startup (0), only_called_at_exit (0),
+ tm_clone (0), dispatcher_function (0), calls_comdat_local (0),
+ icf_merged (0), nonfreeing_fn (0), merged_comdat (0),
+ merged_extern_inline (0), parallelized_function (0), split_part (0),
+ indirect_call_target (0), local (0), versionable (0),
+ can_change_signature (0), redefined_extern_inline (0),
+ tm_may_enter_irr (0), ipcp_clone (0), m_uid (uid), m_summary_id (-1)
Again flags used_as_abstract_oriign...ipcp_clone are better as (false).
OK with these changes.
Honza
Hello.
I included all the Honza's notes and made some additional clean up.
What remains to be resolved is the ggc_new function. I'm testing this updated
patch.
Martin
>From d2bd24b909eb467fcb4176dc878af6701907667a Mon Sep 17 00:00:00 2001
From: Martin Liska <mli...@suse.cz>
Date: Thu, 5 Dec 2019 11:27:43 +0100
Subject: [PATCH] Come up with constructors of symtab_node, cgraph_node and
varpool_node.
gcc/ChangeLog:
2019-12-05 Martin Liska <mli...@suse.cz>
PR ipa/92737
* cgraph.c (symbol_table_test::symbol_table_test):
Use new ggc_new.
* cgraph.h (symtab_node::symtab_node): New constructor.
(cgraph_node::cgraph_node): Likewise.
(varpool_node::varpool_node): Likewise.
(symbol_table::allocate_cgraph_symbol): Use newly
created constructor.
(symbol_table::allocate_cgraph_symbol): Remove.
* cgraphunit.c (symtab_terminator): Likewise.
* ggc.h (ggc_new): New.
* toplev.c (general_init): Use new ggc_new.
* varpool.c (varpool_node::create_empty): Use newly
created constructor.
gcc/c-family/ChangeLog:
2019-12-05 Martin Liska <mli...@suse.cz>
PR ipa/92737
* c-opts.c (c_common_init_options): Use new ggc_new.
---
gcc/c-family/c-opts.c | 4 +--
gcc/cgraph.c | 12 ++-------
gcc/cgraph.h | 63 ++++++++++++++++++++++++++++++-------------
gcc/cgraphunit.c | 2 +-
gcc/ggc.h | 9 +++++++
gcc/toplev.c | 2 +-
gcc/varpool.c | 6 ++---
7 files changed, 61 insertions(+), 37 deletions(-)
diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index c913291c07c..a8d9ddd17dc 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -222,9 +222,7 @@ c_common_init_options (unsigned int decoded_options_count,
unsigned int i;
struct cpp_callbacks *cb;
- g_string_concat_db
- = new (ggc_alloc <string_concat_db> ()) string_concat_db ();
-
+ g_string_concat_db = ggc_new<string_concat_db> ();
parse_in = cpp_create_reader (c_dialect_cxx () ? CLK_GNUCXX: CLK_GNUC89,
ident_hash, line_table);
cb = cpp_get_callbacks (parse_in);
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 7288440708e..f4d76995d92 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -283,14 +283,8 @@ symbol_table::initialize (void)
cgraph_node *
symbol_table::create_empty (void)
{
- cgraph_node *node = allocate_cgraph_symbol ();
-
- node->type = SYMTAB_FUNCTION;
- node->frequency = NODE_FREQUENCY_NORMAL;
- node->count_materialization_scale = REG_BR_PROB_BASE;
cgraph_count++;
-
- return node;
+ return new (ggc_alloc<cgraph_node> ()) cgraph_node (cgraph_max_uid++);
}
/* Register HOOK to be called with DATA on each removed edge. */
@@ -510,8 +504,6 @@ cgraph_node::create (tree decl)
node->decl = decl;
- node->count = profile_count::uninitialized ();
-
if ((flag_openacc || flag_openmp)
&& lookup_attribute ("omp declare target", DECL_ATTRIBUTES (decl)))
{
@@ -3750,7 +3742,7 @@ symbol_table_test::symbol_table_test ()
{
gcc_assert (saved_symtab == NULL);
saved_symtab = symtab;
- symtab = new (ggc_alloc <symbol_table> ()) symbol_table ();
+ symtab = ggc_new<symbol_table> ();
}
/* Destructor. Restore the old value of symtab. */
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 9c086fedaef..b7dea696782 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -109,6 +109,23 @@ struct GTY((desc ("%h.type"), tag ("SYMTAB_SYMBOL"),
public:
friend class symbol_table;
+ /* Default constructor. */
+ symtab_node (symtab_type t)
+ : type (t), resolution (LDPR_UNKNOWN), definition (false), alias (false),
+ transparent_alias (false), weakref (false), cpp_implicit_alias (false),
+ symver (false), analyzed (false), writeonly (false),
+ refuse_visibility_changes (false), externally_visible (false),
+ no_reorder (false), force_output (false), forced_by_abi (false),
+ unique_name (false), implicit_section (false), body_removed (false),
+ used_from_other_partition (false), in_other_partition (false),
+ address_taken (false), in_init_priority_hash (false),
+ need_lto_streaming (false), offloadable (false), ifunc_resolver (false),
+ order (false), next_sharing_asm_name (NULL),
+ previous_sharing_asm_name (NULL), same_comdat_group (NULL), ref_list (),
+ alias_target (NULL), lto_file_data (NULL), aux (NULL),
+ x_comdat_group (NULL_TREE), x_section (NULL)
+ {}
+
/* Return name. */
const char *name () const;
@@ -901,6 +918,28 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node : public symtab_node
{
friend class symbol_table;
+ /* Default constructor. */
+ cgraph_node (int uid)
+ : symtab_node (SYMTAB_FUNCTION), callees (NULL), callers (NULL),
+ indirect_calls (NULL), origin (NULL), nested (NULL), next_nested (NULL),
+ next_sibling_clone (NULL), prev_sibling_clone (NULL), clones (NULL),
+ clone_of (NULL), call_site_hash (NULL), former_clone_of (NULL),
+ simdclone (NULL), simd_clones (NULL), ipa_transforms_to_apply (vNULL),
+ inlined_to (NULL), rtl (NULL), clone (), thunk (),
+ count (profile_count::uninitialized ()),
+ count_materialization_scale (REG_BR_PROB_BASE), profile_id (0),
+ unit_id (0), tp_first_run (false), used_as_abstract_origin (false),
+ lowered (false), process (false), frequency (NODE_FREQUENCY_NORMAL),
+ only_called_at_startup (false), only_called_at_exit (false),
+ tm_clone (false), dispatcher_function (false), calls_comdat_local (false),
+ icf_merged (false), nonfreeing_fn (false), merged_comdat (false),
+ merged_extern_inline (false), parallelized_function (false),
+ split_part (false), indirect_call_target (false), local (false),
+ versionable (false), can_change_signature (false),
+ redefined_extern_inline (false), tm_may_enter_irr (false),
+ ipcp_clone (false), m_uid (uid), m_summary_id (-1)
+ {}
+
/* Remove the node from cgraph and all inline clones inlined into it.
Skip however removal of FORBIDDEN_NODE and return true if it needs to be
removed. This allows to call the function from outer loop walking clone
@@ -1877,6 +1916,12 @@ private:
struct GTY((tag ("SYMTAB_VARIABLE"))) varpool_node : public symtab_node
{
+ /* Default constructor. */
+ varpool_node ()
+ : symtab_node (SYMTAB_VARIABLE), output (0), dynamically_initialized (0),
+ tls_model (TLS_MODEL_NONE), used_by_single_function (0)
+ {}
+
/* Dump given varpool node to F. */
void dump (FILE *f);
@@ -2389,9 +2434,6 @@ public:
hash_set <const cgraph_node *> GTY ((skip)) cloned_nodes;
private:
- /* Allocate new callgraph node. */
- inline cgraph_node * allocate_cgraph_symbol (void);
-
/* Allocate a cgraph_edge structure and fill it with data according to the
parameters of which only CALLEE can be NULL (when creating an indirect
call edge). CLONING_P should be set if properties that are copied from an
@@ -2716,21 +2758,6 @@ symbol_table::release_symbol (cgraph_node *node)
ggc_free (node);
}
-/* Allocate new callgraph node. */
-
-inline cgraph_node *
-symbol_table::allocate_cgraph_symbol (void)
-{
- cgraph_node *node;
-
- node = ggc_cleared_alloc<cgraph_node> ();
- node->type = SYMTAB_FUNCTION;
- node->m_summary_id = -1;
- node->m_uid = cgraph_max_uid++;
- return node;
-}
-
-
/* Return first static symbol with definition. */
inline symtab_node *
symbol_table::first_symbol (void)
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 1b3d2812152..b80071cad99 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -274,7 +274,7 @@ symtab_node::needed_p (void)
/* Head and terminator of the queue of nodes to be processed while building
callgraph. */
-static symtab_node symtab_terminator;
+static symtab_node symtab_terminator (SYMTAB_SYMBOL);
static symtab_node *queued_nodes = &symtab_terminator;
/* Add NODE to queue starting at QUEUED_NODES.
diff --git a/gcc/ggc.h b/gcc/ggc.h
index 6c64caaafb2..11022a36324 100644
--- a/gcc/ggc.h
+++ b/gcc/ggc.h
@@ -185,6 +185,15 @@ ggc_alloc (ALONE_CXX_MEM_STAT_INFO)
PASS_MEM_STAT));
}
+/* Allocate GGC memory of type T and call default constructor. */
+
+template<typename T>
+inline T *
+ggc_new (ALONE_CXX_MEM_STAT_INFO)
+{
+ return new (ggc_alloc<T> ()) T ();
+}
+
/* GGC allocation function that does not call finalizer for type
that have need_finalization_p equal to true. User is responsible
for calling of the destructor. */
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 059046f40f3..26af9b81fb8 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1243,7 +1243,7 @@ general_init (const char *argv0, bool init_signals)
/* Create the passes. */
g->set_passes (new gcc::pass_manager (g));
- symtab = new (ggc_alloc <symbol_table> ()) symbol_table ();
+ symtab = ggc_new<symbol_table> ();
statistics_early_init ();
debuginfo_early_init ();
diff --git a/gcc/varpool.c b/gcc/varpool.c
index 1a30ae49d54..9d8d06bef58 100644
--- a/gcc/varpool.c
+++ b/gcc/varpool.c
@@ -133,10 +133,8 @@ symbol_table::call_varpool_insertion_hooks (varpool_node *node)
varpool_node *
varpool_node::create_empty (void)
-{
- varpool_node *node = ggc_cleared_alloc<varpool_node> ();
- node->type = SYMTAB_VARIABLE;
- return node;
+{
+ return ggc_new<varpool_node> ();
}
/* Return varpool node assigned to DECL. Create new one when needed. */
--
2.24.0
