Edges with locus are candidates for splitting so that the edge with
locus is the only edge out of a basic block, except when the locuses
match. The test checks the last (non-debug) statement in a basic block,
but this causes an unnecessary split when the edge locus go to a block
which coincidentally has an unrelated label. Comparing the first
statement of the destination block is the same check but does not get
tripped up by labels.

An example with source/edge/dest locus when an edge is split:

      1  int fn (int a, int b, int c) {
      2        int x = 0;
      3        if (a && b) {
      4                x = a;
      5        } else {
      6  a_:
      7            x = (a - b);
      8        }
      9
     10        return x;
     11  }

block  file  line   col  stmt

src     t.c     3    10  if (a_3(D) != 0)
edge    t.c     6     1
dest    t.c     6     1  a_:

src     t.c     3    13  if (b_4(D) != 0)
edge    t.c     6     1
dst     t.c     6     1  a_:

With label removed:

      1  int fn (int a, int b, int c) {
      2        int x = 0;
      3        if (a && b) {
      4                x = a;
      5        } else {
      6  // a_: <- label removed
      7            x = (a - b);
      8        }
      9
     10        return x;
     11

block  file  line   col  stmt

src     t.c     3    10  if (a_3(D) != 0)
edge  (null)    0     0
dest    t.c     6     1  a_:

src     t.c     3    13  if (b_4(D) != 0)
edge  (null)    0     0
dst     t.c     6     1  a_:

and this is extract from gcov-4b.c which *should* split:

    205  int
    206  test_switch (int i, int j)
    207  {
    208    int result = 0;
    209
    210    switch (i)                            /* branch(80 25) */
    211                                          /* branch(end) */
    212      {
    213        case 1:
    214          result = do_something (2);
    215          break;
    216        case 2:
    217          result = do_something (1024);
    218          break;
    219        case 3:
    220        case 4:
    221          if (j == 2)                     /* branch(67) */
    222                                          /* branch(end) */
    223            return do_something (4);
    224          result = do_something (8);
    225          break;
    226        default:
    227          result = do_something (32);
    228          switch_m++;
    229          break;
    230      }
    231    return result;
    231  }

block  file  line   col  stmt

src    4b.c   214    18  result_18 = do_something (2);
edge   4b.c   215     9
dst    4b.c   231    10  _22 = result_3;

src    4b.c   217    18  result_16 = do_something (1024);
edge   4b.c   218     9
dst    4b.c   231    10  _22 = result_3;

src    4b.c   224    18  result_12 = do_something (8);
edge   4b.c   225     9
dst    4b.c   231    10  _22 = result_3;

Note that the behaviour of comparison is preserved for the (switch) edge
splitting case. The former case now fails the check (even though
e->goto_locus is no longer a reserved location) because the dest matches
the e->locus.

gcc/ChangeLog:

        * profile.cc (branch_prob): Compare edge locus to dest, not src.
---
 gcc/profile.cc | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/gcc/profile.cc b/gcc/profile.cc
index 96121d60711..c13a79a84c2 100644
--- a/gcc/profile.cc
+++ b/gcc/profile.cc
@@ -1208,17 +1208,17 @@ branch_prob (bool thunk)
          FOR_EACH_EDGE (e, ei, bb->succs)
            {
              gimple_stmt_iterator gsi;
-             gimple *last = NULL;
+             gimple *dest = NULL;
 
              /* It may happen that there are compiler generated statements
                 without a locus at all.  Go through the basic block from the
                 last to the first statement looking for a locus.  */
-             for (gsi = gsi_last_nondebug_bb (bb);
+             for (gsi = gsi_start_nondebug_bb (bb);
                   !gsi_end_p (gsi);
-                  gsi_prev_nondebug (&gsi))
+                  gsi_next_nondebug (&gsi))
                {
-                 last = gsi_stmt (gsi);
-                 if (!RESERVED_LOCATION_P (gimple_location (last)))
+                 dest = gsi_stmt (gsi);
+                 if (!RESERVED_LOCATION_P (gimple_location (dest)))
                    break;
                }
 
@@ -1227,14 +1227,14 @@ branch_prob (bool thunk)
                 Don't do that when the locuses match, so
                 if (blah) goto something;
                 is not computed twice.  */
-             if (last
-                 && gimple_has_location (last)
+             if (dest
+                 && gimple_has_location (dest)
                  && !RESERVED_LOCATION_P (e->goto_locus)
                  && !single_succ_p (bb)
                  && (LOCATION_FILE (e->goto_locus)
-                     != LOCATION_FILE (gimple_location (last))
+                     != LOCATION_FILE (gimple_location (dest))
                      || (LOCATION_LINE (e->goto_locus)
-                         != LOCATION_LINE (gimple_location (last)))))
+                         != LOCATION_LINE (gimple_location (dest)))))
                {
                  basic_block new_bb = split_edge (e);
                  edge ne = single_succ_edge (new_bb);
-- 
2.30.2

Reply via email to