From: "Jørgen Kvalsvik" <jorgen.kvalsvik@woven-planet.global>
To: gcc-patches@gcc.gnu.org
Cc: "Jørgen Kvalsvik" <jorgen.kvalsvik@woven-planet.global>
Subject: [PATCH 2/2] Split edge when edge locus and dest don't match
Date: Wed, 5 Oct 2022 14:04:03 +0200 [thread overview]
Message-ID: <20221005120403.68935-3-jorgen.kvalsvik@woven-planet.global> (raw)
In-Reply-To: <20221005120403.68935-1-jorgen.kvalsvik@woven-planet.global>
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
next prev parent reply other threads:[~2022-10-05 12:04 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-05 12:04 [PATCH 0/2] gcov: Split when edge locus differ from dest bb Jørgen Kvalsvik
2022-10-05 12:04 ` [PATCH 1/2] gcov: test switch/break line counts Jørgen Kvalsvik
2022-10-05 12:27 ` Martin Liška
2022-10-05 12:04 ` Jørgen Kvalsvik [this message]
2022-10-05 12:49 ` [PATCH 2/2] Split edge when edge locus and dest don't match Martin Liška
2022-10-06 8:12 ` Richard Biener
2022-10-06 14:28 ` Jørgen Kvalsvik
2022-10-07 6:53 ` Richard Biener
2022-10-07 11:45 ` Jørgen Kvalsvik
2022-10-11 10:57 ` Jørgen Kvalsvik
2022-10-11 11:27 ` Richard Biener
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221005120403.68935-3-jorgen.kvalsvik@woven-planet.global \
--to=jorgen.kvalsvik@woven-planet.global \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).