public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Alexander Monakov <amonakov@ispras.ru>
To: gcc-patches@gcc.gnu.org
Cc: "Vladimir N. Makarov" <vmakarov@redhat.com>,
	Dmitry Melnik <dm@ispras.ru>
Subject: [PATCH] sel-sched: Fix erroneous re-use of pointer to last insn in a BB (PR 48235)
Date: Thu, 07 Apr 2011 17:45:00 -0000	[thread overview]
Message-ID: <alpine.LNX.2.00.1104072134510.17990@monoid.intra.ispras.ru> (raw)

Hello,

This patch fixes a couple of places where code motion machinery wrongly
attempts to re-use a pointer to the last insn in a BB after control flow
following that BB has been changed (so the last jump might have been removed
or replaced).  This is not too frequent, so the solution is to simply
recompute the last instruction if we notice the CFG change.

Bootstrapped and regtested on x86_64-linux together with other recently
submitted sel-sched fixes; OK for trunk?  (I forgot to mention this in two
other e-mails; sorry).


2011-04-07  Dmitry Melnik  <dm@ispras.ru>

	PR rtl-optimization/48235
	* sel-sched.c (code_motion_process_successors): Recompute the last
	insn in basic block if control flow changed.
	(code_motion_path_driver): Ditto.  Recompute the first insn as well.
	Update condition for ilist_remove.

testsuite:
	gcc.dg/pr48235.c: New.

diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
index 48fb2e0..f409c4f 100644
--- a/gcc/sel-sched.c
+++ b/gcc/sel-sched.c
@@ -6369,7 +6369,10 @@ code_motion_process_successors (insn_t insn, av_set_t orig_ops,
          the iterator becomes invalid.  We need to try again.  */
       if (BLOCK_FOR_INSN (insn)->index != old_index
           || EDGE_COUNT (bb->succs) != old_succs)
-        goto rescan;
+        {
+          insn = sel_bb_end (BLOCK_FOR_INSN (insn));
+          goto rescan;
+        }
     }
 
 #ifdef ENABLE_CHECKING
@@ -6587,21 +6590,37 @@ code_motion_path_driver (insn_t insn, av_set_t orig_ops, ilist_t path,
   if (!expr)
     {
       int res;
+      rtx last_insn = PREV_INSN (insn);
+      bool added_to_path;
 
       gcc_assert (insn == sel_bb_end (bb));
 
       /* Add bb tail to PATH (but it doesn't make any sense if it's a bb_head -
 	 it's already in PATH then).  */
       if (insn != first_insn)
-	ilist_add (&path, insn);
+	{
+	  ilist_add (&path, insn);
+	  added_to_path = true;
+	}
+      else
+        added_to_path = false;
 
       /* Process_successors should be able to find at least one
 	 successor for which code_motion_path_driver returns TRUE.  */
       res = code_motion_process_successors (insn, orig_ops,
                                             path, static_params);
 
+      /* Jump in the end of basic block could have been removed or replaced
+         during code_motion_process_successors, so recompute insn as the
+         last insn in bb.  */
+      if (NEXT_INSN (last_insn) != insn)
+        {
+          insn = sel_bb_end (bb);
+          first_insn = sel_bb_head (bb);
+        }
+
       /* Remove bb tail from path.  */
-      if (insn != first_insn)
+      if (added_to_path)
 	ilist_remove (&path);
 
       if (res != 1)
diff --git a/gcc/testsuite/gcc.dg/pr48235.c b/gcc/testsuite/gcc.dg/pr48235.c
new file mode 100644
index 0000000..8ec5edb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr48235.c
@@ -0,0 +1,57 @@
+/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+/* { dg-options "-O -fno-guess-branch-probability -fpeel-loops -freorder-blocks-and-partition -fschedule-insns2 -fsel-sched-pipelining -fselective-scheduling2" } */
+struct intC
+{
+  short x;
+  short y;
+};
+
+int size_x;
+
+static inline int
+TileDiffXY (int x, int y)
+{
+  return (y * size_x) + x;
+}
+
+struct HangarTileTable
+{
+  struct intC ti;
+  int hangar_num;
+};
+
+struct AirportSpec
+{
+  struct HangarTileTable *depot_table;
+  int size;
+};
+
+void Get ();
+struct AirportSpec dummy;
+
+static inline int
+GetRotatedTileFromOffset (int *a, struct intC tidc)
+{
+  if (!*a)
+    Get ();
+  switch (*a)
+    {
+    case 0:
+      return (tidc.y << size_x) + tidc.x;
+    case 1:
+      return TileDiffXY (tidc.y, dummy.size - tidc.x);
+    case 2:
+      return TileDiffXY (tidc.x, dummy.size - tidc.y);
+    case 3:
+      return TileDiffXY (dummy.size - 1, tidc.x);
+    }
+}
+
+int
+GetHangarNum (int *a)
+{
+	int i;
+  for (i = 0; i < dummy.size; i++)
+    if (GetRotatedTileFromOffset (a, dummy.depot_table[i].ti))
+      return dummy.depot_table[i].hangar_num;
+}

             reply	other threads:[~2011-04-07 17:45 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-07 17:45 Alexander Monakov [this message]
2011-04-07 20:20 ` Vladimir Makarov
2011-04-15 19:52 ` Steve Ellcey
2011-04-18 12:09   ` Alexander Monakov

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=alpine.LNX.2.00.1104072134510.17990@monoid.intra.ispras.ru \
    --to=amonakov@ispras.ru \
    --cc=dm@ispras.ru \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=vmakarov@redhat.com \
    /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).