This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix PR57077 (issue8840045)


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  <tejohnson@google.com>

	* 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.

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.  */

--
This patch is available for review at http://codereview.appspot.com/8840045


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]