public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Fix profiledbootstrap
@ 2023-08-03 20:44 Jan Hubicka
  0 siblings, 0 replies; 2+ messages in thread
From: Jan Hubicka @ 2023-08-03 20:44 UTC (permalink / raw)
  To: gcc-patches

Hi,
Profiledbootstrap fails with ICE in update_loop_exit_probability_scale_dom_bbs
called from loop unroling.
The reason is that under relatively rare situations, we may run into case where
loop has multiple exits and all are considered as likely but then we scale down
the profile and one of the exits becomes unlikely. 

We pass around unadjusted_exit_count to scale exit probability correctly.  In this
case we may end up using uninitialized value and profile-count type intentionally
bombs on that.

Profiledbootstrapped x86_64-linux, comitted.

gcc/ChangeLog:

	PR bootstrap/110857
	* cfgloopmanip.cc (scale_loop_profile): (Un)initialize
	unadjusted_exit_count.

diff --git a/gcc/cfgloopmanip.cc b/gcc/cfgloopmanip.cc
index 86360b5f380..b237ad4e8ac 100644
--- a/gcc/cfgloopmanip.cc
+++ b/gcc/cfgloopmanip.cc
@@ -742,7 +742,7 @@ scale_loop_profile (class loop *loop, profile_probability p,
   /* In a consistent profile unadjusted_exit_count should be same as count_in,
      however to preserve as much of the original info, avoid recomputing
      it.  */
-  profile_count unadjusted_exit_count;
+  profile_count unadjusted_exit_count = profile_count::uninitialized ();
   if (exit_edge)
     unadjusted_exit_count = exit_edge->count ();
   scale_loop_frequencies (loop, scale_prob);

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Fix profiledbootstrap
@ 2017-07-20 23:36 Jan Hubicka
  0 siblings, 0 replies; 2+ messages in thread
From: Jan Hubicka @ 2017-07-20 23:36 UTC (permalink / raw)
  To: gcc-patches

Hi,
this patch fixes ICE during profiledbootstrap about hot BB being dominated by
cold.  This is verified by RTL verify_flow_info and was added by Theresa
along with patches to undo mistakes in in sane profile.

The implementation is odd because this is not about dominance but reachability.
It is also bit questionable decision because it is possible for very hot loop
to be reahcable only by cold path (i.e. not executed every time program is
executed). But I guess it makes sense to stay in cold region once it is entered.

This patch reimplements the dominance based decision to reachability and makes
bb-reorder to promote hot bbs that are only reahcable by cold BBs to cold section.

Profilebootstrapped/regtested x86_64-linux, will commit it tomorrow.

	* bb-reorder.c (find_rarely_executed_basic_blocks_and_crossing_edges):
	Put all BBs reachable only via paths crossing cold region to cold
	region.
	* cfgrtl.c (find_bbs_reachable_by_hot_paths): New function.
Index: bb-reorder.c
===================================================================
--- bb-reorder.c	(revision 250390)
+++ bb-reorder.c	(working copy)
@@ -1665,6 +1665,12 @@ find_rarely_executed_basic_blocks_and_cr
                                           &bbs_in_hot_partition);
       if (cold_bb_count)
         sanitize_hot_paths (false, cold_bb_count, &bbs_in_hot_partition);
+
+      hash_set <basic_block> set;
+      find_bbs_reachable_by_hot_paths (&set);
+      FOR_EACH_BB_FN (bb, cfun)
+	if (!set.contains (bb))
+	  BB_SET_PARTITION (bb, BB_COLD_PARTITION);
     }
 
   /* The format of .gcc_except_table does not allow landing pads to
Index: cfgrtl.c
===================================================================
--- cfgrtl.c	(revision 250378)
+++ cfgrtl.c	(working copy)
@@ -2282,6 +2282,29 @@ get_last_bb_insn (basic_block bb)
   return end;
 }
 
+/* Add all BBs reachable from entry via hot paths into the SET.  */
+
+void
+find_bbs_reachable_by_hot_paths (hash_set<basic_block> *set)
+{
+  auto_vec<basic_block, 64> worklist;
+
+  set->add (ENTRY_BLOCK_PTR_FOR_FN (cfun));
+  worklist.safe_push (ENTRY_BLOCK_PTR_FOR_FN (cfun));
+
+  while (worklist.length () > 0)
+    {
+      basic_block bb = worklist.pop ();
+      edge_iterator ei;
+      edge e;
+
+      FOR_EACH_EDGE (e, ei, bb->succs)
+	if (BB_PARTITION (e->dest) != BB_COLD_PARTITION
+	    && !set->add (e->dest))
+	  worklist.safe_push (e->dest);
+    }
+}
+
 /* Sanity check partition hotness to ensure that basic blocks in
    the cold partition don't dominate basic blocks in the hot partition.
    If FLAG_ONLY is true, report violations as errors. Otherwise
@@ -2295,49 +2318,25 @@ find_partition_fixes (bool flag_only)
   basic_block bb;
   vec<basic_block> bbs_in_cold_partition = vNULL;
   vec<basic_block> bbs_to_fix = vNULL;
+  hash_set<basic_block> set;
 
   /* Callers check this.  */
   gcc_checking_assert (crtl->has_bb_partition);
 
-  FOR_EACH_BB_FN (bb, cfun)
-    if ((BB_PARTITION (bb) == BB_COLD_PARTITION))
-      bbs_in_cold_partition.safe_push (bb);
-
-  if (bbs_in_cold_partition.is_empty ())
-    return vNULL;
-
-  bool dom_calculated_here = !dom_info_available_p (CDI_DOMINATORS);
-
-  if (dom_calculated_here)
-    calculate_dominance_info (CDI_DOMINATORS);
+  find_bbs_reachable_by_hot_paths (&set);
 
-  while (! bbs_in_cold_partition.is_empty  ())
-    {
-      bb = bbs_in_cold_partition.pop ();
-      /* Any blocks dominated by a block in the cold section
-         must also be cold.  */
-      basic_block son;
-      for (son = first_dom_son (CDI_DOMINATORS, bb);
-           son;
-           son = next_dom_son (CDI_DOMINATORS, son))
-        {
-          /* If son is not yet cold, then mark it cold here and
-             enqueue it for further processing.  */
-          if ((BB_PARTITION (son) != BB_COLD_PARTITION))
-            {
-              if (flag_only)
-                error ("non-cold basic block %d dominated "
-                       "by a block in the cold partition (%d)", son->index, bb->index);
-              else
-                BB_SET_PARTITION (son, BB_COLD_PARTITION);
-              bbs_to_fix.safe_push (son);
-              bbs_in_cold_partition.safe_push (son);
-            }
-        }
-    }
-
-  if (dom_calculated_here)
-    free_dominance_info (CDI_DOMINATORS);
+  FOR_EACH_BB_FN (bb, cfun)
+    if (!set.contains (bb)
+	&& BB_PARTITION (bb) != BB_COLD_PARTITION)
+      {
+	if (flag_only)
+	  error ("non-cold basic block %d reachable only "
+		 "by paths crossing the cold partition", bb->index);
+	else
+	  BB_SET_PARTITION (bb, BB_COLD_PARTITION);
+	bbs_to_fix.safe_push (bb);
+	bbs_in_cold_partition.safe_push (bb);
+      }
 
   return bbs_to_fix;
 }
Index: cfgrtl.h
===================================================================
--- cfgrtl.h	(revision 250378)
+++ cfgrtl.h	(working copy)
@@ -54,5 +54,6 @@ extern void cfg_layout_initialize (int);
 extern void cfg_layout_finalize (void);
 extern void break_superblocks (void);
 extern void init_rtl_bb_info (basic_block);
+extern void find_bbs_reachable_by_hot_paths (hash_set <basic_block> *);
 
 #endif /* GCC_CFGRTL_H */

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-08-03 20:44 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-03 20:44 Fix profiledbootstrap Jan Hubicka
  -- strict thread matches above, loose matches on Subject: below --
2017-07-20 23:36 Jan Hubicka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).