On 12/5/19 5:25 PM, Martin Sebor wrote:
On 12/5/19 8:13 AM, Martin Liška wrote:
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?
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;
Hello.
+ /* Default constructor. */
+ symtab_node (symtab_type t)
I've adjusted this in the patch.
Since it takes an argument the above is not the default ctor.
It can be made one by providing a default argument for t, such
as NULL_TREE if that's valid.
I don't know the details of how these things are defined now
(POD vs non-POD) or how they're used (where POD is expected)
but having been bitten a bunch of times recently by various
GCC container templates making assumptions about their
elements being PODs, it seems worth pointing it out here.
If without this change symtab_node is a POD, adding one
could cause problems. Same for the other structs.
+ : 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)
+ {}
This is a matter of personal preference but for whatever it's
worth, I like using default zero initialization for zero-
initialized members, e.g.,
definition (), alias (), ... alias_target (), ...
Besides being less verbose it has the advantage that changing
the type of the member doesn't need to require changing its
initializer. (An argument for the more verbose form might
be that it makes the initial value immediately clear.)
Well, I do prefer the more explicit form where we will use 'false'
as default value for various flag member variables.
I'm going to install the following patch.
Martin
Martin
>From 15994371176a7e8a2efcfe94e6f6dcf9bb3a80e0 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): Fix
coding style.
* 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.
* varpool.c (varpool_node::create_empty): Use newly
created constructor.
---
gcc/cgraph.c | 12 ++-------
gcc/cgraph.h | 63 ++++++++++++++++++++++++++++++++++--------------
gcc/cgraphunit.c | 2 +-
gcc/varpool.c | 6 ++---
4 files changed, 50 insertions(+), 33 deletions(-)
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 5c72e832a23..5c7a03d61be 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 = new (ggc_alloc<symbol_table> ()) symbol_table ();
}
/* Destructor. Restore the old value of symtab. */
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 1b8a167fbdc..cdeea4d9953 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;
+ /* Constructor. */
+ explicit 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;
+ /* Constructor. */
+ explicit 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 ()), tp_first_run (false),
+ count_materialization_scale (REG_BR_PROB_BASE), profile_id (0),
+ unit_id (0), 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
{
+ /* Constructor. */
+ explicit 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
@@ -2717,21 +2759,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 0468ccfc483..e630a4f5b49 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/varpool.c b/gcc/varpool.c
index 1a30ae49d54..25a26b21341 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 new (ggc_alloc<varpool_node> ()) varpool_node ();
}
/* Return varpool node assigned to DECL. Create new one when needed. */
--
2.24.0
