From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from nikam.ms.mff.cuni.cz (nikam.ms.mff.cuni.cz [195.113.20.16]) by sourceware.org (Postfix) with ESMTPS id 961063861838 for ; Tue, 11 Aug 2020 10:03:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 961063861838 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=ucw.cz Authentication-Results: sourceware.org; spf=none smtp.mailfrom=hubicka@kam.mff.cuni.cz Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 4DA29280876; Tue, 11 Aug 2020 12:03:48 +0200 (CEST) Date: Tue, 11 Aug 2020 12:03:48 +0200 From: Jan Hubicka To: gcc-patches@gcc.gnu.org Subject: Do not combine PRED_LOOP_GUARD and PRED_LOOP_GUARD_WITH_RECURSION Message-ID: <20200811100348.GA2190@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.10.1 (2018-07-13) X-Spam-Status: No, score=-19.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 Aug 2020 10:03:50 -0000 Hi, this patch avoids both PRED_LOOP_GUARD and PRED_LOOP_GUARD_WITH_RECURSION to be attached to one edge. We have logic that prevents same predictor to apply to one edge twice, but since we split LOOP_GUARD to two more specialized cases, this no longer fires. Double prediction happens in exchange benchmark and leads to unrealistically low hitrates on some edges which in turn leads to bad IPA profile and misguides ipa-cp. Unforutnately it seems that the bad profile also leads to bit better performance by disabling some of loop stuff, but that really ought to be done in some meaningful way, not by an accident. Bootstrapped/regtested x86_64-linux, comitted. Honza gcc/ChangeLog: 2020-08-11 Jan Hubicka * predict.c (not_loop_guard_equal_edge_p): New function. (maybe_predict_edge): New function. (predict_paths_for_bb): Use it. (predict_paths_leading_to_edge): Use it. gcc/testsuite/ChangeLog: 2020-08-11 Jan Hubicka * gcc.dg/ipa/ipa-clone-2.c: Lower threshold from 500 to 400. diff --git a/gcc/predict.c b/gcc/predict.c index 2164a06e083..4c4bba54939 100644 --- a/gcc/predict.c +++ b/gcc/predict.c @@ -3122,6 +3122,35 @@ tree_guess_outgoing_edge_probabilities (basic_block bb) bb_predictions = NULL; } +/* Filter function predicate that returns true for a edge predicate P + if its edge is equal to DATA. */ + +static bool +not_loop_guard_equal_edge_p (edge_prediction *p, void *data) +{ + return p->ep_edge != (edge)data || p->ep_predictor != PRED_LOOP_GUARD; +} + +/* Predict edge E with PRED unless it is already predicted by some predictor + considered equivalent. */ + +static void +maybe_predict_edge (edge e, enum br_predictor pred, enum prediction taken) +{ + if (edge_predicted_by_p (e, pred, taken)) + return; + if (pred == PRED_LOOP_GUARD + && edge_predicted_by_p (e, PRED_LOOP_GUARD_WITH_RECURSION, taken)) + return; + /* Consider PRED_LOOP_GUARD_WITH_RECURSION superrior to LOOP_GUARD. */ + if (pred == PRED_LOOP_GUARD_WITH_RECURSION) + { + edge_prediction **preds = bb_predictions->get (e->src); + if (preds) + filter_predictions (preds, not_loop_guard_equal_edge_p, e); + } + predict_edge_def (e, pred, taken); +} /* Predict edges to successors of CUR whose sources are not postdominated by BB by PRED and recurse to all postdominators. */ @@ -3177,10 +3206,7 @@ predict_paths_for_bb (basic_block cur, basic_block bb, regions that are only reachable by abnormal edges. We simply prevent visiting given BB twice. */ if (found) - { - if (!edge_predicted_by_p (e, pred, taken)) - predict_edge_def (e, pred, taken); - } + maybe_predict_edge (e, pred, taken); else if (bitmap_set_bit (visited, e->src->index)) predict_paths_for_bb (e->src, e->src, pred, taken, visited, in_loop); } @@ -3223,7 +3249,7 @@ predict_paths_leading_to_edge (edge e, enum br_predictor pred, if (!has_nonloop_edge) predict_paths_for_bb (bb, bb, pred, taken, auto_bitmap (), in_loop); else - predict_edge_def (e, pred, taken); + maybe_predict_edge (e, pred, taken); } /* This is used to carry information about basic blocks. It is diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-clone-2.c b/gcc/testsuite/gcc.dg/ipa/ipa-clone-2.c index d513020ee8b..53ae25a1e24 100644 --- a/gcc/testsuite/gcc.dg/ipa/ipa-clone-2.c +++ b/gcc/testsuite/gcc.dg/ipa/ipa-clone-2.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O3 -fdump-ipa-cp-details -fno-early-inlining --param ipa-cp-max-recursive-depth=8" } */ +/* { dg-options "-O3 -fdump-ipa-cp-details -fno-early-inlining --param ipa-cp-max-recursive-depth=8 --param=ipa-cp-eval-threshold=400" } */ int fn();