From patchwork Fri Apr 26 18:52:32 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Teresa Johnson X-Patchwork-Id: 239996 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 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id DDEA82C0097 for ; Sat, 27 Apr 2013 04:52:43 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :to:subject:mime-version:content-type:content-transfer-encoding :message-id:from; q=dns; s=default; b=LEFPYIcYKGQhyt4XZNUvOj5Ln7 q18ZzT04HXsTmb3UWOvF2DVhva5Jh/AsKdWeXah2veRQTVxM5TSen4+aslkTAvUM f3BJfHQfd8rIJH0rjU/ai5VtYddK3vL8AeZGtGlL0pB6I+7mPkNKvVeZVNrnzch/ 7sJNX3HPTZfK6My8U= 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 :to:subject:mime-version:content-type:content-transfer-encoding :message-id:from; s=default; bh=nZ0xIWIPVADOUiqJD3p1HaTdClU=; b= X50gfTiL4kvaP+jzj+g7AdOfmilQytEwskCrhfF9jr+1sZQvRRiEv2+wPIOXinuA 9JXT8RrvM80DufZv5oT9yRTOgps5TIPOSgntmV1nQCdZmZ1kWZUdZTlaTf/PNv4Z 7WyOOc+y2p0FmTkweFnksqHzCZHMAsd4HUKfGbKeBFA= Received: (qmail 12685 invoked by alias); 26 Apr 2013 18:52:36 -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 12658 invoked by uid 89); 26 Apr 2013 18:52:36 -0000 X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE, RP_MATCHES_RCVD, SPF_PASS, TW_CF, TW_TM autolearn=ham version=3.3.1 Received: from mail-gg0-f201.google.com (HELO mail-gg0-f201.google.com) (209.85.161.201) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 26 Apr 2013 18:52:35 +0000 Received: by mail-gg0-f201.google.com with SMTP id y3so220137ggc.0 for ; Fri, 26 Apr 2013 11:52:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:date:to:subject:user-agent:mime-version:content-type :content-transfer-encoding:message-id:from:x-gm-message-state; bh=rOCg2tWkX+yMIQhXBgyrPK4Kux6aTJZ3a3l2t8nRpl8=; b=pC4R600eVyvlbxXLqhEvtQj9pz6HBzNhZw6egZMClkUjR9B9mmygyQTdLORcd/Qd0c F6RLui/UKO/g4zQJzxlcPtY+8SF8nl2/xMrJPSjJ9ltbvKpKFsBH0yLxLrpF/khfregn 2wnalVNVPv0Y1/hXZc+RPjSizxd3DmFmbcwZV8bk2mScyFpRTkHr6iPjrO0lgXPMGCAC l2yrs5D0IDfJ9AxfPkJ/f1YLokH4oiKAAFxhkH15UsBu/A8+HuwD7Tcp5YNY1lPB2MiY +RJpjV9QB6TNiUk0iVWG5d8IX8kqgfZCJ49uQB8CfcHyWa7tUUEcfaQCWYdXfG/Cl7iS Koyw== X-Received: by 10.236.181.234 with SMTP id l70mr21893547yhm.53.1367002353578; Fri, 26 Apr 2013 11:52:33 -0700 (PDT) Received: from corp2gmr1-2.hot.corp.google.com (corp2gmr1-2.hot.corp.google.com [172.24.189.93]) by gmr-mx.google.com with ESMTPS id a64si718186yhi.1.2013.04.26.11.52.33 for (version=TLSv1.1 cipher=AES128-SHA bits=128/128); Fri, 26 Apr 2013 11:52:33 -0700 (PDT) Received: from tjsboxrox.mtv.corp.google.com (tjsboxrox.mtv.corp.google.com [172.17.129.49]) by corp2gmr1-2.hot.corp.google.com (Postfix) with ESMTP id 5CBC95A43B0; Fri, 26 Apr 2013 11:52:33 -0700 (PDT) Received: by tjsboxrox.mtv.corp.google.com (Postfix, from userid 147431) id D132D808B4; Fri, 26 Apr 2013 11:52:32 -0700 (PDT) Date: Fri, 26 Apr 2013 11:52:32 -0700 To: gcc-patches@gcc.gnu.org, hjl.tools@gmail.com, richard.guenther@gmail.com, reply@codereview.appspotmail.com Subject: [PATCH] Fix PR57077 (issue8840045) User-Agent: Heirloom mailx 12.5 6/20/10 MIME-Version: 1.0 Message-Id: <20130426185232.D132D808B4@tjsboxrox.mtv.corp.google.com> From: tejohnson@google.com (Teresa Johnson) X-Gm-Message-State: ALoCoQkuCSmrqV+jkUeK/47lkGmjk54TxDyzRbxZzwkM7WpcDaFdxtV1ncp0EHUA/5JDN5ax01Ddx5AFmSgInDpxQD0HJTTOKIQ0cVA4G/+vlAneOwmWHNaHJILCAuT3mYI8CptaQIapJWRoHmKfyT6nqRXlW7ZsAv2PSUq1sZH7Axlzg4k85EhzgPeizLgg8vwIptHD9nTd This patch fixes PR57077. Certain new uses of apply_probability are actually scaling the counts up, and the scale factor should not be treated as a probability as the value may exceed REG_BR_PROB_BASE. One example (from the PR) is when scaling counts up in LTO when merging profiles. Another example I found when preparing the patch to use the rounding divide in more places is when inlining COMDAT functions. Add new helper function apply_scale that does the scaling without the probability range check. I audited the new uses of apply_probability and changed the calls as appropriate. Profilebootstrapped and tested on x86_64-unknown-linux-gnu. Verified that this fixes the lto-bootstrap issue. Ok for trunk? 2013-04-26 Teresa Johnson * basic-block.h (apply_scale): New function. (apply_probability): Use apply_scale. * gimple-streamer-in.c (input_bb): Ditto. * lto-streamer-in.c (input_cfg): Ditto. * lto-cgraph.c (merge_profile_summaries): Ditto. * tree-optimize.c (execute_fixup_cfg): Ditto. * tree-inline.c (copy_bb): Update comment to use apply_scale. (copy_edges_for_bb): Ditto. (copy_cfg_body): Ditto. --- This patch is available for review at http://codereview.appspot.com/8840045 Index: gimple-streamer-in.c =================================================================== --- gimple-streamer-in.c (revision 198344) +++ gimple-streamer-in.c (working copy) @@ -329,8 +329,8 @@ input_bb (struct lto_input_block *ib, enum LTO_tag index = streamer_read_uhwi (ib); bb = BASIC_BLOCK_FOR_FUNCTION (fn, index); - bb->count = apply_probability (streamer_read_gcov_count (ib), - count_materialization_scale); + bb->count = apply_scale (streamer_read_gcov_count (ib), + count_materialization_scale); bb->frequency = streamer_read_hwi (ib); bb->flags = streamer_read_hwi (ib); Index: lto-streamer-in.c =================================================================== --- lto-streamer-in.c (revision 198344) +++ lto-streamer-in.c (working copy) @@ -635,8 +635,8 @@ input_cfg (struct lto_input_block *ib, struct func dest_index = streamer_read_uhwi (ib); probability = (int) streamer_read_hwi (ib); - count = apply_probability ((gcov_type) streamer_read_gcov_count (ib), - count_materialization_scale); + count = apply_scale ((gcov_type) streamer_read_gcov_count (ib), + count_materialization_scale); edge_flags = streamer_read_uhwi (ib); dest = BASIC_BLOCK_FOR_FUNCTION (fn, dest_index); Index: tree-inline.c =================================================================== --- tree-inline.c (revision 198344) +++ tree-inline.c (working copy) @@ -1519,7 +1519,7 @@ copy_bb (copy_body_data *id, basic_block bb, int f basic_block_info automatically. */ copy_basic_block = create_basic_block (NULL, (void *) 0, (basic_block) prev->aux); - /* Update to use apply_probability(). */ + /* Update to use apply_scale(). */ copy_basic_block->count = bb->count * count_scale / REG_BR_PROB_BASE; /* We are going to rebuild frequencies from scratch. These values @@ -1891,7 +1891,7 @@ copy_edges_for_bb (basic_block bb, gcov_type count && old_edge->dest->aux != EXIT_BLOCK_PTR) flags |= EDGE_FALLTHRU; new_edge = make_edge (new_bb, (basic_block) old_edge->dest->aux, flags); - /* Update to use apply_probability(). */ + /* Update to use apply_scale(). */ new_edge->count = old_edge->count * count_scale / REG_BR_PROB_BASE; new_edge->probability = old_edge->probability; } @@ -2278,7 +2278,7 @@ copy_cfg_body (copy_body_data * id, gcov_type coun incoming_frequency += EDGE_FREQUENCY (e); incoming_count += e->count; } - /* Update to use apply_probability(). */ + /* Update to use apply_scale(). */ incoming_count = incoming_count * count_scale / REG_BR_PROB_BASE; /* Update to use EDGE_FREQUENCY. */ incoming_frequency Index: tree-optimize.c =================================================================== --- tree-optimize.c (revision 198344) +++ tree-optimize.c (working copy) @@ -131,15 +131,15 @@ execute_fixup_cfg (void) ENTRY_BLOCK_PTR->count); ENTRY_BLOCK_PTR->count = cgraph_get_node (current_function_decl)->count; - EXIT_BLOCK_PTR->count = apply_probability (EXIT_BLOCK_PTR->count, - count_scale); + EXIT_BLOCK_PTR->count = apply_scale (EXIT_BLOCK_PTR->count, + count_scale); FOR_EACH_EDGE (e, ei, ENTRY_BLOCK_PTR->succs) - e->count = apply_probability (e->count, count_scale); + e->count = apply_scale (e->count, count_scale); FOR_EACH_BB (bb) { - bb->count = apply_probability (bb->count, count_scale); + bb->count = apply_scale (bb->count, count_scale); for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) { gimple stmt = gsi_stmt (gsi); @@ -172,7 +172,7 @@ execute_fixup_cfg (void) } FOR_EACH_EDGE (e, ei, bb->succs) - e->count = apply_probability (e->count, count_scale); + e->count = apply_scale (e->count, count_scale); /* If we have a basic block with no successors that does not end with a control statement or a noreturn call end it with Index: lto-cgraph.c =================================================================== --- lto-cgraph.c (revision 198344) +++ lto-cgraph.c (working copy) @@ -1347,10 +1347,10 @@ merge_profile_summaries (struct lto_file_decl_data file_data->profile_info.runs); lto_gcov_summary.sum_max = MAX (lto_gcov_summary.sum_max, - apply_probability (file_data->profile_info.sum_max, scale)); + apply_scale (file_data->profile_info.sum_max, scale)); lto_gcov_summary.sum_all = MAX (lto_gcov_summary.sum_all, - apply_probability (file_data->profile_info.sum_all, scale)); + apply_scale (file_data->profile_info.sum_all, scale)); /* Save a pointer to the profile_info with the largest scaled sum_all and the scale for use in merging the histogram. */ @@ -1372,8 +1372,8 @@ merge_profile_summaries (struct lto_file_decl_data /* Scale up the min value as we did the corresponding sum_all above. Use that to find the new histogram index. */ gcov_type scaled_min - = apply_probability (saved_profile_info->histogram[h_ix].min_value, - saved_scale); + = apply_scale (saved_profile_info->histogram[h_ix].min_value, + saved_scale); /* The new index may be shared with another scaled histogram entry, so we need to account for a non-zero histogram entry at new_ix. */ unsigned new_ix = gcov_histo_index (scaled_min); @@ -1386,8 +1386,8 @@ merge_profile_summaries (struct lto_file_decl_data here and place the scaled cumulative counter value in the bucket corresponding to the scaled minimum counter value. */ lto_gcov_summary.histogram[new_ix].cum_value - += apply_probability (saved_profile_info->histogram[h_ix].cum_value, - saved_scale); + += apply_scale (saved_profile_info->histogram[h_ix].cum_value, + saved_scale); lto_gcov_summary.histogram[new_ix].num_counters += saved_profile_info->histogram[h_ix].num_counters; } @@ -1419,8 +1419,8 @@ merge_profile_summaries (struct lto_file_decl_data if (scale == REG_BR_PROB_BASE) continue; for (edge = node->callees; edge; edge = edge->next_callee) - edge->count = apply_probability (edge->count, scale); - node->count = apply_probability (node->count, scale); + edge->count = apply_scale (edge->count, scale); + node->count = apply_scale (node->count, scale); } } Index: basic-block.h =================================================================== --- basic-block.h (revision 198344) +++ basic-block.h (working copy) @@ -500,7 +500,7 @@ struct edge_list REG_BR_PROB_BASE) /* Compute a scale factor (or probability) suitable for scaling of - gcov_type values via apply_probability(). */ + gcov_type values via apply_probability() and apply_scale(). */ #define GCOV_COMPUTE_SCALE(num,den) \ ((den) ? RDIV ((num) * REG_BR_PROB_BASE, (den)) : REG_BR_PROB_BASE) @@ -952,13 +952,23 @@ combine_probabilities (int prob1, int prob2) return RDIV (prob1 * prob2, REG_BR_PROB_BASE); } +/* Apply scale factor SCALE on frequency or count FREQ. Use this + interface when potentially scaling up, so that SCALE is not + constrained to be < REG_BR_PROB_BASE. */ + +static inline gcov_type +apply_scale (gcov_type freq, int scale) +{ + return RDIV (freq * scale, REG_BR_PROB_BASE); +} + /* Apply probability PROB on frequency or count FREQ. */ static inline gcov_type apply_probability (gcov_type freq, int prob) { check_probability (prob); - return RDIV (freq * prob, REG_BR_PROB_BASE); + return apply_scale (freq, prob); } /* Return inverse probability for PROB. */