On 2023/3/2 18:45, Richard Biener wrote:


   small.gcno:  648:                  block 2:`small.c':1, 3, 4, 6
   small.gcno:  688:    01450000:  36:LINES
   small.gcno:  700:                  block 3:`small.c':8, 9
   small.gcno:  732:    01450000:  32:LINES
   small.gcno:  744:                  block 5:`small.c':10
-small.gcno:  772:    01450000:  32:LINES
-small.gcno:  784:                  block 6:`small.c':12
-small.gcno:  812:    01450000:  36:LINES
-small.gcno:  824:                  block 7:`small.c':12, 13
+small.gcno:  772:    01450000:  36:LINES
+small.gcno:  784:                  block 6:`small.c':12, 13
+small.gcno:  816:    01450000:  32:LINES
+small.gcno:  828:                  block 8:`small.c':14
   small.gcno:  856:    01450000:  32:LINES
-small.gcno:  868:                  block 8:`small.c':14
-small.gcno:  896:    01450000:  32:LINES
-small.gcno:  908:                  block 9:`small.c':17
+small.gcno:  868:                  block 9:`small.c':17

Looking at the CFG and the instrumentation shows

   <bb 2> :
   PROF_edge_counter_17 = __gcov0.f[0];
   PROF_edge_counter_18 = PROF_edge_counter_17 + 1;
   __gcov0.f[0] = PROF_edge_counter_18;
   [t.c:3:7] p_6 = 0;
   [t.c:5:3] switch (s_7(D)) <default: <L6> [INV], [t.c:7:5] case 0:
<L0> [INV], [t.c:11:5] case 1: <L3> [INV]>

   <bb 3> :
   # n_1 = PHI <n_8(D)(2), [t.c:8:28] n_13(4)>
   # p_3 = PHI <[t.c:3:7] p_6(2), [t.c:8:15] p_12(4)>
[t.c:7:5] <L0>:
   [t.c:8:15] p_12 = p_3 + 1;
   [t.c:8:28] n_13 = n_1 + -1;
   [t.c:8:28] if (n_13 != 0)
     goto <bb 4>; [INV]
   else
     goto <bb 5>; [INV]

   <bb 4> :
   PROF_edge_counter_21 = __gcov0.f[2];
   PROF_edge_counter_22 = PROF_edge_counter_21 + 1;
   __gcov0.f[2] = PROF_edge_counter_22;
   [t.c:7:5] goto <bb 3>; [100.00%]

   <bb 5> :
   PROF_edge_counter_23 = __gcov0.f[3];
   PROF_edge_counter_24 = PROF_edge_counter_23 + 1;
   __gcov0.f[3] = PROF_edge_counter_24;
   [t.c:9:16] _14 = p_12;
   [t.c:9:16] goto <bb 10>; [INV]

so the reason this goes wrong is that gcov associates the "wrong"
counter with the block containing
the 'case' label(s), for the case 0 it should have chosen the counter
from bb5 but it likely
computed the count of bb3?

It might be that ordering blocks differently puts the instrumentation
to different blocks or it
makes gcovs association chose different blocks but that means it's
just luck and not fixing
the actual issue?

To me it looks like the correct thing to investigate is switch
statement and/or case label
handling.  One can also see that <L0> having line number 7 is wrong to
the extent that
the position of the label doesn't match the number of times it
executes in the source.  So
placement of the label is wrong here, possibly caused by CFG cleanup
after CFG build
(but generally labels are not used for anything once the CFG is built
and coverage
instrumentation is late so it might fail due to us moving labels).  It
might be OK to
avoid moving labels for --coverage but then coverage should possibly
look at edges
rather than labels?


Thanks, I investigated the Labels, it seems wrong at the beginning from
.gimple to .cfg very early quite like PR90574:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90574

.gimple:

int f (int s, int n)
[small.c:2:1] {
  int D.2755;
  int p;

  [small.c:3:7] p = 0;
  [small.c:5:3] switch (s) <default: <D.2756>, [small.c:7:5] case 0: <D.2743>, 
[small.c:11:5] case 1: <D.2744>>
  [small.c:7:5] <D.2743>:          <= case label
  <D.2748>:                        <= loop label
  [small.c:8:13] p = p + 1;
  [small.c:8:26] n = n + -1;
  [small.c:8:26] if (n != 0) goto <D.2748>; else goto <D.2746>;
  <D.2746>:
  [small.c:9:14] D.2755 = p;
  [small.c:9:14] return D.2755;
  [small.c:11:5] <D.2744>:
  <D.2751>:
  [small.c:12:13] p = p + 1;
  [small.c:12:26] n = n + -1;
  [small.c:12:26] if (n != 0) goto <D.2751>; else goto <D.2749>;
  <D.2749>:
  [small.c:13:14] D.2755 = p;
  [small.c:13:14] return D.2755;
  <D.2756>:
  [small.c:16:10] D.2755 = 0;
  [small.c:16:10] return D.2755;
}

.cfg:

int f (int s, int n)
{
  int p;
  int D.2755;

  <bb 2> :
  [small.c:3:7] p = 0;
  [small.c:5:3] switch (s) <default: <L6> [INV], [small.c:7:5] case 0: <L0> [INV], 
[small.c:11:5] case 1: <L3> [INV]>

  <bb 3> :
[small.c:7:5] <L0>:           <= case 0
  [small.c:8:13 discrim 1] p = p + 1;
  [small.c:8:26 discrim 1] n = n + -1;
  [small.c:8:26 discrim 1] if (n != 0)
    goto <bb 3>; [INV]
  else
    goto <bb 4>; [INV]

  <bb 4> :
  [small.c:9:14] D.2755 = p;
  [small.c:9:14] goto <bb 8>; [INV]

  <bb 5> :
[small.c:11:5] <L3>:          <= case 1
  [small.c:12:13 discrim 1] p = p + 1;
  [small.c:12:26 discrim 1] n = n + -1;
  [small.c:12:26 discrim 1] if (n != 0)
    goto <bb 5>; [INV]
  else
    goto <bb 6>; [INV]


The labels are merged into the loop unexpected, so I tried below fix
for --coverage if two labels are not on same line to start new basic block:


index 10ca86714f4..b788198ac31 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -2860,6 +2860,13 @@ stmt_starts_bb_p (gimple *stmt, gimple *prev_stmt)
              || !DECL_ARTIFICIAL (gimple_label_label (plabel)))
            return true;

+         location_t loc_prev = gimple_location (plabel);
+         location_t locus = gimple_location (label_stmt);
+         expanded_location locus_e = expand_location (locus);
+
+         if (flag_test_coverage && !same_line_p (locus, &locus_e, loc_prev))
+           return true;
+
          cfg_stats.num_merged_labels++;
          return false;
        }

.profile:

  <bb 2> :
  PROF_edge_counter_17 = __gcov0.f[0];
  PROF_edge_counter_18 = PROF_edge_counter_17 + 1;
  __gcov0.f[0] = PROF_edge_counter_18;
  [small.c:3:7] p_6 = 0;
  [small.c:5:3] switch (s_7(D)) <default: <L6> [INV], [small.c:7:5] case 0: <L0> [INV], 
[small.c:11:5] case 1: <L3> [INV]>

  <bb 3> :
[small.c:7:5] <L0>:             <= case 0
  PROF_edge_counter_19 = __gcov0.f[1];
  PROF_edge_counter_20 = PROF_edge_counter_19 + 1;
  __gcov0.f[1] = PROF_edge_counter_20;

  <bb 4> :
  # n_1 = PHI <n_8(D)(3), [small.c:8:26 discrim 1] n_13(5)>
  # p_3 = PHI <[small.c:3:7] p_6(3), [small.c:8:13 discrim 1] p_12(5)>
  [small.c:8:13 discrim 1] p_12 = p_3 + 1;
  [small.c:8:26 discrim 1] n_13 = n_1 + -1;
  [small.c:8:26 discrim 1] if (n_13 != 0)
    goto <bb 5>; [INV]
  else
    goto <bb 6>; [INV]

  <bb 5> :
  PROF_edge_counter_23 = __gcov0.f[3];
  PROF_edge_counter_24 = PROF_edge_counter_23 + 1;
  __gcov0.f[3] = PROF_edge_counter_24;
  goto <bb 4>; [100.00%]

  <bb 6> :
  [small.c:9:14] _14 = p_12;
  [small.c:9:14] goto <bb 12>; [INV]

  <bb 7> :
[small.c:11:5] <L3>:               <= case 1
  PROF_edge_counter_21 = __gcov0.f[2];
  PROF_edge_counter_22 = PROF_edge_counter_21 + 1;
  __gcov0.f[2] = PROF_edge_counter_22;


  <bb 8> :
  # n_2 = PHI <n_8(D)(7), [small.c:12:26 discrim 1] n_10(9)>
  # p_4 = PHI <[small.c:3:7] p_6(7), [small.c:12:13 discrim 1] p_9(9)>
  [small.c:12:13 discrim 1] p_9 = p_4 + 1;
  [small.c:12:26 discrim 1] n_10 = n_2 + -1;
  [small.c:12:26 discrim 1] if (n_10 != 0)
    goto <bb 9>; [INV]
  else
    goto <bb 10>; [INV]

  <bb 9> :
  PROF_edge_counter_25 = __gcov0.f[4];
  PROF_edge_counter_26 = PROF_edge_counter_25 + 1;
  __gcov0.f[4] = PROF_edge_counter_26;
  goto <bb 8>; [100.00%]


then label lines are also correct now:

.c.gcov:

Lines executed:90.91% of 11
        -:    0:Source:small.c
        -:    0:Graph:small.gcno
        -:    0:Data:small.gcda
        -:    0:Runs:1
        2:    1:int f(int s, int n)
        -:    2:{
        2:    3:  int p = 0;
        -:    4:
        2:    5:  switch (s)
        -:    6:    {
        1:    7:    case 0:
        5:    8:      do { p++; } while (--n);
        1:    9:      return p;
        -:   10:
        1:   11:    case 1:
        5:   12:      do { p++; } while (--n);
        1:   13:      return p;
        -:   14:    }
        -:   15:
    #####:   16:  return 0;
        -:   17:}
        -:   18:
        1:   19:int main() { f(0, 5); f(1, 5);}

Reply via email to