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 <a...@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;
> + }
> +}
>
>
>
>
>
>
