Selective scheduling fixes

Selective scheduling fixes

[Andrey Belevantsev]

Hello,

I will post patches to various recent selective scheduling PRs as replies
to this mail.  The patches have been tested on x86-64 (default languages)
and ia64 (c.c++), in case of ppc issues I've checked on the ppc cross
compiler.  I will also run the ia64 boostrap with enabled sel-sched but it
will take more time.

Best,
Andrey

Fix PR 83530

[Andrey Belevantsev]

Hello,

This issue is when we cannot correctly make insn tick data for the
unscheduled code (but processed by the modulo scheduler).  Fixed by closely
following usual scheduling process as described in the PR.

Best,
Andrey

2018-04-03  Andrey Belevantsev  <abel@ispras.ru>

	PR rtl-optimization/83530

	* sel-sched.c (force_next_insn): New global variable.
	(remove_insn_for_debug): When force_next_insn is true, also leave only
next insn
	in the ready list.
	(sel_sched_region): When the region wasn't scheduled, make another pass
over it
	with force_next_insn set to 1.


	* gcc.dg/pr8350.c: New test.

diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
index 76092f9587a..fca2b69c5ee 100644
--- a/gcc/sel-sched.c
+++ b/gcc/sel-sched.c
@@ -5004,12 +5004,16 @@ remove_temp_moveop_nops (bool full_tidying)
    distinguishing between bookkeeping copies and original insns.  */
 static int max_uid_before_move_op = 0;

+/* When true, we're always scheduling next insn on the already scheduled code
+   to get the right insn data for the following bundling or other passes.  */
+static int force_next_insn = 0;
+
 /* Remove from AV_VLIW_P all instructions but next when debug counter
    tells us so.  Next instruction is fetched from BNDS.  */
 static void
 remove_insns_for_debug (blist_t bnds, av_set_t *av_vliw_p)
 {
-  if (! dbg_cnt (sel_sched_insn_cnt))
+  if (! dbg_cnt (sel_sched_insn_cnt) || force_next_insn)
     /* Leave only the next insn in av_vliw.  */
     {
       av_set_iterator av_it;
@@ -7642,7 +7646,13 @@ sel_sched_region (int rgn)
     sel_sched_region_1 ();
   else
     /* Force initialization of INSN_SCHED_CYCLEs for correct bundling.  */
-    reset_sched_cycles_p = true;
+    {
+      reset_sched_cycles_p = false;
+      pipelining_p = false;
+      force_next_insn = 1;
+      sel_sched_region_1 ();
+      force_next_insn = 0;
+    }

   sel_region_finish (reset_sched_cycles_p);
 }
diff --git a/gcc/testsuite/gcc.dg/pr83530.c b/gcc/testsuite/gcc.dg/pr83530.c
new file mode 100644
index 00000000000..f4d8927de92
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr83530.c
@@ -0,0 +1,15 @@
+/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+/* { dg-options "-O2 -fmodulo-sched -fselective-scheduling2" } */
+int vm, z0;
+short int mz;
+
+int
+ny (void)
+{
+  int ch;
+
+  for (ch = 0; ch < 6; ++ch)
+    vm += ch / vm;
+
+  return z0 + mz;
+}

Fix PR 83962

[Andrey Belevantsev]

Hello,

This issues is about the correct order in which we need to call the
routines that fix up the control flow for us.

Best,
Andrey

2018-04-03  Andrey Belevantsev  <abel@ispras.ru>

	PR rtl-optimization/83962

	* sel-sched-ir.c (tidy_control_flow): Correct the order in which we call
tidy_fallthru_edge
	and tidy_control_flow.

	* gcc.dg/pr83962.c: New test.

diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
index a965d2ec42f..f6de96a7f3d 100644
--- a/gcc/sel-sched-ir.c
+++ b/gcc/sel-sched-ir.c
@@ -3839,9 +3839,13 @@ tidy_control_flow (basic_block xbb, bool full_tidying)
       && INSN_SCHED_TIMES (BB_END (xbb)) == 0
       && !IN_CURRENT_FENCE_P (BB_END (xbb)))
     {
-      if (sel_remove_insn (BB_END (xbb), false, false))
-        return true;
+      /* We used to call sel_remove_insn here that can trigger
tidy_control_flow
+         before we fix up the fallthru edge.  Correct that ordering by
+        explicitly doing the latter before the former.  */
+      clear_expr (INSN_EXPR (BB_END (xbb)));
       tidy_fallthru_edge (EDGE_SUCC (xbb, 0));
+      if (tidy_control_flow (xbb, false))
+       return true;
     }

   first = sel_bb_head (xbb);
diff --git a/gcc/testsuite/gcc.dg/pr83962.c b/gcc/testsuite/gcc.dg/pr83962.c
new file mode 100644
index 00000000000..0547e218715
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr83962.c
@@ -0,0 +1,10 @@
+/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+/* { dg-options "-std=gnu99 -O1 -fselective-scheduling2 -fschedule-insns2
-fcse-follow-jumps -fno-ssa-phiopt -fno-guess-branch-probability" } */
+unsigned int ca;
+
+void
+v6 (long long unsigned int as, int p9)
+{
+  while (p9 < 1)
+    as = (as != ca) || (as > 1);
+}

Fix PR 83913

[Andrey Belevantsev]

Hello,

This issue ended up being fixed the way different from described in the PR.
 We do not want to walk away from the invariant "zero SCHED_TIMES -- insn
is not scheduled" even for bookkeeping copies (testing showed it trips over
asserts designed to catch this).  Rather we choose merging exprs in the way
the larger sched-times wins.

Best,
Andrey

2018-04-03  Andrey Belevantsev  <abel@ispras.ru>

	PR rtl-optimization/83913

	* sel-sched-ir.c (merge_expr_data): Choose the middle between two
different sched-times
	when merging exprs.

	* gcc.dg/pr83913.c: New test.

diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
index a965d2ec42f..f6de96a7f3d 100644
--- a/gcc/sel-sched-ir.c
+++ b/gcc/sel-sched-ir.c
@@ -1837,8 +1837,9 @@ merge_expr_data (expr_t to, expr_t from, insn_t
split_point)
   if (EXPR_PRIORITY (to) < EXPR_PRIORITY (from))
     EXPR_PRIORITY (to) = EXPR_PRIORITY (from);

-  if (EXPR_SCHED_TIMES (to) > EXPR_SCHED_TIMES (from))
-    EXPR_SCHED_TIMES (to) = EXPR_SCHED_TIMES (from);
+  if (EXPR_SCHED_TIMES (to) != EXPR_SCHED_TIMES (from))
+    EXPR_SCHED_TIMES (to) = ((EXPR_SCHED_TIMES (from) + EXPR_SCHED_TIMES (to)
+                             + 1) / 2);

   if (EXPR_ORIG_BB_INDEX (to) != EXPR_ORIG_BB_INDEX (from))
     EXPR_ORIG_BB_INDEX (to) = 0;
@@ -3293,11 +3294,22 @@ has_dependence_note_mem_dep (rtx mem ATTRIBUTE_UNUSED,

 /* Note a dependence.  */
 static void
diff --git a/gcc/testsuite/gcc.dg/pr83913.c b/gcc/testsuite/gcc.dg/pr83913.c
new file mode 100644
index 00000000000..c898d71a261
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr83913.c
@@ -0,0 +1,26 @@
+/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+/* { dg-options "-O2 -funroll-all-loops -fselective-scheduling
-fsel-sched-pipelining -fschedule-insns -fno-dce -fno-forward-propagate
-fno-rerun-cse-after-loop -fno-web" } */
+
+int jo, z4;
+
+int
+be (long unsigned int l7, int nt)
+{
+  int en;
+
+  jo = l7;
+  for (en = 0; en < 24; ++en)
+    {
+      jo = (jo / z4) * (!!jo >= ((!!nt) & 2));
+      if (jo == 0)
+        nt = 0;
+      else
+        {
+          nt = z4;
+          ++z4;
+          nt = (long unsigned int) nt == (l7 + 1);
+        }
+    }
+
+  return nt;
+}

Re: Fix PR 83913

[Alexander Monakov]

On Tue, 3 Apr 2018, Andrey Belevantsev wrote:

> Hello,
> 
> This issue ended up being fixed the way different from described in the PR.
>  We do not want to walk away from the invariant "zero SCHED_TIMES -- insn
> is not scheduled" even for bookkeeping copies (testing showed it trips over
> asserts designed to catch this).  Rather we choose merging exprs in the way
> the larger sched-times wins.

... but the Changelog and the actual patch take the average rather than the
maximum sched-time? :)  I believe either way would be acceptable, but please
clarify the intent.

Alexander

Fix PRs 80463, 83972, 83480

[Andrey Belevantsev]

Hello,

In these PRs we deal with the dependencies we are forced to make between a
debug insn and its previous insn (unless bb head).  In the latter case, if
such an insn has been moved, we fixup its movement so it aligns with the
sel-sched invariants.  We also carefully adjust seqnos in the case we had a
single debug insn left in the block.

Best,
Andrey

2018-04-03  Andrey Belevantsev  <abel@ispras.ru>

	PR rtl-optimization/80463
	PR rtl-optimization/83972
	PR rtl-optimization/83480

	* sel-sched-ir.c (has_dependence_note_mem_dep): Take into account the
correct producer for the insn.
	(tidy_control_flow): Fixup seqnos in case of debug insns.

	* gcc.dg/pr80463.c: New test.
	* g++.dg/pr80463.C: Likewise.
	* gcc.dg/pr83972.c: Likewise.

diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
index a965d2ec42f..f6de96a7f3d 100644
--- a/gcc/sel-sched-ir.c
+++ b/gcc/sel-sched-ir.c
@@ -3293,11 +3293,22 @@ has_dependence_note_mem_dep (rtx mem ATTRIBUTE_UNUSED,

 /* Note a dependence.  */
 static void
-has_dependence_note_dep (insn_t pro ATTRIBUTE_UNUSED,
+has_dependence_note_dep (insn_t pro,
                         ds_t ds ATTRIBUTE_UNUSED)
 {
-  if (!sched_insns_conditions_mutex_p (has_dependence_data.pro,
-                                      VINSN_INSN_RTX
(has_dependence_data.con)))
+  insn_t real_pro = has_dependence_data.pro;
+  insn_t real_con = VINSN_INSN_RTX (has_dependence_data.con);
+
+  /* We do not allow for debug insns to move through others unless they
+     are at the start of bb.  Such insns may create bookkeeping copies
+     that would not be able to move up breaking sel-sched invariants.
+     Detect that here and allow that movement if we allowed it before
+     in the first place.  */
+  if (DEBUG_INSN_P (real_con)
+      && INSN_UID (NEXT_INSN (pro)) == INSN_UID (real_con))
+    return;
+
+  if (!sched_insns_conditions_mutex_p (real_pro, real_con))
     {
       ds_t *dsp = &has_dependence_data.has_dep_p[has_dependence_data.where];

@@ -3890,6 +3905,19 @@ tidy_control_flow (basic_block xbb, bool full_tidying)

       gcc_assert (EDGE_SUCC (xbb->prev_bb, 0)->flags & EDGE_FALLTHRU);

+      /* We could have skipped some debug insns which did not get removed
with the block,
+         and the seqnos could become incorrect.  Fix them up here.  */
+      if (MAY_HAVE_DEBUG_INSNS && (sel_bb_head (xbb) != first ||
sel_bb_end (xbb) != last))
+       {
+         if (!sel_bb_empty_p (xbb->prev_bb))
+           {
+             int prev_seqno = INSN_SEQNO (sel_bb_end (xbb->prev_bb));
+             if (prev_seqno > INSN_SEQNO (sel_bb_head (xbb)))
+               for (insn_t insn = sel_bb_head (xbb); insn != first; insn =
NEXT_INSN (insn))
+                 INSN_SEQNO (insn) = prev_seqno + 1;
+           }
+       }
+
       /* It can turn out that after removing unused jump, basic block
          that contained that jump, becomes empty too.  In such case
          remove it too.  */
diff --git a/gcc/testsuite/g++.dg/pr80463.C b/gcc/testsuite/g++.dg/pr80463.C
new file mode 100644
index 00000000000..5614c28ca45
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr80463.C
@@ -0,0 +1,20 @@
+/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+/* { dg-options "-g -fselective-scheduling2 -O2
-fvar-tracking-assignments" } */
+
+int *a;
+int b, c;
+void
+d ()
+{
+  for (int e; c; e++)
+    switch (e)
+      {
+      case 0:
+       a[e] = 1;
+      case 1:
+       b = 2;
+       break;
+      default:
+       a[e] = 3;
+      }
+}
diff --git a/gcc/testsuite/gcc.dg/pr80463.c b/gcc/testsuite/gcc.dg/pr80463.c
new file mode 100644
index 00000000000..cebf2fef1f3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr80463.c
@@ -0,0 +1,54 @@
+/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+/* { dg-options "-g -O2 -fvar-tracking-assignments -fselective-scheduling2
-ftree-loop-vectorize -fnon-call-exceptions -fno-tree-vrp -fno-gcse-lm
-fno-tree-loop-im -fno-reorder-blocks-and-partition -fno-reorder-blocks
-fno-move-loop-invariants -w" } */
+
+short int t2;
+int cd, aa, ft;
+
+void
+dh (void)
+{
+  int qs = 0;
+
+  if (t2 < 1)
+    {
+      int bq = 0;
+
+      while (bq < 1)
+        {
+        }
+
+      while (t2 < 1)
+        {
+          if (t2 == 0)
+            {
+              bq = 0;
+              cd = !!cd;
+            }
+          else
+            {
+              bq = 1;
+              cd = bq > qs;
+            }
+
+          t2 += cd;
+          bq = (t2 / qs) == bq;
+
+          if (aa != ft)
+            {
+              qs %= 0;
+              while (bq != 0)
+                {
+ ro:
+                  ;
+                }
+            }
+
+          ++t2;
+        }
+
+ ia:
+      goto ro;
+    }
+
+  goto ia;
+}
diff --git a/gcc/testsuite/gcc.dg/pr83972.c b/gcc/testsuite/gcc.dg/pr83972.c
new file mode 100644
index 00000000000..b8de42cef0a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr83972.c
@@ -0,0 +1,26 @@
+/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+/* { dg-options "-O1 -fschedule-insns -fselective-scheduling
-fsel-sched-pipelining -fvar-tracking-assignments -funroll-loops
-fno-tree-dominator-opts -w" } */
+
+int s7, p0;
+
+void
+i0 (int ke)
+{
+  while (ke < 1)
+    {
+      if (s7 == 0)
+        p0 = 0;
+      else
+        {
+          if (p0 == 0)
+            s7 = 0;
+
+          if (!!s7 || !!p0)
+            s7 = 0;
+          else
+            p0 = 0;
+        }
+
+      ++ke;
+    }
+}

Re: Fix PR 83913

[Andrey Belevantsev]

On 03.04.2018 19:02, Alexander Monakov wrote:
> On Tue, 3 Apr 2018, Andrey Belevantsev wrote:
> 
>> Hello,
>>
>> This issue ended up being fixed the way different from described in the PR.
>>  We do not want to walk away from the invariant "zero SCHED_TIMES -- insn
>> is not scheduled" even for bookkeeping copies (testing showed it trips over
>> asserts designed to catch this).  Rather we choose merging exprs in the way
>> the larger sched-times wins.
> 
> ... but the Changelog and the actual patch take the average rather than the
> maximum sched-time? :)  I believe either way would be acceptable, but please
> clarify the intent.

Sorry, the average is the intent.  Just to have a bit more of pipelining
chances.

Andrey

> 
> Alexander
> 

Re: Fix PR 83530

[Alexander Monakov]

On Tue, 3 Apr 2018, Andrey Belevantsev wrote:

> Hello,
> 
> This issue is when we cannot correctly make insn tick data for the
> unscheduled code (but processed by the modulo scheduler).  Fixed by closely
> following usual scheduling process as described in the PR.

This is ok with the following nit-picks fixed.

> 2018-04-03  Andrey Belevantsev  <abel@ispras.ru>
> 
> 	PR rtl-optimization/83530
> 
> 	* sel-sched.c (force_next_insn): New global variable.
> 	(remove_insn_for_debug): When force_next_insn is true, also leave only
> next insn
> 	in the ready list.
> 	(sel_sched_region): When the region wasn't scheduled, make another pass
> over it
> 	with force_next_insn set to 1.

Overlong lines.

> 	* gcc.dg/pr8350.c: New test.

Typo in test name.
 
> --- a/gcc/sel-sched.c
> +++ b/gcc/sel-sched.c
> @@ -5004,12 +5004,16 @@ remove_temp_moveop_nops (bool full_tidying)
>     distinguishing between bookkeeping copies and original insns.  */
>  static int max_uid_before_move_op = 0;
> 
> +/* When true, we're always scheduling next insn on the already scheduled code
> +   to get the right insn data for the following bundling or other passes.  */
> +static int force_next_insn = 0;
> +
>  /* Remove from AV_VLIW_P all instructions but next when debug counter
>     tells us so.  Next instruction is fetched from BNDS.  */
>  static void
>  remove_insns_for_debug (blist_t bnds, av_set_t *av_vliw_p)
>  {
> -  if (! dbg_cnt (sel_sched_insn_cnt))
> +  if (! dbg_cnt (sel_sched_insn_cnt) || force_next_insn)
>      /* Leave only the next insn in av_vliw.  */
>      {
>        av_set_iterator av_it;
> @@ -7642,7 +7646,13 @@ sel_sched_region (int rgn)
>      sel_sched_region_1 ();
>    else
>      /* Force initialization of INSN_SCHED_CYCLEs for correct bundling.  */

I believe this comment needs updating.

Please also consider moving both assignments of reset_sched_cycles_p to
after the if-else statement, just before sel_region_finish call.

> -    reset_sched_cycles_p = true;
> +    {
> +      reset_sched_cycles_p = false;
> +      pipelining_p = false;
> +      force_next_insn = 1;
> +      sel_sched_region_1 ();
> +      force_next_insn = 0;
> +    }
> 
>    sel_region_finish (reset_sched_cycles_p);
>  }

Re: Fix PR 83962

[Alexander Monakov]

On Tue, 3 Apr 2018, Andrey Belevantsev wrote:

> Hello,
> 
> This issues is about the correct order in which we need to call the
> routines that fix up the control flow for us.

OK with formatting both in the new comment and the Changelog fixed.

> Best,
> Andrey
> 
> 2018-04-03  Andrey Belevantsev  <abel@ispras.ru>
> 
> 	PR rtl-optimization/83962
> 
> 	* sel-sched-ir.c (tidy_control_flow): Correct the order in which we call
> tidy_fallthru_edge
> 	and tidy_control_flow.
> 
> 	* gcc.dg/pr83962.c: New test.
> 
> diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
> index a965d2ec42f..f6de96a7f3d 100644
> --- a/gcc/sel-sched-ir.c
> +++ b/gcc/sel-sched-ir.c
> @@ -3839,9 +3839,13 @@ tidy_control_flow (basic_block xbb, bool full_tidying)
>        && INSN_SCHED_TIMES (BB_END (xbb)) == 0
>        && !IN_CURRENT_FENCE_P (BB_END (xbb)))
>      {
> -      if (sel_remove_insn (BB_END (xbb), false, false))
> -        return true;
> +      /* We used to call sel_remove_insn here that can trigger
> tidy_control_flow
> +         before we fix up the fallthru edge.  Correct that ordering by
> +        explicitly doing the latter before the former.  */
> +      clear_expr (INSN_EXPR (BB_END (xbb)));
>        tidy_fallthru_edge (EDGE_SUCC (xbb, 0));
> +      if (tidy_control_flow (xbb, false))
> +       return true;
>      }
> 
>    first = sel_bb_head (xbb);
> diff --git a/gcc/testsuite/gcc.dg/pr83962.c b/gcc/testsuite/gcc.dg/pr83962.c
> new file mode 100644
> index 00000000000..0547e218715
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr83962.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
> +/* { dg-options "-std=gnu99 -O1 -fselective-scheduling2 -fschedule-insns2
> -fcse-follow-jumps -fno-ssa-phiopt -fno-guess-branch-probability" } */
> +unsigned int ca;
> +
> +void
> +v6 (long long unsigned int as, int p9)
> +{
> +  while (p9 < 1)
> +    as = (as != ca) || (as > 1);
> +}
> 

Re: Fix PR 83913

[Alexander Monakov]

On Tue, 3 Apr 2018, Andrey Belevantsev wrote:

> Hello,
> 
> This issue ended up being fixed the way different from described in the PR.
>  We do not want to walk away from the invariant "zero SCHED_TIMES -- insn
> is not scheduled" even for bookkeeping copies (testing showed it trips over
> asserts designed to catch this).  Rather we choose merging exprs in the way
> the larger sched-times wins.

My understanding is this is not a "complete" solution to the problem, and a
chance for a similar blowup on some other testcase remains. Still, avoiding
picking the minimum sched-times value should be a good mitigation.

Please consider adding a comment that the average sched-times value is taken
as a compromise to thwart "endless" pipelining of bookkeeping-producing insns
available anywhere vs. pipelining of useful insns, or something like that?

OK with that considered/added.

> 
> Best,
> Andrey
> 
> 2018-04-03  Andrey Belevantsev  <abel@ispras.ru>
> 
> 	PR rtl-optimization/83913
> 
> 	* sel-sched-ir.c (merge_expr_data): Choose the middle between two
> different sched-times
> 	when merging exprs.
> 
> 	* gcc.dg/pr83913.c: New test.
> 
> diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
> index a965d2ec42f..f6de96a7f3d 100644
> --- a/gcc/sel-sched-ir.c
> +++ b/gcc/sel-sched-ir.c
> @@ -1837,8 +1837,9 @@ merge_expr_data (expr_t to, expr_t from, insn_t
> split_point)
>    if (EXPR_PRIORITY (to) < EXPR_PRIORITY (from))
>      EXPR_PRIORITY (to) = EXPR_PRIORITY (from);
> 
> -  if (EXPR_SCHED_TIMES (to) > EXPR_SCHED_TIMES (from))
> -    EXPR_SCHED_TIMES (to) = EXPR_SCHED_TIMES (from);
> +  if (EXPR_SCHED_TIMES (to) != EXPR_SCHED_TIMES (from))
> +    EXPR_SCHED_TIMES (to) = ((EXPR_SCHED_TIMES (from) + EXPR_SCHED_TIMES (to)
> +                             + 1) / 2);
> 
>    if (EXPR_ORIG_BB_INDEX (to) != EXPR_ORIG_BB_INDEX (from))
>      EXPR_ORIG_BB_INDEX (to) = 0;
> @@ -3293,11 +3294,22 @@ has_dependence_note_mem_dep (rtx mem ATTRIBUTE_UNUSED,
> 
>  /* Note a dependence.  */
>  static void
> diff --git a/gcc/testsuite/gcc.dg/pr83913.c b/gcc/testsuite/gcc.dg/pr83913.c
> new file mode 100644
> index 00000000000..c898d71a261
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr83913.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
> +/* { dg-options "-O2 -funroll-all-loops -fselective-scheduling
> -fsel-sched-pipelining -fschedule-insns -fno-dce -fno-forward-propagate
> -fno-rerun-cse-after-loop -fno-web" } */
> +
> +int jo, z4;
> +
> +int
> +be (long unsigned int l7, int nt)
> +{
> +  int en;
> +
> +  jo = l7;
> +  for (en = 0; en < 24; ++en)
> +    {
> +      jo = (jo / z4) * (!!jo >= ((!!nt) & 2));
> +      if (jo == 0)
> +        nt = 0;
> +      else
> +        {
> +          nt = z4;
> +          ++z4;
> +          nt = (long unsigned int) nt == (l7 + 1);
> +        }
> +    }
> +
> +  return nt;
> +}
> 
> 
> 

Re: Fix PRs 80463, 83972, 83480

[Alexander Monakov]

On Tue, 3 Apr 2018, Andrey Belevantsev wrote:

> Hello,
> 
> In these PRs we deal with the dependencies we are forced to make between a
> debug insn and its previous insn (unless bb head).  In the latter case, if
> such an insn has been moved, we fixup its movement so it aligns with the
> sel-sched invariants.  We also carefully adjust seqnos in the case we had a
> single debug insn left in the block.

This is OK with overlong lines fixed and the new comment reworded for clarity
(see below).

> Best,
> Andrey
> 
> 2018-04-03  Andrey Belevantsev  <abel@ispras.ru>
> 
> 	PR rtl-optimization/80463
> 	PR rtl-optimization/83972
> 	PR rtl-optimization/83480
> 
> 	* sel-sched-ir.c (has_dependence_note_mem_dep): Take into account the
> correct producer for the insn.
> 	(tidy_control_flow): Fixup seqnos in case of debug insns.
> 
> 	* gcc.dg/pr80463.c: New test.
> 	* g++.dg/pr80463.C: Likewise.
> 	* gcc.dg/pr83972.c: Likewise.
> 
> diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
> index a965d2ec42f..f6de96a7f3d 100644
> --- a/gcc/sel-sched-ir.c
> +++ b/gcc/sel-sched-ir.c
> @@ -3293,11 +3293,22 @@ has_dependence_note_mem_dep (rtx mem ATTRIBUTE_UNUSED,
> 
>  /* Note a dependence.  */
>  static void
> -has_dependence_note_dep (insn_t pro ATTRIBUTE_UNUSED,
> +has_dependence_note_dep (insn_t pro,
>                          ds_t ds ATTRIBUTE_UNUSED)
>  {
> -  if (!sched_insns_conditions_mutex_p (has_dependence_data.pro,
> -                                      VINSN_INSN_RTX
> (has_dependence_data.con)))
> +  insn_t real_pro = has_dependence_data.pro;
> +  insn_t real_con = VINSN_INSN_RTX (has_dependence_data.con);
> +
> +  /* We do not allow for debug insns to move through others unless they
> +     are at the start of bb.  Such insns may create bookkeeping copies
> +     that would not be able to move up breaking sel-sched invariants.

I have trouble parsing this, it seems a word is accidentally between "move up"
and "breaking". Also the "such" is a bit ambiguous.

> +     Detect that here and allow that movement if we allowed it before
> +     in the first place.  */
> +  if (DEBUG_INSN_P (real_con)
> +      && INSN_UID (NEXT_INSN (pro)) == INSN_UID (real_con))
> +    return;

Should we be concerned about debug insns appearing in sequence here? E.g. if
pro and real_con are not consecutive, but all insns in between are debug insns?

> +
> +  if (!sched_insns_conditions_mutex_p (real_pro, real_con))
>      {
>        ds_t *dsp = &has_dependence_data.has_dep_p[has_dependence_data.where];
> 
> @@ -3890,6 +3905,19 @@ tidy_control_flow (basic_block xbb, bool full_tidying)
> 
>        gcc_assert (EDGE_SUCC (xbb->prev_bb, 0)->flags & EDGE_FALLTHRU);
> 
> +      /* We could have skipped some debug insns which did not get removed
> with the block,
> +         and the seqnos could become incorrect.  Fix them up here.  */
> +      if (MAY_HAVE_DEBUG_INSNS && (sel_bb_head (xbb) != first ||
> sel_bb_end (xbb) != last))
> +       {
> +         if (!sel_bb_empty_p (xbb->prev_bb))
> +           {
> +             int prev_seqno = INSN_SEQNO (sel_bb_end (xbb->prev_bb));
> +             if (prev_seqno > INSN_SEQNO (sel_bb_head (xbb)))
> +               for (insn_t insn = sel_bb_head (xbb); insn != first; insn =
> NEXT_INSN (insn))
> +                 INSN_SEQNO (insn) = prev_seqno + 1;
> +           }
> +       }
> +
>        /* It can turn out that after removing unused jump, basic block
>           that contained that jump, becomes empty too.  In such case
>           remove it too.  */
> diff --git a/gcc/testsuite/g++.dg/pr80463.C b/gcc/testsuite/g++.dg/pr80463.C
> new file mode 100644
> index 00000000000..5614c28ca45
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr80463.C
> @@ -0,0 +1,20 @@
> +/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
> +/* { dg-options "-g -fselective-scheduling2 -O2
> -fvar-tracking-assignments" } */
> +
> +int *a;
> +int b, c;
> +void
> +d ()
> +{
> +  for (int e; c; e++)
> +    switch (e)
> +      {
> +      case 0:
> +       a[e] = 1;
> +      case 1:
> +       b = 2;
> +       break;
> +      default:
> +       a[e] = 3;
> +      }
> +}
> diff --git a/gcc/testsuite/gcc.dg/pr80463.c b/gcc/testsuite/gcc.dg/pr80463.c
> new file mode 100644
> index 00000000000..cebf2fef1f3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr80463.c
> @@ -0,0 +1,54 @@
> +/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
> +/* { dg-options "-g -O2 -fvar-tracking-assignments -fselective-scheduling2
> -ftree-loop-vectorize -fnon-call-exceptions -fno-tree-vrp -fno-gcse-lm
> -fno-tree-loop-im -fno-reorder-blocks-and-partition -fno-reorder-blocks
> -fno-move-loop-invariants -w" } */
> +
> +short int t2;
> +int cd, aa, ft;
> +
> +void
> +dh (void)
> +{
> +  int qs = 0;
> +
> +  if (t2 < 1)
> +    {
> +      int bq = 0;
> +
> +      while (bq < 1)
> +        {
> +        }
> +
> +      while (t2 < 1)
> +        {
> +          if (t2 == 0)
> +            {
> +              bq = 0;
> +              cd = !!cd;
> +            }
> +          else
> +            {
> +              bq = 1;
> +              cd = bq > qs;
> +            }
> +
> +          t2 += cd;
> +          bq = (t2 / qs) == bq;
> +
> +          if (aa != ft)
> +            {
> +              qs %= 0;
> +              while (bq != 0)
> +                {
> + ro:
> +                  ;
> +                }
> +            }
> +
> +          ++t2;
> +        }
> +
> + ia:
> +      goto ro;
> +    }
> +
> +  goto ia;
> +}
> diff --git a/gcc/testsuite/gcc.dg/pr83972.c b/gcc/testsuite/gcc.dg/pr83972.c
> new file mode 100644
> index 00000000000..b8de42cef0a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr83972.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
> +/* { dg-options "-O1 -fschedule-insns -fselective-scheduling
> -fsel-sched-pipelining -fvar-tracking-assignments -funroll-loops
> -fno-tree-dominator-opts -w" } */
> +
> +int s7, p0;
> +
> +void
> +i0 (int ke)
> +{
> +  while (ke < 1)
> +    {
> +      if (s7 == 0)
> +        p0 = 0;
> +      else
> +        {
> +          if (p0 == 0)
> +            s7 = 0;
> +
> +          if (!!s7 || !!p0)
> +            s7 = 0;
> +          else
> +            p0 = 0;
> +        }
> +
> +      ++ke;
> +    }
> +}
> 
> 
> 
> 
> 
> 

Re: Fix PR 83530

[Andrey Belevantsev]

[-- Attachment #1: Type: text/plain, Size: 2399 bytes --]

On 06.04.2018 18:55, Alexander Monakov wrote:
> On Tue, 3 Apr 2018, Andrey Belevantsev wrote:
> 
>> Hello,
>>
>> This issue is when we cannot correctly make insn tick data for the
>> unscheduled code (but processed by the modulo scheduler).  Fixed by closely
>> following usual scheduling process as described in the PR.
> 
> This is ok with the following nit-picks fixed.

Thank, I've committed the attached.

Andrey

> 
>> 2018-04-03  Andrey Belevantsev  <abel@ispras.ru>
>>
>> 	PR rtl-optimization/83530
>>
>> 	* sel-sched.c (force_next_insn): New global variable.
>> 	(remove_insn_for_debug): When force_next_insn is true, also leave only
>> next insn
>> 	in the ready list.
>> 	(sel_sched_region): When the region wasn't scheduled, make another pass
>> over it
>> 	with force_next_insn set to 1.
> 
> Overlong lines.
> 
>> 	* gcc.dg/pr8350.c: New test.
> 
> Typo in test name.
>  
>> --- a/gcc/sel-sched.c
>> +++ b/gcc/sel-sched.c
>> @@ -5004,12 +5004,16 @@ remove_temp_moveop_nops (bool full_tidying)
>>     distinguishing between bookkeeping copies and original insns.  */
>>  static int max_uid_before_move_op = 0;
>>
>> +/* When true, we're always scheduling next insn on the already scheduled code
>> +   to get the right insn data for the following bundling or other passes.  */
>> +static int force_next_insn = 0;
>> +
>>  /* Remove from AV_VLIW_P all instructions but next when debug counter
>>     tells us so.  Next instruction is fetched from BNDS.  */
>>  static void
>>  remove_insns_for_debug (blist_t bnds, av_set_t *av_vliw_p)
>>  {
>> -  if (! dbg_cnt (sel_sched_insn_cnt))
>> +  if (! dbg_cnt (sel_sched_insn_cnt) || force_next_insn)
>>      /* Leave only the next insn in av_vliw.  */
>>      {
>>        av_set_iterator av_it;
>> @@ -7642,7 +7646,13 @@ sel_sched_region (int rgn)
>>      sel_sched_region_1 ();
>>    else
>>      /* Force initialization of INSN_SCHED_CYCLEs for correct bundling.  */
> 
> I believe this comment needs updating.
> 
> Please also consider moving both assignments of reset_sched_cycles_p to
> after the if-else statement, just before sel_region_finish call.
> 
>> -    reset_sched_cycles_p = true;
>> +    {
>> +      reset_sched_cycles_p = false;
>> +      pipelining_p = false;
>> +      force_next_insn = 1;
>> +      sel_sched_region_1 ();
>> +      force_next_insn = 0;
>> +    }
>>
>>    sel_region_finish (reset_sched_cycles_p);
>>  }


[-- Attachment #2: pr83530.diff --]
[-- Type: text/plain, Size: 3778 bytes --]

Index: gcc/ChangeLog
===================================================================
*** gcc/ChangeLog	(revision 259227)
--- gcc/ChangeLog	(revision 259228)
***************
*** 1,3 ****
--- 1,13 ----
+ 2018-04-09  Andrey Belevantsev  <abel@ispras.ru>
+ 
+ 	PR rtl-optimization/83530
+ 
+ 	* sel-sched.c (force_next_insn): New global variable.
+ 	(remove_insn_for_debug): When force_next_insn is true, also leave only
+ 	next insn in the ready list.
+ 	(sel_sched_region): When the region wasn't scheduled, make another pass
+ 	over it with force_next_insn set to 1.
+ 
  2018-04-08  Monk Chiang  <sh.chiang04@gmail.com>
  
  	* config.gcc (nds32le-*-*, nds32be-*-*): Add nds32/nds32_intrinsic.h
Index: gcc/testsuite/gcc.dg/pr83530.c
===================================================================
*** gcc/testsuite/gcc.dg/pr83530.c	(revision 0)
--- gcc/testsuite/gcc.dg/pr83530.c	(revision 259228)
***************
*** 0 ****
--- 1,15 ----
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+ /* { dg-options "-O2 -fmodulo-sched -fselective-scheduling2" } */
+ int vm, z0;
+ short int mz;
+ 
+ int
+ ny (void)
+ {
+   int ch;
+ 
+   for (ch = 0; ch < 6; ++ch)
+     vm += ch / vm;
+ 
+   return z0 + mz;
+ }
Index: gcc/testsuite/ChangeLog
===================================================================
*** gcc/testsuite/ChangeLog	(revision 259227)
--- gcc/testsuite/ChangeLog	(revision 259228)
***************
*** 1,3 ****
--- 1,8 ----
+ 2018-04-09  Andrey Belevantsev  <abel@ispras.ru>
+ 
+ 	PR rtl-optimization/83530
+ 	* gcc.dg/pr83530.c: New test.
+ 
  2018-04-07  Thomas Koenig  <tkoenig@gcc.gnu.org>
  
  	PR middle-end/82976
Index: gcc/sel-sched.c
===================================================================
*** gcc/sel-sched.c	(revision 259227)
--- gcc/sel-sched.c	(revision 259228)
*************** remove_temp_moveop_nops (bool full_tidyi
*** 5004,5015 ****
     distinguishing between bookkeeping copies and original insns.  */
  static int max_uid_before_move_op = 0;
  
  /* Remove from AV_VLIW_P all instructions but next when debug counter
     tells us so.  Next instruction is fetched from BNDS.  */
  static void
  remove_insns_for_debug (blist_t bnds, av_set_t *av_vliw_p)
  {
!   if (! dbg_cnt (sel_sched_insn_cnt))
      /* Leave only the next insn in av_vliw.  */
      {
        av_set_iterator av_it;
--- 5004,5019 ----
     distinguishing between bookkeeping copies and original insns.  */
  static int max_uid_before_move_op = 0;
  
+ /* When true, we're always scheduling next insn on the already scheduled code
+    to get the right insn data for the following bundling or other passes.  */
+ static int force_next_insn = 0;
+ 
  /* Remove from AV_VLIW_P all instructions but next when debug counter
     tells us so.  Next instruction is fetched from BNDS.  */
  static void
  remove_insns_for_debug (blist_t bnds, av_set_t *av_vliw_p)
  {
!   if (! dbg_cnt (sel_sched_insn_cnt) || force_next_insn)
      /* Leave only the next insn in av_vliw.  */
      {
        av_set_iterator av_it;
*************** sel_sched_region (int rgn)
*** 7641,7649 ****
    if (schedule_p)
      sel_sched_region_1 ();
    else
!     /* Force initialization of INSN_SCHED_CYCLEs for correct bundling.  */
!     reset_sched_cycles_p = true;
! 
    sel_region_finish (reset_sched_cycles_p);
  }
  
--- 7645,7659 ----
    if (schedule_p)
      sel_sched_region_1 ();
    else
!     {
!       /* Schedule always selecting the next insn to make the correct data
! 	 for bundling or other later passes.  */
!       pipelining_p = false;
!       force_next_insn = 1;
!       sel_sched_region_1 ();
!       force_next_insn = 0;
!     }
!   reset_sched_cycles_p = pipelining_p;
    sel_region_finish (reset_sched_cycles_p);
  }
  

Re: Fix PR 83962

[Andrey Belevantsev]

On 06.04.2018 18:59, Alexander Monakov wrote:
> On Tue, 3 Apr 2018, Andrey Belevantsev wrote:
> 
>> Hello,
>>
>> This issues is about the correct order in which we need to call the
>> routines that fix up the control flow for us.
> 
> OK with formatting both in the new comment and the Changelog fixed.

Thanks, fixed that in rev. 259229.

Andrey

> 
>> Best,
>> Andrey
>>
>> 2018-04-03  Andrey Belevantsev  <abel@ispras.ru>
>>
>> 	PR rtl-optimization/83962
>>
>> 	* sel-sched-ir.c (tidy_control_flow): Correct the order in which we call
>> tidy_fallthru_edge
>> 	and tidy_control_flow.
>>
>> 	* gcc.dg/pr83962.c: New test.
>>
>> diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
>> index a965d2ec42f..f6de96a7f3d 100644
>> --- a/gcc/sel-sched-ir.c
>> +++ b/gcc/sel-sched-ir.c
>> @@ -3839,9 +3839,13 @@ tidy_control_flow (basic_block xbb, bool full_tidying)
>>        && INSN_SCHED_TIMES (BB_END (xbb)) == 0
>>        && !IN_CURRENT_FENCE_P (BB_END (xbb)))
>>      {
>> -      if (sel_remove_insn (BB_END (xbb), false, false))
>> -        return true;
>> +      /* We used to call sel_remove_insn here that can trigger
>> tidy_control_flow
>> +         before we fix up the fallthru edge.  Correct that ordering by
>> +        explicitly doing the latter before the former.  */
>> +      clear_expr (INSN_EXPR (BB_END (xbb)));
>>        tidy_fallthru_edge (EDGE_SUCC (xbb, 0));
>> +      if (tidy_control_flow (xbb, false))
>> +       return true;
>>      }
>>
>>    first = sel_bb_head (xbb);
>> diff --git a/gcc/testsuite/gcc.dg/pr83962.c b/gcc/testsuite/gcc.dg/pr83962.c
>> new file mode 100644
>> index 00000000000..0547e218715
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/pr83962.c
>> @@ -0,0 +1,10 @@
>> +/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
>> +/* { dg-options "-std=gnu99 -O1 -fselective-scheduling2 -fschedule-insns2
>> -fcse-follow-jumps -fno-ssa-phiopt -fno-guess-branch-probability" } */
>> +unsigned int ca;
>> +
>> +void
>> +v6 (long long unsigned int as, int p9)
>> +{
>> +  while (p9 < 1)
>> +    as = (as != ca) || (as > 1);
>> +}
>>

Re: Fix PR 83913

[Andrey Belevantsev]

[-- Attachment #1: Type: text/plain, Size: 3302 bytes --]

On 06.04.2018 19:10, Alexander Monakov wrote:
> On Tue, 3 Apr 2018, Andrey Belevantsev wrote:
> 
>> Hello,
>>
>> This issue ended up being fixed the way different from described in the PR.
>>  We do not want to walk away from the invariant "zero SCHED_TIMES -- insn
>> is not scheduled" even for bookkeeping copies (testing showed it trips over
>> asserts designed to catch this).  Rather we choose merging exprs in the way
>> the larger sched-times wins.
> 
> My understanding is this is not a "complete" solution to the problem, and a
> chance for a similar blowup on some other testcase remains. Still, avoiding
> picking the minimum sched-times value should be a good mitigation.

Well, it's not much different with any other situation when we pose a limit
on pipelining with the sched-times values.  At least for now I can't think
of something better.

Adjusted the comment as per your suggestion and committed the attached patch.

Andrey

> 
> Please consider adding a comment that the average sched-times value is taken
> as a compromise to thwart "endless" pipelining of bookkeeping-producing insns
> available anywhere vs. pipelining of useful insns, or something like that?
> 
> OK with that considered/added.
> 
>>
>> Best,
>> Andrey
>>
>> 2018-04-03  Andrey Belevantsev  <abel@ispras.ru>
>>
>> 	PR rtl-optimization/83913
>>
>> 	* sel-sched-ir.c (merge_expr_data): Choose the middle between two
>> different sched-times
>> 	when merging exprs.
>>
>> 	* gcc.dg/pr83913.c: New test.
>>
>> diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
>> index a965d2ec42f..f6de96a7f3d 100644
>> --- a/gcc/sel-sched-ir.c
>> +++ b/gcc/sel-sched-ir.c
>> @@ -1837,8 +1837,9 @@ merge_expr_data (expr_t to, expr_t from, insn_t
>> split_point)
>>    if (EXPR_PRIORITY (to) < EXPR_PRIORITY (from))
>>      EXPR_PRIORITY (to) = EXPR_PRIORITY (from);
>>
>> -  if (EXPR_SCHED_TIMES (to) > EXPR_SCHED_TIMES (from))
>> -    EXPR_SCHED_TIMES (to) = EXPR_SCHED_TIMES (from);
>> +  if (EXPR_SCHED_TIMES (to) != EXPR_SCHED_TIMES (from))
>> +    EXPR_SCHED_TIMES (to) = ((EXPR_SCHED_TIMES (from) + EXPR_SCHED_TIMES (to)
>> +                             + 1) / 2);
>>
>>    if (EXPR_ORIG_BB_INDEX (to) != EXPR_ORIG_BB_INDEX (from))
>>      EXPR_ORIG_BB_INDEX (to) = 0;
>> @@ -3293,11 +3294,22 @@ has_dependence_note_mem_dep (rtx mem ATTRIBUTE_UNUSED,
>>
>>  /* Note a dependence.  */
>>  static void
>> diff --git a/gcc/testsuite/gcc.dg/pr83913.c b/gcc/testsuite/gcc.dg/pr83913.c
>> new file mode 100644
>> index 00000000000..c898d71a261
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/pr83913.c
>> @@ -0,0 +1,26 @@
>> +/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
>> +/* { dg-options "-O2 -funroll-all-loops -fselective-scheduling
>> -fsel-sched-pipelining -fschedule-insns -fno-dce -fno-forward-propagate
>> -fno-rerun-cse-after-loop -fno-web" } */
>> +
>> +int jo, z4;
>> +
>> +int
>> +be (long unsigned int l7, int nt)
>> +{
>> +  int en;
>> +
>> +  jo = l7;
>> +  for (en = 0; en < 24; ++en)
>> +    {
>> +      jo = (jo / z4) * (!!jo >= ((!!nt) & 2));
>> +      if (jo == 0)
>> +        nt = 0;
>> +      else
>> +        {
>> +          nt = z4;
>> +          ++z4;
>> +          nt = (long unsigned int) nt == (l7 + 1);
>> +        }
>> +    }
>> +
>> +  return nt;
>> +}
>>
>>
>>


[-- Attachment #2: pr83913.diff --]
[-- Type: text/plain, Size: 3121 bytes --]

Index: gcc/ChangeLog
===================================================================
*** gcc/ChangeLog	(revision 259229)
--- gcc/ChangeLog	(revision 259230)
***************
*** 1,5 ****
--- 1,12 ----
  2018-04-09  Andrey Belevantsev  <abel@ispras.ru>
  
+ 	PR rtl-optimization/83913
+ 
+ 	* sel-sched-ir.c (merge_expr_data): Choose the middle between two
+ 	different sched-times when merging exprs.
+ 
+ 2018-04-09  Andrey Belevantsev  <abel@ispras.ru>
+ 
  	PR rtl-optimization/83962
  
  	* sel-sched-ir.c (tidy_control_flow): Correct the order in which we call
Index: gcc/testsuite/gcc.dg/pr83913.c
===================================================================
*** gcc/testsuite/gcc.dg/pr83913.c	(revision 0)
--- gcc/testsuite/gcc.dg/pr83913.c	(revision 259230)
***************
*** 0 ****
--- 1,26 ----
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+ /* { dg-options "-O2 -funroll-all-loops -fselective-scheduling -fsel-sched-pipelining -fschedule-insns -fno-dce -fno-forward-propagate -fno-rerun-cse-after-loop -fno-web" } */
+ 
+ int jo, z4;
+ 
+ int
+ be (long unsigned int l7, int nt)
+ {
+   int en;
+ 
+   jo = l7;
+   for (en = 0; en < 24; ++en)
+     {
+       jo = (jo / z4) * (!!jo >= ((!!nt) & 2));
+       if (jo == 0)
+         nt = 0;
+       else
+         {
+           nt = z4;
+           ++z4;
+           nt = (long unsigned int) nt == (l7 + 1);
+         }
+     }
+ 
+   return nt;
+ }
Index: gcc/testsuite/ChangeLog
===================================================================
*** gcc/testsuite/ChangeLog	(revision 259229)
--- gcc/testsuite/ChangeLog	(revision 259230)
***************
*** 1,5 ****
--- 1,10 ----
  2018-04-09  Andrey Belevantsev  <abel@ispras.ru>
  
+ 	PR rtl-optimization/83913
+ 	* gcc.dg/pr83913.c: New test.
+ 
+ 2018-04-09  Andrey Belevantsev  <abel@ispras.ru>
+ 
  	PR rtl-optimization/83962
  	* gcc.dg/pr83962.c: New test.
  
Index: gcc/sel-sched-ir.c
===================================================================
*** gcc/sel-sched-ir.c	(revision 259229)
--- gcc/sel-sched-ir.c	(revision 259230)
*************** merge_expr_data (expr_t to, expr_t from,
*** 1837,1844 ****
    if (EXPR_PRIORITY (to) < EXPR_PRIORITY (from))
      EXPR_PRIORITY (to) = EXPR_PRIORITY (from);
  
!   if (EXPR_SCHED_TIMES (to) > EXPR_SCHED_TIMES (from))
!     EXPR_SCHED_TIMES (to) = EXPR_SCHED_TIMES (from);
  
    if (EXPR_ORIG_BB_INDEX (to) != EXPR_ORIG_BB_INDEX (from))
      EXPR_ORIG_BB_INDEX (to) = 0;
--- 1837,1848 ----
    if (EXPR_PRIORITY (to) < EXPR_PRIORITY (from))
      EXPR_PRIORITY (to) = EXPR_PRIORITY (from);
  
!   /* We merge sched-times half-way to the larger value to avoid the endless
!      pipelining of unneeded insns.  The average seems to be good compromise
!      between pipelining opportunities and avoiding extra work.  */
!   if (EXPR_SCHED_TIMES (to) != EXPR_SCHED_TIMES (from))
!     EXPR_SCHED_TIMES (to) = ((EXPR_SCHED_TIMES (from) + EXPR_SCHED_TIMES (to)
!                              + 1) / 2);
  
    if (EXPR_ORIG_BB_INDEX (to) != EXPR_ORIG_BB_INDEX (from))
      EXPR_ORIG_BB_INDEX (to) = 0;

Re: Fix PRs 80463, 83972, 83480

[Andrey Belevantsev]

[-- Attachment #1: Type: text/plain, Size: 6894 bytes --]

On 06.04.2018 19:18, Alexander Monakov wrote:
> On Tue, 3 Apr 2018, Andrey Belevantsev wrote:
> 
>> Hello,
>>
>> In these PRs we deal with the dependencies we are forced to make between a
>> debug insn and its previous insn (unless bb head).  In the latter case, if
>> such an insn has been moved, we fixup its movement so it aligns with the
>> sel-sched invariants.  We also carefully adjust seqnos in the case we had a
>> single debug insn left in the block.
> 
> This is OK with overlong lines fixed and the new comment reworded for clarity
> (see below).
> 
>> Best,
>> Andrey
>>
>> 2018-04-03  Andrey Belevantsev  <abel@ispras.ru>
>>
>> 	PR rtl-optimization/80463
>> 	PR rtl-optimization/83972
>> 	PR rtl-optimization/83480
>>
>> 	* sel-sched-ir.c (has_dependence_note_mem_dep): Take into account the
>> correct producer for the insn.
>> 	(tidy_control_flow): Fixup seqnos in case of debug insns.
>>
>> 	* gcc.dg/pr80463.c: New test.
>> 	* g++.dg/pr80463.C: Likewise.
>> 	* gcc.dg/pr83972.c: Likewise.
>>
>> diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
>> index a965d2ec42f..f6de96a7f3d 100644
>> --- a/gcc/sel-sched-ir.c
>> +++ b/gcc/sel-sched-ir.c
>> @@ -3293,11 +3293,22 @@ has_dependence_note_mem_dep (rtx mem ATTRIBUTE_UNUSED,
>>
>>  /* Note a dependence.  */
>>  static void
>> -has_dependence_note_dep (insn_t pro ATTRIBUTE_UNUSED,
>> +has_dependence_note_dep (insn_t pro,
>>                          ds_t ds ATTRIBUTE_UNUSED)
>>  {
>> -  if (!sched_insns_conditions_mutex_p (has_dependence_data.pro,
>> -                                      VINSN_INSN_RTX
>> (has_dependence_data.con)))
>> +  insn_t real_pro = has_dependence_data.pro;
>> +  insn_t real_con = VINSN_INSN_RTX (has_dependence_data.con);
>> +
>> +  /* We do not allow for debug insns to move through others unless they
>> +     are at the start of bb.  Such insns may create bookkeeping copies
>> +     that would not be able to move up breaking sel-sched invariants.
> 
> I have trouble parsing this, it seems a word is accidentally between "move up"
> and "breaking". Also the "such" is a bit ambiguous.
> 
>> +     Detect that here and allow that movement if we allowed it before
>> +     in the first place.  */
>> +  if (DEBUG_INSN_P (real_con)
>> +      && INSN_UID (NEXT_INSN (pro)) == INSN_UID (real_con))
>> +    return;
> 
> Should we be concerned about debug insns appearing in sequence here? E.g. if
> pro and real_con are not consecutive, but all insns in between are debug insns?

I think that should be fine, that is, as long as the insn moved up through
all those debug insns, the copy will do that as well.  It's that
problematic conditional in sched-deps.c that we should take care of.

I've reworded the comment and committed the attached patch.  Thanks for
your help.

Andrey

> 
>> +
>> +  if (!sched_insns_conditions_mutex_p (real_pro, real_con))
>>      {
>>        ds_t *dsp = &has_dependence_data.has_dep_p[has_dependence_data.where];
>>
>> @@ -3890,6 +3905,19 @@ tidy_control_flow (basic_block xbb, bool full_tidying)
>>
>>        gcc_assert (EDGE_SUCC (xbb->prev_bb, 0)->flags & EDGE_FALLTHRU);
>>
>> +      /* We could have skipped some debug insns which did not get removed
>> with the block,
>> +         and the seqnos could become incorrect.  Fix them up here.  */
>> +      if (MAY_HAVE_DEBUG_INSNS && (sel_bb_head (xbb) != first ||
>> sel_bb_end (xbb) != last))
>> +       {
>> +         if (!sel_bb_empty_p (xbb->prev_bb))
>> +           {
>> +             int prev_seqno = INSN_SEQNO (sel_bb_end (xbb->prev_bb));
>> +             if (prev_seqno > INSN_SEQNO (sel_bb_head (xbb)))
>> +               for (insn_t insn = sel_bb_head (xbb); insn != first; insn =
>> NEXT_INSN (insn))
>> +                 INSN_SEQNO (insn) = prev_seqno + 1;
>> +           }
>> +       }
>> +
>>        /* It can turn out that after removing unused jump, basic block
>>           that contained that jump, becomes empty too.  In such case
>>           remove it too.  */
>> diff --git a/gcc/testsuite/g++.dg/pr80463.C b/gcc/testsuite/g++.dg/pr80463.C
>> new file mode 100644
>> index 00000000000..5614c28ca45
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/pr80463.C
>> @@ -0,0 +1,20 @@
>> +/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
>> +/* { dg-options "-g -fselective-scheduling2 -O2
>> -fvar-tracking-assignments" } */
>> +
>> +int *a;
>> +int b, c;
>> +void
>> +d ()
>> +{
>> +  for (int e; c; e++)
>> +    switch (e)
>> +      {
>> +      case 0:
>> +       a[e] = 1;
>> +      case 1:
>> +       b = 2;
>> +       break;
>> +      default:
>> +       a[e] = 3;
>> +      }
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/pr80463.c b/gcc/testsuite/gcc.dg/pr80463.c
>> new file mode 100644
>> index 00000000000..cebf2fef1f3
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/pr80463.c
>> @@ -0,0 +1,54 @@
>> +/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
>> +/* { dg-options "-g -O2 -fvar-tracking-assignments -fselective-scheduling2
>> -ftree-loop-vectorize -fnon-call-exceptions -fno-tree-vrp -fno-gcse-lm
>> -fno-tree-loop-im -fno-reorder-blocks-and-partition -fno-reorder-blocks
>> -fno-move-loop-invariants -w" } */
>> +
>> +short int t2;
>> +int cd, aa, ft;
>> +
>> +void
>> +dh (void)
>> +{
>> +  int qs = 0;
>> +
>> +  if (t2 < 1)
>> +    {
>> +      int bq = 0;
>> +
>> +      while (bq < 1)
>> +        {
>> +        }
>> +
>> +      while (t2 < 1)
>> +        {
>> +          if (t2 == 0)
>> +            {
>> +              bq = 0;
>> +              cd = !!cd;
>> +            }
>> +          else
>> +            {
>> +              bq = 1;
>> +              cd = bq > qs;
>> +            }
>> +
>> +          t2 += cd;
>> +          bq = (t2 / qs) == bq;
>> +
>> +          if (aa != ft)
>> +            {
>> +              qs %= 0;
>> +              while (bq != 0)
>> +                {
>> + ro:
>> +                  ;
>> +                }
>> +            }
>> +
>> +          ++t2;
>> +        }
>> +
>> + ia:
>> +      goto ro;
>> +    }
>> +
>> +  goto ia;
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/pr83972.c b/gcc/testsuite/gcc.dg/pr83972.c
>> new file mode 100644
>> index 00000000000..b8de42cef0a
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/pr83972.c
>> @@ -0,0 +1,26 @@
>> +/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
>> +/* { dg-options "-O1 -fschedule-insns -fselective-scheduling
>> -fsel-sched-pipelining -fvar-tracking-assignments -funroll-loops
>> -fno-tree-dominator-opts -w" } */
>> +
>> +int s7, p0;
>> +
>> +void
>> +i0 (int ke)
>> +{
>> +  while (ke < 1)
>> +    {
>> +      if (s7 == 0)
>> +        p0 = 0;
>> +      else
>> +        {
>> +          if (p0 == 0)
>> +            s7 = 0;
>> +
>> +          if (!!s7 || !!p0)
>> +            s7 = 0;
>> +          else
>> +            p0 = 0;
>> +        }
>> +
>> +      ++ke;
>> +    }
>> +}
>>
>>
>>
>>
>>
>>


[-- Attachment #2: prs.diff --]
[-- Type: text/plain, Size: 6558 bytes --]

Index: gcc/ChangeLog
===================================================================
*** gcc/ChangeLog	(revision 259230)
--- gcc/ChangeLog	(revision 259231)
***************
*** 1,5 ****
--- 1,15 ----
  2018-04-09  Andrey Belevantsev  <abel@ispras.ru>
  
+ 	PR rtl-optimization/80463
+ 	PR rtl-optimization/83972
+ 	PR rtl-optimization/83480
+ 
+ 	* sel-sched-ir.c (has_dependence_note_mem_dep): Take into account the
+ 	correct producer for the insn.
+ 	(tidy_control_flow): Fixup seqnos in case of debug insns.
+ 
+ 2018-04-09  Andrey Belevantsev  <abel@ispras.ru>
+ 
  	PR rtl-optimization/83913
  
  	* sel-sched-ir.c (merge_expr_data): Choose the middle between two
Index: gcc/testsuite/gcc.dg/pr83972.c
===================================================================
*** gcc/testsuite/gcc.dg/pr83972.c	(revision 0)
--- gcc/testsuite/gcc.dg/pr83972.c	(revision 259231)
***************
*** 0 ****
--- 1,26 ----
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+ /* { dg-options "-O1 -fschedule-insns -fselective-scheduling -fsel-sched-pipelining -fvar-tracking-assignments -funroll-loops -fno-tree-dominator-opts -w" } */
+ 
+ int s7, p0;
+ 
+ void
+ i0 (int ke)
+ {
+   while (ke < 1)
+     {
+       if (s7 == 0)
+         p0 = 0;
+       else
+         {
+           if (p0 == 0)
+             s7 = 0;
+ 
+           if (!!s7 || !!p0)
+             s7 = 0;
+           else
+             p0 = 0;
+         }
+ 
+       ++ke;
+     }
+ }
Index: gcc/testsuite/gcc.dg/pr80463.c
===================================================================
*** gcc/testsuite/gcc.dg/pr80463.c	(revision 0)
--- gcc/testsuite/gcc.dg/pr80463.c	(revision 259231)
***************
*** 0 ****
--- 1,54 ----
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+ /* { dg-options "-g -O2 -fvar-tracking-assignments -fselective-scheduling2 -ftree-loop-vectorize -fnon-call-exceptions -fno-tree-vrp -fno-gcse-lm -fno-tree-loop-im -fno-reorder-blocks-and-partition -fno-reorder-blocks -fno-move-loop-invariants -w" } */
+ 
+ short int t2;
+ int cd, aa, ft;
+ 
+ void
+ dh (void)
+ {
+   int qs = 0;
+ 
+   if (t2 < 1)
+     {
+       int bq = 0;
+ 
+       while (bq < 1)
+         {
+         }
+ 
+       while (t2 < 1)
+         {
+           if (t2 == 0)
+             {
+               bq = 0;
+               cd = !!cd;
+             }
+           else
+             {
+               bq = 1;
+               cd = bq > qs;
+             }
+ 
+           t2 += cd;
+           bq = (t2 / qs) == bq;
+ 
+           if (aa != ft)
+             {
+               qs %= 0;
+               while (bq != 0)
+                 {
+  ro:
+                   ;
+                 }
+             }
+ 
+           ++t2;
+         }
+ 
+  ia:
+       goto ro;
+     }
+ 
+   goto ia;
+ }
Index: gcc/testsuite/ChangeLog
===================================================================
*** gcc/testsuite/ChangeLog	(revision 259230)
--- gcc/testsuite/ChangeLog	(revision 259231)
***************
*** 1,5 ****
--- 1,15 ----
  2018-04-09  Andrey Belevantsev  <abel@ispras.ru>
  
+ 	PR rtl-optimization/80463
+ 	PR rtl-optimization/83972
+ 	PR rtl-optimization/83480
+ 
+ 	* gcc.dg/pr80463.c: New test.
+ 	* g++.dg/pr80463.C: Likewise.
+ 	* gcc.dg/pr83972.c: Likewise.
+ 
+ 2018-04-09  Andrey Belevantsev  <abel@ispras.ru>
+ 
  	PR rtl-optimization/83913
  	* gcc.dg/pr83913.c: New test.
  
Index: gcc/testsuite/g++.dg/pr80463.C
===================================================================
*** gcc/testsuite/g++.dg/pr80463.C	(revision 0)
--- gcc/testsuite/g++.dg/pr80463.C	(revision 259231)
***************
*** 0 ****
--- 1,20 ----
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+ /* { dg-options "-g -fselective-scheduling2 -O2 -fvar-tracking-assignments" } */
+ 
+ int *a;
+ int b, c;
+ void
+ d ()
+ {
+   for (int e; c; e++)
+     switch (e)
+       {
+       case 0:
+        a[e] = 1;
+       case 1:
+        b = 2;
+        break;
+       default:
+        a[e] = 3;
+       }
+ }
Index: gcc/sel-sched-ir.c
===================================================================
*** gcc/sel-sched-ir.c	(revision 259230)
--- gcc/sel-sched-ir.c	(revision 259231)
*************** has_dependence_note_mem_dep (rtx mem ATT
*** 3297,3307 ****
  
  /* Note a dependence.  */
  static void
! has_dependence_note_dep (insn_t pro ATTRIBUTE_UNUSED,
! 			 ds_t ds ATTRIBUTE_UNUSED)
  {
!   if (!sched_insns_conditions_mutex_p (has_dependence_data.pro,
! 				       VINSN_INSN_RTX (has_dependence_data.con)))
      {
        ds_t *dsp = &has_dependence_data.has_dep_p[has_dependence_data.where];
  
--- 3297,3318 ----
  
  /* Note a dependence.  */
  static void
! has_dependence_note_dep (insn_t pro, ds_t ds ATTRIBUTE_UNUSED)
  {
!   insn_t real_pro = has_dependence_data.pro;
!   insn_t real_con = VINSN_INSN_RTX (has_dependence_data.con);
! 
!   /* We do not allow for debug insns to move through others unless they
!      are at the start of bb.  This movement may create bookkeeping copies
!      that later would not be able to move up, violating the invariant
!      that a bookkeeping copy should be movable as the original insn.
!      Detect that here and allow that movement if we allowed it before
!      in the first place.  */
!   if (DEBUG_INSN_P (real_con)
!       && INSN_UID (NEXT_INSN (pro)) == INSN_UID (real_con))
!     return;
! 
!   if (!sched_insns_conditions_mutex_p (real_pro, real_con))
      {
        ds_t *dsp = &has_dependence_data.has_dep_p[has_dependence_data.where];
  
*************** tidy_control_flow (basic_block xbb, bool
*** 3898,3903 ****
--- 3909,3927 ----
  
        gcc_assert (EDGE_SUCC (xbb->prev_bb, 0)->flags & EDGE_FALLTHRU);
  
+       /* We could have skipped some debug insns which did not get removed with the block,
+          and the seqnos could become incorrect.  Fix them up here.  */
+       if (MAY_HAVE_DEBUG_INSNS && (sel_bb_head (xbb) != first || sel_bb_end (xbb) != last))
+        {
+          if (!sel_bb_empty_p (xbb->prev_bb))
+            {
+              int prev_seqno = INSN_SEQNO (sel_bb_end (xbb->prev_bb));
+              if (prev_seqno > INSN_SEQNO (sel_bb_head (xbb)))
+                for (insn_t insn = sel_bb_head (xbb); insn != first; insn = NEXT_INSN (insn))
+                  INSN_SEQNO (insn) = prev_seqno + 1;
+            }
+        }
+ 
        /* It can turn out that after removing unused jump, basic block
           that contained that jump, becomes empty too.  In such case
           remove it too.  */

Re: Fix PRs 80463, 83972, 83480

[Jakub Jelinek]

On Mon, Apr 09, 2018 at 01:30:12PM +0300, Andrey Belevantsev wrote:
> I think that should be fine, that is, as long as the insn moved up through
> all those debug insns, the copy will do that as well.  It's that
> problematic conditional in sched-deps.c that we should take care of.
> 
> I've reworded the comment and committed the attached patch.  Thanks for
> your help.

The C++ testcase FAILs everywhere:
FAIL: g++.dg/pr80463.C  -std=gnu++98 (test for excess errors)
Excess errors:
cc1plus: warning: var-tracking-assignments changes selective scheduling

The other testcases in the patch used -w probably to disable the same
warning, so I've committed following as obvious to trunk after regtesting it
on x86_64-linux and i686-linux:

2018-04-09  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/80463
	* g++.dg/pr80463.C: Add -w to dg-options.

--- gcc/testsuite/g++.dg/pr80463.C.jj	2018-04-09 20:15:47.226631780 +0200
+++ gcc/testsuite/g++.dg/pr80463.C	2018-04-09 20:19:43.783616136 +0200
@@ -1,5 +1,5 @@
 /* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
-/* { dg-options "-g -fselective-scheduling2 -O2 -fvar-tracking-assignments" } */
+/* { dg-options "-g -fselective-scheduling2 -O2 -fvar-tracking-assignments -w" } */
 
 int *a;
 int b, c;


	Jakub

Re: Fix PRs 80463, 83972, 83480

[Andrey Belevantsev]

On 09.04.2018 21:23, Jakub Jelinek wrote:
> On Mon, Apr 09, 2018 at 01:30:12PM +0300, Andrey Belevantsev wrote:
>> I think that should be fine, that is, as long as the insn moved up through
>> all those debug insns, the copy will do that as well.  It's that
>> problematic conditional in sched-deps.c that we should take care of.
>>
>> I've reworded the comment and committed the attached patch.  Thanks for
>> your help.
> 
> The C++ testcase FAILs everywhere:
> FAIL: g++.dg/pr80463.C  -std=gnu++98 (test for excess errors)
> Excess errors:
> cc1plus: warning: var-tracking-assignments changes selective scheduling
> 
> The other testcases in the patch used -w probably to disable the same
> warning, so I've committed following as obvious to trunk after regtesting it
> on x86_64-linux and i686-linux:

Thank you very much, I have missed this when committing.  Sorry for the noise.

Andrey

> 
> 2018-04-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR rtl-optimization/80463
> 	* g++.dg/pr80463.C: Add -w to dg-options.
> 
> --- gcc/testsuite/g++.dg/pr80463.C.jj	2018-04-09 20:15:47.226631780 +0200
> +++ gcc/testsuite/g++.dg/pr80463.C	2018-04-09 20:19:43.783616136 +0200
> @@ -1,5 +1,5 @@
>  /* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
> -/* { dg-options "-g -fselective-scheduling2 -O2 -fvar-tracking-assignments" } */
> +/* { dg-options "-g -fselective-scheduling2 -O2 -fvar-tracking-assignments -w" } */
>  
>  int *a;
>  int b, c;
> 
> 
> 	Jakub
> 

Add test from PR 83852 (was Re: Fix PR 83962)

[Andrey Belevantsev]

[-- Attachment #1: Type: text/plain, Size: 577 bytes --]

On 09.04.2018 12:16, Andrey Belevantsev wrote:
> On 06.04.2018 18:59, Alexander Monakov wrote:
>> On Tue, 3 Apr 2018, Andrey Belevantsev wrote:
>>
>>> Hello,
>>>
>>> This issues is about the correct order in which we need to call the
>>> routines that fix up the control flow for us.
>>
>> OK with formatting both in the new comment and the Changelog fixed.
> 
> Thanks, fixed that in rev. 259229.

I've found out that this patch also fixes PR 83852, so I've committed the
test from that PR as obvious after verifying that it works on cross-ppc
compiler and on x86-64.

Andrey

[-- Attachment #2: pr83852.diff --]
[-- Type: text/plain, Size: 1305 bytes --]

Index: gcc.dg/pr83852.c
===================================================================
*** gcc.dg/pr83852.c	(revision 0)
--- gcc.dg/pr83852.c	(revision 259373)
***************
*** 0 ****
--- 1,33 ----
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+ /* { dg-options "-std=gnu99 -O2 -fselective-scheduling -fno-if-conversion -fno-tree-dse -w" } */
+ long long int uo;
+ unsigned int vt;
+ 
+ void
+ r5 (long long int h8, long long int pu)
+ {
+   short int wj;
+   long long int *mh = h8;
+ 
+   for (wj = 0; wj < 3; ++wj)
+     {
+       int oq;
+       long long int ns, xf;
+ 
+       h8 += 2;
+       oq = !!h8 && !!wj;
+       ++uo;
+       vt ^= oq + uo;
+       ns = !!uo && !!vt;
+       xf = (h8 != 0) ? mh : 1;
+       pu += ns < xf;
+     }
+ 
+   for (pu = 0; pu < 1; ++pu)
+     {
+       int *sc;
+ 
+       sc = (int *)&pu;
+       *sc = 0;
+     }
+ }
Index: ChangeLog
===================================================================
*** ChangeLog	(revision 259372)
--- ChangeLog	(revision 259373)
***************
*** 1,3 ****
--- 1,8 ----
+ 2018-04-13  Andrey Belevantsev  <abel@ispras.ru>
+ 
+ 	PR rtl-optimization/83852
+ 	* gcc.dg/pr83852.c: New testcase.
+ 
  2018-04-13  Andreas Krebbel  <krebbel@linux.ibm.com>
  
          PR testsuite/85326

Fix PR 85423 (Re: Fix PRs 80463, 83972, 83480)

[Andrey Belevantsev]

[-- Attachment #1: Type: text/plain, Size: 781 bytes --]

Hello,

So this PR shows that I have incorrectly mirrored the conditional from
sched-deps.c that creates the dependence from a debug insn on the previous
insn (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80463#c3 for the
hunk).  Thus we have incorrectly discarded some legitimate debug-debug
dependencies.  The straightforward fix works for all of four PRs, tested on
x86-64.

I have put the test in gcc.dg though it requires -march=nano.  Do you want
me to create an extra machine-dependent test?

Best,
Andrey

2018-04-23  Andrey Belevantsev  <abel@ispras.ru>

        PR rtl-optimization/85423

        * sel-sched-ir.c (has_dependence_note_mem_dep): Only discard
        dependencies to debug insns when previous insn is non-debug.

        * gcc.dg/pr85423.c: New test.


[-- Attachment #2: pr85423.diff --]
[-- Type: text/plain, Size: 1281 bytes --]

diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
index ee970522890..85ff5bd3eb4 100644
--- a/gcc/sel-sched-ir.c
+++ b/gcc/sel-sched-ir.c
@@ -3308,7 +3308,7 @@ has_dependence_note_dep (insn_t pro, ds_t ds ATTRIBUTE_UNUSED)
      that a bookkeeping copy should be movable as the original insn.
      Detect that here and allow that movement if we allowed it before
      in the first place.  */
-  if (DEBUG_INSN_P (real_con)
+  if (DEBUG_INSN_P (real_con) && !DEBUG_INSN_P (real_pro)
       && INSN_UID (NEXT_INSN (pro)) == INSN_UID (real_con))
     return;
 
diff --git a/gcc/testsuite/gcc.dg/pr85423.c b/gcc/testsuite/gcc.dg/pr85423.c
new file mode 100644
index 00000000000..21d4a2eb4b9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr85423.c
@@ -0,0 +1,26 @@
+/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+/* { dg-options "-O2 -fselective-scheduling2 -fvar-tracking-assignments -fno-guess-branch-probability -fno-peephole2 -fno-ssa-phiopt -fno-tree-pre --param max-jump-thread-duplication-stmts=8 -w" } */
+
+int vn, xm;
+
+void
+i1 (int);
+
+void
+mb (int *ap, int ev)
+{
+  while (vn < 1)
+    {
+      i1 (vn);
+
+      ev += *ap && ++vn;
+
+      while (xm < 1)
+        ++xm;
+
+      if (*ap == 0)
+        *ap = ev;
+
+      ++vn;
+    }
+}

Re: Fix PR 85423 (Re: Fix PRs 80463, 83972, 83480)

[Alexander Monakov]

On Mon, 23 Apr 2018, Andrey Belevantsev wrote:

> Hello,
> 
> So this PR shows that I have incorrectly mirrored the conditional from
> sched-deps.c that creates the dependence from a debug insn on the previous
> insn (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80463#c3 for the
> hunk).  Thus we have incorrectly discarded some legitimate debug-debug
> dependencies.  The straightforward fix works for all of four PRs, tested on
> x86-64.
> 
> I have put the test in gcc.dg though it requires -march=nano.  Do you want
> me to create an extra machine-dependent test?

I see Jakub addressed this question (thanks!) so please incorporate his
suggestion.  There's also a typo in the ChangeLog, OK with that fixed.

Alexander

> 2018-04-23  Andrey Belevantsev  <abel@ispras.ru>
> 
>         PR rtl-optimization/85423
> 
>        * sel-sched-ir.c (has_dependence_note_mem_dep): Only discard

Should be "has_dependence_note_dep" (without the "mem_" ;)

Alexander

Re: Fix PR 85423 (Re: Fix PRs 80463, 83972, 83480)

[Jakub Jelinek]

On Mon, Apr 23, 2018 at 05:03:09PM +0300, Andrey Belevantsev wrote:
> I have put the test in gcc.dg though it requires -march=nano.  Do you want
> me to create an extra machine-dependent test?

If it is a compile time test, no need for that.
Just add
/* { dg-additional-options "-march=nano" { target i?86-*-* x86_64-*-* } } */

	Jakub