Re: [PATCH] sel-sched: Fix erroneous re-use of pointer to last insn in a BB (PR 48235)

2011-04-18 Thread Alexander Monakov


On Fri, 15 Apr 2011, Steve Ellcey wrote:

 Vladimir and Dmitry,
 
 The new test you added, gcc.dg/pr48235.c, is failing on IA64 because
 it gets this excess message:
 
 cc1: note: -freorder-blocks-and-partition does not work on this architecture
 
 Could you modify the test to either filter out this message or not 
 use the -freorder-blocks-and-partition option in IA64 or not run the
 test at all on IA64.  I notice that gcc.dg/tree-prof/pr45354.c looks
 like this test but also has:
 
 /* { dg-require-effective-target freorder } */
 
 That test does not fail, or run, on IA64 due to this dg option.

I have committed the following patch.  Sorry about the problem.

2011-04-18  Alexander Monakov  amona...@ispras.ru

* gcc.dg/pr48235.c: Add dg-require-effective-target freorder.

Index: gcc/testsuite/gcc.dg/pr48235.c
===
--- gcc/testsuite/gcc.dg/pr48235.c  (revision 172642)
+++ gcc/testsuite/gcc.dg/pr48235.c  (working copy)
@@ -1,4 +1,5 @@
 /* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+/* { dg-require-effective-target freorder } */
 /* { dg-options -O -fno-guess-branch-probability -fpeel-loops 
-freorder-blocks-and-partition -fschedule-insns2 -fsel-sched-pipelining 
-fselective-scheduling2 } */
 struct intC
 {



Re: [PATCH] sel-sched: Fix erroneous re-use of pointer to last insn in a BB (PR 48235)

2011-04-15 Thread Steve Ellcey
Vladimir and Dmitry,

The new test you added, gcc.dg/pr48235.c, is failing on IA64 because
it gets this excess message:

cc1: note: -freorder-blocks-and-partition does not work on this architecture

Could you modify the test to either filter out this message or not 
use the -freorder-blocks-and-partition option in IA64 or not run the
test at all on IA64.  I notice that gcc.dg/tree-prof/pr45354.c looks
like this test but also has:

/* { dg-require-effective-target freorder } */

That test does not fail, or run, on IA64 due to this dg option.

Steve Ellcey
s...@cup.hp.com


[PATCH] sel-sched: Fix erroneous re-use of pointer to last insn in a BB (PR 48235)

2011-04-07 Thread Alexander Monakov
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  d...@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 000..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;
+}