This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [lto][patch] Fix how we handle clones in wpa
- From: Diego Novillo <dnovillo at google dot com>
- To: Rafael Espindola <espindola at google dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 19 Feb 2009 09:55:08 -0500
- Subject: Re: [lto][patch] Fix how we handle clones in wpa
- References: <38a0d8450902190450o395d2529s71f3a205a80cc658@mail.gmail.com>
On Thu, Feb 19, 2009 at 07:50, Rafael Espindola <espindola@google.com> wrote:
> @@ -232,14 +207,14 @@ output_edge (struct lto_simple_output_block *ob,
>
> static void
> output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
> - lto_cgraph_encoder_t encoder, cgraph_node_set set)
> + lto_cgraph_encoder_t encoder, cgraph_node_set set,
> + bitmap wrote_master)
The comments to output_node are stale. Needs documentation for
WROTE_MASTER and it's referring to arguments that don't exist
anymore.
> {
> unsigned int tag;
> unsigned HOST_WIDEST_INT flags = 0;
> - struct cgraph_node *master_clone = node->master_clone;
> unsigned local, externally_visible, inlinable;
> bool boundary_p = !cgraph_node_in_set_p (node, set);
> - bool clone_p = node->master_clone && node != node->master_clone;
> + bool clone_p = bitmap_bit_p (wrote_master, DECL_UID (node->decl));
Rename clone_p to wrote_master_p? We are testing whether we
wrote a node for node->decl here. Whether node is a clone or not
is not important.
> @@ -302,13 +277,12 @@ output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
>
> if (clone_p)
> {
> - intptr_t ref = lto_cgraph_encoder_lookup (encoder, master_clone);
> LTO_DEBUG_TOKEN ("master");
> - gcc_assert (ref != LCC_NOT_FOUND);
> - lto_output_sleb128_stream (ob->main_stream, ref);
> + lto_output_fn_decl_index (ob->decl_state, ob->main_stream, node->decl);
> }
> else
> - {
> + {
> + bitmap_set_bit (wrote_master, DECL_UID (node->decl));
> lto_output_fn_decl_index (ob->decl_state, ob->main_stream, node->decl);
> LTO_DEBUG_FN_NAME (node->decl);
> }
What I don't follow here is that in both cases we write
node->decl. What difference does it make whether we find
node->decl in the wrote_master bitmap?
Couldn't this whole if() be simplified to
lto_output_fn_decl_index (ob->decl_state, ob->main_stream, node->decl);
if (!clone_p)
bitmap_set_bit (wrote_master, DECL_UID (node->decl));
> + /* The decls for which we have written a master node. Note the the node
> + we select as master might not be the current master. In fact, the
> + current master might not be in the current file at all! */
Add some comment about this not being important. Any node can be
considered to be the master clone.
> + decl_index = lto_input_uleb128 (ib);
> + fn_decl = lto_file_decl_data_get_fn_decl (file_data, decl_index);
> + /* Cannot use cgraph_master_clone in here. Fais assert that
> + cgraph_function_flags_ready is true. */
s/Fais/Fails/
> + master_clone = cgraph_node(fn_decl)->master_clone;
Space before '('.
> + struct cgraph_node *caller;
> + node = csi_node (csi);
> +
> + /* node was not inlined. We still need it. */
s/node/NODE/
> + if (!node->global.inlined_to)
> + continue;
> +
> + inlined_to = node->global.inlined_to;
> + caller = node->callers->caller;
> +
> + /* only one caller */
/* NODE should only have one caller. */
Looks OK otherwise.
Diego.