public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Richard Biener <rguenther@suse.de>, Jan Hubicka <jh@suse.cz>
Cc: gcc-patches@gcc.gnu.org
Subject: [PATCH] cfgrtl: Fix up fixup_partitions caused ICE [PR99085]
Date: Tue, 16 Feb 2021 09:13:59 +0100	[thread overview]
Message-ID: <20210216081359.GU4020736@tucnak> (raw)

Hi!

fixup_partitions sometimes changes some basic blocks from hot partition to
cold partition, in particular if after unreachable block removal or other
optimizations a hot partition block is dominated by cold partition block(s).
It fixes up the edges and jumps on those edges, but when after reorder
blocks and in rtl (non-cfglayout) mode that is clearly not enough, because
it keeps the block order the same and so we can end up with more than
1 hot/cold section transition in the same function.

So, this patch fixes that up too.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2021-02-15  Jakub Jelinek  <jakub@redhat.com>

	PR target/99085
	* cfgrtl.c (fixup_partitions): When changing some bbs from hot to cold
	partitions, if in non-layout mode after reorder_blocks also move
	affected blocks to ensure a single partition transition.

	* gcc.dg/graphite/pr99085.c: New test.

--- gcc/cfgrtl.c.jj	2021-02-10 19:27:33.000000000 +0100
+++ gcc/cfgrtl.c	2021-02-15 16:47:41.625798717 +0100
@@ -2414,8 +2414,6 @@ find_partition_fixes (bool flag_only)
 void
 fixup_partitions (void)
 {
-  basic_block bb;
-
   if (!crtl->has_bb_partition)
     return;
 
@@ -2436,10 +2434,61 @@ fixup_partitions (void)
   /* Do the partition fixup after all necessary blocks have been converted to
      cold, so that we only update the region crossings the minimum number of
      places, which can require forcing edges to be non fallthru.  */
-  while (! bbs_to_fix.is_empty ())
+  if (! bbs_to_fix.is_empty ())
     {
-      bb = bbs_to_fix.pop ();
-      fixup_new_cold_bb (bb);
+      do
+	{
+	  basic_block bb = bbs_to_fix.pop ();
+	  fixup_new_cold_bb (bb);
+	}
+      while (! bbs_to_fix.is_empty ());
+
+      /* Fix up hot cold block grouping if needed.  */
+      if (crtl->bb_reorder_complete && current_ir_type () == IR_RTL_CFGRTL)
+	{
+	  basic_block bb, first = NULL, second = NULL;
+	  int current_partition = BB_UNPARTITIONED;
+
+	  FOR_EACH_BB_FN (bb, cfun)
+	    {
+	      if (current_partition != BB_UNPARTITIONED
+		  && BB_PARTITION (bb) != current_partition)
+		{
+		  if (first == NULL)
+		    first = bb;
+		  else if (second == NULL)
+		    second = bb;
+		  else
+		    {
+		      /* If we switch partitions for the 3rd, 5th etc. time,
+			 move bbs first (inclusive) .. second (exclusive) right
+			 before bb.  */
+		      basic_block prev_first = first->prev_bb;
+		      basic_block prev_second = second->prev_bb;
+		      basic_block prev_bb = bb->prev_bb;
+		      prev_first->next_bb = second;
+		      second->prev_bb = prev_first;
+		      prev_second->next_bb = bb;
+		      bb->prev_bb = prev_second;
+		      prev_bb->next_bb = first;
+		      first->prev_bb = prev_bb;
+		      rtx_insn *prev_first_insn = PREV_INSN (BB_HEAD (first));
+		      rtx_insn *prev_second_insn
+			= PREV_INSN (BB_HEAD (second));
+		      rtx_insn *prev_bb_insn = PREV_INSN (BB_HEAD (bb));
+		      SET_NEXT_INSN (prev_first_insn) = BB_HEAD (second);
+		      SET_PREV_INSN (BB_HEAD (second)) = prev_first_insn;
+		      SET_NEXT_INSN (prev_second_insn) = BB_HEAD (bb);
+		      SET_PREV_INSN (BB_HEAD (bb)) = prev_second_insn;
+		      SET_NEXT_INSN (prev_bb_insn) = BB_HEAD (first);
+		      SET_PREV_INSN (BB_HEAD (first)) = prev_bb_insn;
+		      second = NULL;
+		    }
+		}
+	      current_partition = BB_PARTITION (bb);
+	    }
+	  gcc_assert (!second);
+	}
     }
 }
 
--- gcc/testsuite/gcc.dg/graphite/pr99085.c.jj	2021-02-15 16:52:59.313169888 +0100
+++ gcc/testsuite/gcc.dg/graphite/pr99085.c	2021-02-15 16:51:54.589910609 +0100
@@ -0,0 +1,20 @@
+/* PR target/99085 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fgraphite-identity -fsel-sched-pipelining -fselective-scheduling2" } */
+
+void
+foo (int m, int n, int o, int i)
+{
+  long double a2[m];
+  int c2[n][o];
+  int j, k;
+
+  while (i < m)
+    a2[i++] = 13.132L;
+
+  for (j = 0; j < n; j++)
+    for (k = 0; k < o; k++)
+      c2[j][k] = 1;
+
+  __builtin_abort ();
+}

	Jakub


             reply	other threads:[~2021-02-16  8:14 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-16  8:13 Jakub Jelinek [this message]
2021-02-23  8:49 ` Patch ping Jakub Jelinek
2021-03-01 13:01   ` Patch ping^2 Jakub Jelinek
2021-03-02 23:39 ` [PATCH] cfgrtl: Fix up fixup_partitions caused ICE [PR99085] Jeff Law

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210216081359.GU4020736@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jh@suse.cz \
    --cc=rguenther@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).