diff mbox

[02/05] Fix PR 63384

Message ID 693a88a9-8878-b169-2bcf-f35e3b52c859@ispras.ru
State New
Headers show

Commit Message

Andrey Belevantsev March 14, 2016, 9:31 a.m. UTC
Hello,

Here we're looping because we decrease the counter of the insns we still 
can issue on a DEBUG_INSN thus rendering the counter negative.  The fix is 
to not count debug insns in the corresponding code.  The selective 
scheduling is known to spoil the result of var tracking, but still it is 
not the reason to hang in there.

The toggle option used in the test seems to be the equivalent of just 
enabling var-tracking-assignments which should lead to the same situation; 
however, if specified as is, var-tracking-assignments will be disabled by 
the toplev.c:1460 code.  Maybe we also need the same treatment for 
flag_var_tracking_assignments_toggle.

Ok for trunk?

gcc/

2016-03-14  Andrey Belevantsev  <abel@ispras.ru>

     PR rtl-optimization/63384
     * sel-sched.c (invoke_aftermath_hooks): Do not decrease issue_more on 
DEBUG_INSN_P insns.

testsuite/

2016-03-14  Andrey Belevantsev  <abel@ispras.ru>

     PR rtl-optimization/63384
     * testsuite/g++.dg/pr63384.C: New test.

Best,
Andrey

Comments

Marek Polacek March 15, 2016, 5:30 p.m. UTC | #1
On Mon, Mar 14, 2016 at 12:31:24PM +0300, Andrey Belevantsev wrote:
> Hello,
> 
> Here we're looping because we decrease the counter of the insns we still can
> issue on a DEBUG_INSN thus rendering the counter negative.  The fix is to
> not count debug insns in the corresponding code.  The selective scheduling
> is known to spoil the result of var tracking, but still it is not the reason
> to hang in there.
> 
> The toggle option used in the test seems to be the equivalent of just
> enabling var-tracking-assignments which should lead to the same situation;
> however, if specified as is, var-tracking-assignments will be disabled by
> the toplev.c:1460 code.  Maybe we also need the same treatment for
> flag_var_tracking_assignments_toggle.
> 
> Ok for trunk?
> 
> gcc/
> 
> 2016-03-14  Andrey Belevantsev  <abel@ispras.ru>
> 
>     PR rtl-optimization/63384
>     * sel-sched.c (invoke_aftermath_hooks): Do not decrease issue_more on
> DEBUG_INSN_P insns.
> 
> testsuite/
> 
> 2016-03-14  Andrey Belevantsev  <abel@ispras.ru>
> 
>     PR rtl-optimization/63384
>     * testsuite/g++.dg/pr63384.C: New test.
> 
> Best,
> Andrey
> 

> diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
> index c798935..893a3e5 100644
> --- a/gcc/sel-sched.c
> +++ b/gcc/sel-sched.c
> @@ -4249,7 +4249,8 @@ invoke_aftermath_hooks (fence_t fence, rtx_insn *best_insn, int issue_more)
>                                        issue_more);
>        memcpy (FENCE_STATE (fence), curr_state, dfa_state_size);
>      }
> -  else if (GET_CODE (PATTERN (best_insn)) != USE
> +  else if (! DEBUG_INSN_P (best_insn)
> +	   && GET_CODE (PATTERN (best_insn)) != USE
>             && GET_CODE (PATTERN (best_insn)) != CLOBBER)
>      issue_more--;
>  
> diff --git a/gcc/testsuite/g++.dg/pr63384.C b/gcc/testsuite/g++.dg/pr63384.C
> new file mode 100644
> index 0000000..b4e0784
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr63384.C
> @@ -0,0 +1,12 @@
> +/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
> +/* { dg-options "-O2 -fselective-scheduling2 -fsel-sched-pipelining  -fsel-sched-pipelining-outer-loops -fsel-sched-reschedule-pipelined -fvar-tracking-assignments-toggle -ftree-vectorize" } */
> +
> +template <class T> T **make_test_matrix() {
> + T **data = new T *;
> + for (int i = 0; i < 1000; i++)
> +    ;
> +}
> +
> +template <typename T> void test() { T **c = make_test_matrix<T>(); }
> +
> +main() { test<float>(); }

This test fails for me due to
cc1plus: warning: var-tracking-assignments changes selective scheduling

	Marek
Alexander Monakov March 15, 2016, 5:44 p.m. UTC | #2
On Tue, 15 Mar 2016, Marek Polacek wrote:
> This test fails for me due to
> cc1plus: warning: var-tracking-assignments changes selective scheduling

Thanks for the heads-up Marek, and sorry for the trouble.  Like I said in the
adjacent reply, the warning is expected (I didn't realize the testsuite would
notice that, though).  I think the right fix is to simply add "-w" to
dg-options, and while we are at it, we should probably change -fvta-toggle to
just -fvta as well (because VTA is active either way, right?).

Andrey?

Thanks.
Alexander
Andrey Belevantsev March 15, 2016, 5:59 p.m. UTC | #3
On 15.03.2016 20:44, Alexander Monakov wrote:
> On Tue, 15 Mar 2016, Marek Polacek wrote:
>> This test fails for me due to
>> cc1plus: warning: var-tracking-assignments changes selective scheduling
>
> Thanks for the heads-up Marek, and sorry for the trouble.  Like I said in the
> adjacent reply, the warning is expected (I didn't realize the testsuite would
> notice that, though).  I think the right fix is to simply add "-w" to
> dg-options, and while we are at it, we should probably change -fvta-toggle to
> just -fvta as well (because VTA is active either way, right?).

Yes, the -fvta should work.  Sorry for the breakage, I guess I've misread 
the compare-tests output when also checking the run with forced sel-sched 
enabled.

I can take care of the test tomorrow morning or you can do it now.

Best,
Andrey

>
> Andrey?
>
> Thanks.
> Alexander
>
Alexander Monakov March 15, 2016, 6:12 p.m. UTC | #4
On Tue, 15 Mar 2016, Andrey Belevantsev wrote:
> On 15.03.2016 20:44, Alexander Monakov wrote:
> > On Tue, 15 Mar 2016, Marek Polacek wrote:
> > > This test fails for me due to
> > > cc1plus: warning: var-tracking-assignments changes selective scheduling
> >
> > Thanks for the heads-up Marek, and sorry for the trouble.  Like I said in
> > the adjacent reply, the warning is expected (I didn't realize the
> > testsuite would notice that, though).  I think the right fix is to simply
> > add "-w" to dg-options, and while we are at it, we should probably change
> > -fvta-toggle to just -fvta as well (because VTA is active either way,
> > right?).
> 
> Yes, the -fvta should work.  Sorry for the breakage, I guess I've misread the
> compare-tests output when also checking the run with forced sel-sched enabled.
> 
> I can take care of the test tomorrow morning or you can do it now.

Thanks for confirming — committed rev. 234227.

Alexander
diff mbox

Patch

diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
index c798935..893a3e5 100644
--- a/gcc/sel-sched.c
+++ b/gcc/sel-sched.c
@@ -4249,7 +4249,8 @@  invoke_aftermath_hooks (fence_t fence, rtx_insn *best_insn, int issue_more)
                                       issue_more);
       memcpy (FENCE_STATE (fence), curr_state, dfa_state_size);
     }
-  else if (GET_CODE (PATTERN (best_insn)) != USE
+  else if (! DEBUG_INSN_P (best_insn)
+	   && GET_CODE (PATTERN (best_insn)) != USE
            && GET_CODE (PATTERN (best_insn)) != CLOBBER)
     issue_more--;
 
diff --git a/gcc/testsuite/g++.dg/pr63384.C b/gcc/testsuite/g++.dg/pr63384.C
new file mode 100644
index 0000000..b4e0784
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr63384.C
@@ -0,0 +1,12 @@ 
+/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+/* { dg-options "-O2 -fselective-scheduling2 -fsel-sched-pipelining  -fsel-sched-pipelining-outer-loops -fsel-sched-reschedule-pipelined -fvar-tracking-assignments-toggle -ftree-vectorize" } */
+
+template <class T> T **make_test_matrix() {
+ T **data = new T *;
+ for (int i = 0; i < 1000; i++)
+    ;
+}
+
+template <typename T> void test() { T **c = make_test_matrix<T>(); }
+
+main() { test<float>(); }