From patchwork Mon Jul 17 17:37:43 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Hubicka X-Patchwork-Id: 789642 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xB9Vh53Blz9sRV for ; Tue, 18 Jul 2017 03:38:03 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="jJW2LqCp"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=rmFsclchd2ylLUUO0 4IcIdVa+6DsssvKw+unT+zMg7kRivBnXXFFvColmogdKIjIh2n+2AauZ5vvs8o2o n7aoy7HMjHIFgA02+n7aUC0e6ImLDEI9Ue3SqWZawKd0F9oB8m6eWbO3AffOdBqS l0mL+g9Vd/zMobB+89xeliDuF0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=qVMfW/8GS8NrV+5JUuA5EoJ iJEQ=; b=jJW2LqCpfm7C4jND4RVbK9iqsl+4ou7RtztcdvzuC5CeyctlF7OXirM fpoBG+jtURdkJGvC9Rt1nUc/OJjtk0UmeCKl1zuEZIeGuWKjcvp4WmuyIbpd2L8L BnNpxBqE/sgXDaSOX+pbVoXVOk/JIWtw2nPD7FDPYla0dOZ+My58= Received: (qmail 61224 invoked by alias); 17 Jul 2017 17:37:50 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 59208 invoked by uid 89); 17 Jul 2017 17:37:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, URIBL_RED autolearn=ham version=3.3.2 spammy= X-HELO: nikam.ms.mff.cuni.cz Received: from nikam.ms.mff.cuni.cz (HELO nikam.ms.mff.cuni.cz) (195.113.20.16) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 17 Jul 2017 17:37:46 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id EADC55439B2; Mon, 17 Jul 2017 19:37:43 +0200 (CEST) Date: Mon, 17 Jul 2017 19:37:43 +0200 From: Jan Hubicka To: Tom de Vries Cc: GCC Patches Subject: Re: [PATCH, PR81030] Call compute_outgoing_frequencies at expand Message-ID: <20170717173743.GA51167@kam.mff.cuni.cz> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Hi, thanks for looking into this issue. It is quite weird indeed. > Commenting out this bit makes the compilation succeed. so my understanding is that RTL expansions confuses itself and redistributes probabilities to two jumps while immediately optimizing out the second... I think in such case instead of calling compute_outgoing_frequencies which will take confused probability from REG_BR_PROB_NOTE and produce inconsistent profile, it is better to take the existing probabilities in profile and put them back to RTL. Of course it would be nice to teach RTl expansion to not do such mistakes (because the patch bellow helps only in case the BB happens to not split at all), but that would be quite difficult I guess. I am testing the attached patch. Honza > > > III. > > The bit added in find_many_sub_basic_blocks makes sense to me. > > It just seems to me that we have been relying on find_many_sub_basic_blocks > to call compute_outgoing_frequencies for all basic blocks during expand. > This patch restores that situation, and fixes the ICE. > > Bootstrapped and reg-tested on x86_64. > > OK for trunk? > > Thanks, > - Tom > Call compute_outgoing_frequencies at expand > > 2017-07-17 Tom de Vries > > PR middle-end/81030 > * cfgbuild.c (find_many_sub_basic_blocks): Add and handle update_freq > parameter. > * cfgbuild.h (find_many_sub_basic_blocks): Add update_freq parameter. > * cfgexpand.c (pass_expand::execute): Call find_many_sub_basic_blocks > with update_freq == true. > > * gcc.dg/pr81030.c: New test. > > --- > gcc/cfgbuild.c | 4 ++-- > gcc/cfgbuild.h | 3 ++- > gcc/cfgexpand.c | 2 +- > gcc/testsuite/gcc.dg/pr81030.c | 29 +++++++++++++++++++++++++++++ > 4 files changed, 34 insertions(+), 4 deletions(-) > > diff --git a/gcc/cfgbuild.c b/gcc/cfgbuild.c > index 56a2cb9..78036fe 100644 > --- a/gcc/cfgbuild.c > +++ b/gcc/cfgbuild.c > @@ -594,7 +594,7 @@ compute_outgoing_frequencies (basic_block b) > and create edges. */ > > void > -find_many_sub_basic_blocks (sbitmap blocks) > +find_many_sub_basic_blocks (sbitmap blocks, bool update_freq) > { > basic_block bb, min, max; > bool found = false; > @@ -677,7 +677,7 @@ find_many_sub_basic_blocks (sbitmap blocks) > } > else > /* If nothing changed, there is no need to create new BBs. */ > - if (EDGE_COUNT (bb->succs) == n_succs[bb->index]) > + if (!update_freq && EDGE_COUNT (bb->succs) == n_succs[bb->index]) > continue; > > compute_outgoing_frequencies (bb); > diff --git a/gcc/cfgbuild.h b/gcc/cfgbuild.h > index afda5ac..0c58590 100644 > --- a/gcc/cfgbuild.h > +++ b/gcc/cfgbuild.h > @@ -23,6 +23,7 @@ along with GCC; see the file COPYING3. If not see > extern bool inside_basic_block_p (const rtx_insn *); > extern bool control_flow_insn_p (const rtx_insn *); > extern void rtl_make_eh_edge (sbitmap, basic_block, rtx); > -extern void find_many_sub_basic_blocks (sbitmap); > +extern void find_many_sub_basic_blocks (sbitmap block, > + bool update_freq = false); > > #endif /* GCC_CFGBUILD_H */ > diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c > index 3e1d24d..e030a86 100644 > --- a/gcc/cfgexpand.c > +++ b/gcc/cfgexpand.c > @@ -6464,7 +6464,7 @@ pass_expand::execute (function *fun) > > auto_sbitmap blocks (last_basic_block_for_fn (fun)); > bitmap_ones (blocks); > - find_many_sub_basic_blocks (blocks); > + find_many_sub_basic_blocks (blocks, true); > purge_all_dead_edges (); > > expand_stack_alignment (); > diff --git a/gcc/testsuite/gcc.dg/pr81030.c b/gcc/testsuite/gcc.dg/pr81030.c > new file mode 100644 > index 0000000..23da6e5 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr81030.c > @@ -0,0 +1,29 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O1" } */ > + > +void __assert_fail (const char *, const char *, unsigned int, const char *); > + > +int a, b, c, d, e, f, h; > +unsigned char g; > + > +int main () > +{ > + int i, *j = &b; > + if (a) > + { > + if (h) > + { > + int **k = &j; > + d = 0; > + *k = &e; > + } > + else > + for (b = 0; b > -28; b = g) > + ; > + c || !j ? : __assert_fail ("c || !j", "small.c", 20, "main"); > + if (f) > + for (i = 0; i < 1; i++) > + ; > + } > + return 0; > +} Index: cfgbuild.c =================================================================== --- cfgbuild.c (revision 250246) +++ cfgbuild.c (working copy) @@ -676,7 +676,14 @@ find_many_sub_basic_blocks (sbitmap bloc else /* If nothing changed, there is no need to create new BBs. */ if (EDGE_COUNT (bb->succs) == n_succs[bb->index]) - continue; + { + /* In rare occassions RTL expansion might have mistakely assigned + a probabilities different from what is in CFG. This happens + when we try to split branch to two but optimize out the + second branch during the way. See PR81030. */ + update_br_prob_note (bb); + continue; + } compute_outgoing_frequencies (bb); }