Re: Whole program optimization and functions-only-called-once.

[Martin Jambor <mjambor at suse dot cz>]

Hi,


I have two comments and a question about this one too.  Sorry for
going through it this late.

On Mon, Nov 16, 2009 at 10:54:35AM +0100, Jan Hubicka wrote:
> This is patch I intend to commit after re-testing at x86_64-linux after
> some last minute changes.
> 
> 	* cgraph.c (cgraph_release_function_body): Update use of
> 	ipa_transforms_to_apply.
> 	(cgraph_remove_node): Remove ipa_transforms_to_apply.
> 	* cgraph.h (struct cgraph_node): Add ipa_transforms_to_apply.
> 	* cgraphunit.c (save_inline_function_body): Clear ipa_transforms for
> 	copied body.
> 	(cgraph_materialize_clone): Remove original if dead.
> 	* lto-streamer-in.c (lto_read_body): Remove FIXME and
> 	ipa_transforms_to_apply hack.
> 	* function.h (struct function): Add ipa_transforms_to_apply.
> 	* ipa.c (cgraph_remove_unreachable_nodes): Handle dead clone originals.
> 	* tree-inline.c (copy_bb): Update sanity check.
> 	(initialize_cfun): Do not copy ipa_transforms_to_apply.
> 	(expand_call_inline): remove dead clone originals.
> 	(tree_function_versioning): Merge transformation queues.
> 	* passes.c (add_ipa_transform_pass): Remove.
> 	(execute_one_ipa_transform_pass): Update ipa_transforms_to_apply
> 	tracking.
> 	(execute_all_ipa_transforms): Update.
> 	(execute_one_pass): Update.
> 
> 	* lto.c (read_cgraph_and_symbols): Set also ipa_transforms_to_apply.

...

> Index: tree-inline.c
> ===================================================================
> *** tree-inline.c	(revision 154198)
> --- tree-inline.c	(working copy)
> *************** expand_call_inline (basic_block bb, gimp
> *** 3822,3827 ****
> --- 3821,3830 ----
>     (*debug_hooks->outlining_inline_function) (cg_edge->callee->decl);
>   
>     /* Update callgraph if needed.  */
> +   if (cg_edge->callee->clone_of
> +       && !cg_edge->callee->clone_of->next_sibling_clone
> +       && !cg_edge->callee->analyzed)
> +     cgraph_remove_node (cg_edge->callee);
>     cgraph_remove_node (cg_edge->callee);

I guess you forgot to remove the unconditional call to
cgraph_remove_node.  The conditional one was probably never executed
when testing, by the way.

>   
>     id->block = NULL_TREE;
> *************** tree_function_versioning (tree old_decl,
> *** 4848,4853 ****
> --- 4851,4869 ----
>     id.src_node = old_version_node;
>     id.dst_node = new_version_node;
>     id.src_cfun = DECL_STRUCT_FUNCTION (old_decl);
> +   if (id.src_node->ipa_transforms_to_apply)
> +     {
> +       VEC(ipa_opt_pass,heap) * old_transforms_to_apply = id.dst_node->ipa_transforms_to_apply;
> +       unsigned int i;
> + 
> +       id.dst_node->ipa_transforms_to_apply = VEC_copy (ipa_opt_pass, heap,
> + 					               id.src_node->ipa_transforms_to_apply);
> +       for (i = 0; i < VEC_length (ipa_opt_pass, old_transforms_to_apply); i++)
> +         VEC_safe_push (ipa_opt_pass, heap, id.dst_node->ipa_transforms_to_apply,
> + 		       VEC_index (ipa_opt_pass,
> + 		       		  old_transforms_to_apply,
> + 				  i));
> +     }

This really looks like doubling the contents of the vector, rather
than copying it once.  Or am I missing something?

By the way, is it OK to copy it when cloning the body rather than when
creating virtual clones?  (If so, I wonder whether it is necessary at
all.)

Thanks,

Martin