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]

Re: Fwd: [PATCH] Scheduling result adjustment to enable macro-fusion


Sorry about the problem.

For the failed testcase, it was compiled using -fmodulo-sched.
modulo-sched phase set SCHED_GROUP_P of a jump insn to be true, which
means the jump insn should be scheduled with prev insn as a group.
When modulo scheduling is finished, the flag of SCHED_GROUP_P is not
cleaned up. After that, pass_jump2 phase split the bb and move the
prev insn to another bb. Then pass_sched2 see the flag and mistakenly
try to bind the jump insn with a code label.

I am thinking other cases setting SCHED_GROUP_P should have the same
problem because SCHED_GROUP_P is not cleaned up after scheduling is
done. The flag may become inconsistent after some optimizations and
may cause problem if it is used by later scheduling passes. I don't
know why similar problem was never exposed before.

The fix is to simply cleanup SCHED_GROUP_P flag in sched_finish.

bootstrap is ok. regression test is going on. Is it ok if regression passes?

Thanks,
Wei.

2013-11-23  Wei Mi  <wmi@google.com>

        PR rtl-optimization/59020
        * haifa-sched.c (cleanup_sched_group): New function.
        (sched_finish): Call cleanup_sched_group to cleanup SCHED_GROUP_P.

2013-11-23  Wei Mi  <wmi@google.com>
        PR rtl-optimization/59020
        * testsuite/gcc.dg/pr59020.c (void f):

Index: haifa-sched.c
===================================================================
--- haifa-sched.c       (revision 204923)
+++ haifa-sched.c       (working copy)
@@ -6598,6 +6598,23 @@ group_insns_for_macro_fusion ()
     try_group_insn (BB_END (bb));
 }

+/* Cleanup SCHED_GROUP_P after scheduling is done. This is necessary because
+   bb may be changed by other optimizations and the flag from last scheduling
+   may become invalid. If later scheduler see the flag generated from last
+   scheduling, it may produces incorrect result.  */
+
+static void
+cleanup_sched_group ()
+{
+  basic_block bb;
+  rtx insn;
+
+  FOR_EACH_BB (bb)
+    FOR_BB_INSNS(bb, insn)
+      if (INSN_P (insn) && SCHED_GROUP_P (insn))
+       SCHED_GROUP_P (insn) = 0;
+}
+
 /* Initialize some global state for the scheduler.  This function works
    with the common data shared between all the schedulers.  It is called
    from the scheduler specific initialization routine.  */
@@ -6841,6 +6858,8 @@ sched_finish (void)
     }
   free (curr_state);

+  cleanup_sched_group ();
+
   if (targetm.sched.finish_global)
     targetm.sched.finish_global (sched_dump, sched_verbose);

Index: testsuite/gcc.dg/pr59020.c
===================================================================
--- testsuite/gcc.dg/pr59020.c  (revision 0)
+++ testsuite/gcc.dg/pr59020.c  (revision 0)
@@ -0,0 +1,15 @@
+/* PR rtl-optimization/59020 */
+
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-O2 -fmodulo-sched -fno-inline -march=corei7" } */
+
+int a, b, d;
+unsigned c;
+
+void f()
+{
+  unsigned q;
+  for(; a; a++)
+    if(((c %= d && 1) ? : 1) & 1)
+      for(; b; q++);
+}

On Sat, Nov 23, 2013 at 4:34 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Nov 4, 2013 at 1:51 PM, Wei Mi <wmi@google.com> wrote:
>> Thanks! The three patches are commited as r204367, r204369 and r204371.
>>
>
> r204369 caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59020
>
> --
> H.J.


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