Add indirect call value profiling support to GCC

Add indirect call value profiling support to GCC

[davidxl, 2013-04-17 17:18:37]

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c
File gcc/auto-profile.c (right):

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode161
gcc/auto-profile.c:161: };
Why not sharing the same hist_type enum in value-prof.h?

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode171
gcc/auto-profile.c:171: int count;
int --> gcov_type?

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode519
gcc/auto-profile.c:519: static void
New line above.

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode550
gcc/auto-profile.c:550: if (actual_count < 2)
Are the targets sorted?

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode570
gcc/auto-profile.c:570: static void afdo_vpt (gimple stmt, struct gcov_hist *v,
int hist_size)
New line.

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode914
gcc/auto-profile.c:914: for the given POS_STACK.  */
Document new parameters.

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode1193
gcc/auto-profile.c:1193: gcov_functions[i].stacks[j].hist_size =
gcov_read_unsigned ();
Why is the data not optional?

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode1203
gcc/auto-profile.c:1203: file_names[gcov_read_counter()];
missing white space (many other instances)

https://codereview.appspot.com/8814043/diff/1/gcc/tree-inline.c
File gcc/tree-inline.c (right):

https://codereview.appspot.com/8814043/diff/1/gcc/tree-inline.c#newcode1833
gcc/tree-inline.c:1833: gcov_type count = afdo_get_bb_count (copy_basic_block,
false);
comment about the 'false' parameter.

https://codereview.appspot.com/8814043/diff/1/gcc/value-prof.c
File gcc/value-prof.c (right):

https://codereview.appspot.com/8814043/diff/1/gcc/value-prof.c#newcode1223
gcc/value-prof.c:1223: if (!strcmp(fname, name))
You can use assembler_name_hash to find the cgraph_node

https://codereview.appspot.com/8814043/diff/1/gcc/value-prof.c#newcode1626
gcc/value-prof.c:1626: direct_call1 = find_func_by_global_id (val1);
May be add a parameter to find_func_by_global_id (val, is_auto_fdo)?

[dehao, 2013-04-17 20:56:17]

Hi, David,

I've addressed all the comments, and also added an early-inline tuning to make
VPT better.

Thanks,
Dehao

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c
File gcc/auto-profile.c (right):

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode161
gcc/auto-profile.c:161: };
On 2013/04/17 17:18:37, davidxl wrote:
> Why not sharing the same hist_type enum in value-prof.h?

Done.

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode171
gcc/auto-profile.c:171: int count;
On 2013/04/17 17:18:37, davidxl wrote:
> int --> gcov_type?

Done.

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode519
gcc/auto-profile.c:519: static void
On 2013/04/17 17:18:37, davidxl wrote:
> New line above.

Done.

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode550
gcc/auto-profile.c:550: if (actual_count < 2)
On 2013/04/17 17:18:37, davidxl wrote:
> Are the targets sorted?

yes, it's done by profile generator.

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode570
gcc/auto-profile.c:570: static void afdo_vpt (gimple stmt, struct gcov_hist *v,
int hist_size)
On 2013/04/17 17:18:37, davidxl wrote:
> New line.

Done.

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode914
gcc/auto-profile.c:914: for the given POS_STACK.  */
On 2013/04/17 17:18:37, davidxl wrote:
> Document new parameters.

Done.

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode1193
gcc/auto-profile.c:1193: gcov_functions[i].stacks[j].hist_size =
gcov_read_unsigned ();
On 2013/04/17 17:18:37, davidxl wrote:
> Why is the data not optional?

Done.

https://codereview.appspot.com/8814043/diff/1/gcc/auto-profile.c#newcode1203
gcc/auto-profile.c:1203: file_names[gcov_read_counter()];
On 2013/04/17 17:18:37, davidxl wrote:
> missing white space (many other instances)

Done.

https://codereview.appspot.com/8814043/diff/1/gcc/tree-inline.c
File gcc/tree-inline.c (right):

https://codereview.appspot.com/8814043/diff/1/gcc/tree-inline.c#newcode1833
gcc/tree-inline.c:1833: gcov_type count = afdo_get_bb_count (copy_basic_block,
false);
On 2013/04/17 17:18:37, davidxl wrote:
> comment about the 'false' parameter.

Done.

https://codereview.appspot.com/8814043/diff/1/gcc/value-prof.c
File gcc/value-prof.c (right):

https://codereview.appspot.com/8814043/diff/1/gcc/value-prof.c#newcode1223
gcc/value-prof.c:1223: if (!strcmp(fname, name))
On 2013/04/17 17:18:37, davidxl wrote:
> You can use assembler_name_hash to find the cgraph_node

Done.

https://codereview.appspot.com/8814043/diff/1/gcc/value-prof.c#newcode1626
gcc/value-prof.c:1626: direct_call1 = find_func_by_global_id (val1);
On 2013/04/17 17:18:37, davidxl wrote:
> May be add a parameter to find_func_by_global_id (val, is_auto_fdo)?

Done.

[davidxl, 2013-04-17 21:53:56]

https://codereview.appspot.com/8814043/diff/6001/gcc/auto-profile.c
File gcc/auto-profile.c (right):

https://codereview.appspot.com/8814043/diff/6001/gcc/auto-profile.c#newcode1797
gcc/auto-profile.c:1797: afdo_callsite_hot_enough_for_early_inline (struct
cgraph_edge *edge)
How does this heuristic alone affect performance (without looking at vpt)?

https://codereview.appspot.com/8814043/diff/6001/gcc/cgraphbuild.c
File gcc/cgraphbuild.c (right):

https://codereview.appspot.com/8814043/diff/6001/gcc/cgraphbuild.c#newcode306
gcc/cgraphbuild.c:306: direct_call1 = find_func_by_global_id (val1,
flag_auto_profile);
Should it be just 'false'?

[dehao, 2013-04-17 22:24:43]

https://codereview.appspot.com/8814043/diff/6001/gcc/auto-profile.c
File gcc/auto-profile.c (right):

https://codereview.appspot.com/8814043/diff/6001/gcc/auto-profile.c#newcode1797
gcc/auto-profile.c:1797: afdo_callsite_hot_enough_for_early_inline (struct
cgraph_edge *edge)
On 2013/04/17 21:53:56, davidxl wrote:
> How does this heuristic alone affect performance (without looking at vpt)?

I'm still testing it to compare:

vpt+einline v.s. vpt
novpt+einline v.s. novpt

https://codereview.appspot.com/8814043/diff/6001/gcc/cgraphbuild.c
File gcc/cgraphbuild.c (right):

https://codereview.appspot.com/8814043/diff/6001/gcc/cgraphbuild.c#newcode306
gcc/cgraphbuild.c:306: direct_call1 = find_func_by_global_id (val1,
flag_auto_profile);
On 2013/04/17 21:53:56, davidxl wrote:
> Should it be just 'false'?

Done.