Re: PATCH: Profile driven specialization of indirect/virtual calls

[Jan Hubicka <hubicka at ucw dot cz>]

> Hi,
> 
> this is update to current profiler histogram infrastructure.
> 
> Bootstraped & tested on x86-linux, x86_64-linux.
> OK?

+   tmpv = create_tmp_var (optype, "PROF");
+   tmp1 = create_tmp_var (optype, "PROF");
+   stmt1 = build2 (GIMPLE_MODIFY_STMT, optype, tmpv, unshare_expr (TREE_OPERAND (call, 0)));
+   stmt2 = build2 (GIMPLE_MODIFY_STMT, optype, tmp1, fold_convert (optype, build_addr (direct_call->decl, current_function_decl)));

Please watch the length of lines.  One of my favorite asepcts of GNU
coding standard ;)
+   stmt3 = build3 (COND_EXPR, void_type_node,
+ 	    build2 (NE_EXPR, boolean_type_node, tmp1, tmpv),
+ 	    build1 (GOTO_EXPR, void_type_node, label_decl2),
+ 	    build1 (GOTO_EXPR, void_type_node, label_decl1));

This should be probably indented in the way so the later builds appear
after the "(" of toplevel build.
+   /* Fix eh edges */
+   region = lookup_stmt_eh_region (stmt1);
+   if (region >= 0 && !TREE_NOTHROW (new_call))

tree_could_throw (stmt1) is probably canonical way to test whether
statement needs EH.
  
+ /* Find calls inside STMT for that we want to measure histograms for 
+    indirect/virtual call optimization. */ 
+ static void
+ tree_indirect_call_to_profile (tree stmt, histogram_values *values)

There should be vertical space in between comment and function (I know
it is not in a lot
of my code, I will try to fix it gradually).
+   else
+     c_node->needed = true;

You can't just set c_node->needed, you can use cgraph_mark_needed_node (c_node)
if you really mean to force it to be output into file.  
But you should not need this at first place, because the function you get called
at is being output to final file anyway.  Why have you added this?

Index: gcc/libgcov.c
===================================================================
*** gcc/libgcov.c	(revision 119879)
--- gcc/libgcov.c	(working copy)
*************** __gcov_one_value_profiler (gcov_type *co
*** 753,758 ****
--- 753,768 ----
  }
  #endif
  
+ #ifdef L_gcov_indirect_call_profiler
+ /* Tries to determine the most common value among its inputs. */
+ void
+ __gcov_indirect_call_profiler (gcov_type* counter, gcov_type value, void* cur_func, void* callee_func)
+ {
+   if (cur_func == callee_func)
+     __gcov_one_value_profiler (counter, value);
+ }

You want probably to get __gcov_one_value_profiler inlined here t that is
intentionally not done by the macro machinery.  Probably
__gcov_one_value_profiler can be moved into static inline function body visible
for both.

Otherwise the patch seems fine (I would however like to know why you fiddle
with needed flag first) THere are discussion about tree-prof.exp changes.
Please ensure before commiting that the tree-prof.exp your new one is based on
didn't change.

Honza