> Hi.
> 
> I've updated the patch based on Honza's notes and installed it
> as 5f32f9cf13f99f6295591927950aaf98aa8dba91.
Thanks, you omitted the patch, but looking at git log I think there is
still problem I commented on in the previous mail:

  for (unsigned i = 0; i < GCOV_TOPN_VALUES; i++)
     {
       if (value == counters[2 * i])
        {
          if (use_atomic)
-           __atomic_fetch_add (&counters[2 * i + 1], 1, __ATOMIC_RELAXED);
+           __atomic_fetch_add (&counters[2 * i + 1], GCOV_TOPN_VALUES,
+                               __ATOMIC_RELAXED);
          else
-           counters[2 * i + 1]++;
+           counters[2 * i + 1] += GCOV_TOPN_VALUES;
          return;
        }
-      else if (counters[2 * i + 1] == 0)
+      else if (counters[2 * i + 1] <= 0)
        {
          /* We found an empty slot.  */
          counters[2 * i] = value;
-         counters[2 * i + 1] = 1;
+         counters[2 * i + 1] = GCOV_TOPN_VALUES;
          return;
        }

Lets say that sequence is Y X X Z Z X and N = 2
you will get

Y 2 _ 0
Y 2 X 2
Y 2 X 4
Y 1 X 3
Y 0 X 2
X 2 X 2

while it ought to be

Y 0 X 4

the loop really needs to walk all counters since non-zero entries are
not guaranteed to be grouped at the beggining.
(I think reordering counter is worse alternative especially for
concurent updates since we probably do not want N to be very large)
> 
> Just for the record, there are stats for cc1plus for PGO bootstrap:
> 
> before:
> 
> == Stats for gcc ==
> stats for indirect_call:
>   total: 9210
>   invalid: 760
>   tracked values:
>     0 values:     6248 times (67.84%)
>     1 values:     1795 times (19.49%)
>     2 values:      220 times (2.39%)
>     3 values:       87 times (0.94%)
>     4 values:      100 times (1.09%)
> 
> stats for topn:
>   total: 1513
>   invalid: 220
>   tracked values:
>     0 values:     1028 times (67.94%)
>     1 values:      149 times (9.85%)
>     2 values:       50 times (3.30%)
>     3 values:       38 times (2.51%)
>     4 values:       28 times (1.85%)
> 
> after:
> 
> stats for indirect_call:
>   total: 9210
>   invalid: 600
>   tracked values:
>     0 values:     6280 times (68.19%)
>     1 values:     1856 times (20.15%)
>     2 values:      264 times (2.87%)
>     3 values:      157 times (1.70%)
>     4 values:       53 times (0.58%)
> 
> stats for topn:
>   total: 1513
>   invalid: 178
>   tracked values:
>     0 values:     1034 times (68.34%)
>     1 values:      176 times (11.63%)
>     2 values:       68 times (4.49%)
>     3 values:       39 times (2.58%)
>     4 values:       18 times (1.19%)

Thanks, so basically there are 2930 = 1856 + 264 + 157 + 53 + 600
trained entries. Out of that 1795 has always only one target, that
leaves 1135 entries with interesting behaviour wrt merging and we give
up on 600, that is about 50% of them.

Can you send me your stats script?  I will try it out at Clang as well
and will regenerate firefox numbers after the problem above is solved.

Honza
> 
> Martin

Reply via email to