From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id 70BF53858D37 for ; Wed, 5 Oct 2022 12:49:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 70BF53858D37 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=suse.cz Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 80EE81F88E; Wed, 5 Oct 2022 12:49:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1664974159; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=htFlNf7+7bbSWei9yWvc9/ko1FuaX/r3JYEDAaTqp64=; b=vf9pKrnlYwN/x5AMGs7QIxH1/SsOZhnYiPA6eGwmf0J7jbht6zOA5Mi6pyc1d5zdodoWuC ESasuWR+gc4twyp2AvzesQowub7Hp3bvAI3XjB7LmNaTg8GHQWPnfCTqnVzRNMRBJ8eE0/ b4fsOEW5TBtjA6yDyijDHcMnIzXqcdU= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1664974159; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=htFlNf7+7bbSWei9yWvc9/ko1FuaX/r3JYEDAaTqp64=; b=Zazp8yDRspEmTKytXX7Rpl7/P19i8pMMfADvvb+OopWR6GR22jmstEnpSQFqPoR+9l3xFB q0Y6+yY66AhgzoCQ== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 6C1C613ABD; Wed, 5 Oct 2022 12:49:19 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id ybBRGU99PWOJdAAAMHmgww (envelope-from ); Wed, 05 Oct 2022 12:49:19 +0000 Message-ID: <97cd4512-9515-1860-266d-a0bfc809e85f@suse.cz> Date: Wed, 5 Oct 2022 14:49:18 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 Subject: Re: [PATCH 2/2] Split edge when edge locus and dest don't match Content-Language: en-US To: =?UTF-8?Q?J=c3=b8rgen_Kvalsvik?= , gcc-patches@gcc.gnu.org References: <20221005120403.68935-1-jorgen.kvalsvik@woven-planet.global> <20221005120403.68935-3-jorgen.kvalsvik@woven-planet.global> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= In-Reply-To: <20221005120403.68935-3-jorgen.kvalsvik@woven-planet.global> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_SOFTFAIL,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 10/5/22 14:04, Jørgen Kvalsvik via Gcc-patches wrote: > 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. It's fine, please install it. I verified tramp3d coverage output is the same as before the patch. Martin > > 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);