On 01.04.2013 12:38, Andrey Belevantsev wrote:
On 22.02.2013 17:30, Andrey Belevantsev wrote:
Hello,

As found by Jakub and explained in the PR audit trail by Alexander, this
patch fixes the selective scheduler merge glitch of 2008 that added the
unnecessary JUMP_P check to the flush_pending_lists call.  I have removed
the check and expanded the binary negation for clarity.

The patch was tested on x86-64, ia64, and ppc64 to be safe.  The patch
should be conservatively safe at this stage as it adds more flushes and
thus more dependencies to the scheduler.  The original test is fixed, but I
don't know how to add the test checking assembly insns order to the
testsuite.

OK for trunk?

Andrey

2012-02-22  Alexander Monakov  <amona...@ispras.ru>
         Andrey Belevantsev  <a...@ispras.ru>

     PR middle-end/56077
     * sched-deps.c (sched_analyze_insn): When reg_pending_barrier,
     flush pending lists also on non-jumps.

Now backported to 4.7 and 4.6.

Leonid has reported that this patch breaks the kernel build on mipsel-linux. I have quickly analyzed the issue. The patch removes the JUMP_P check when flushing pending lists at the reg_pending_barrier, restoring the 2008 year behavior. But at that time we didn't have debug insns. Now with the removed check we flush pending lists also on debug insns, which results in freeing pending_read_mems but not in freeing pending_read_insns, as in add_dependence_list_and_free we explicitly forbid freeing the dependency list for debug insns (that was for fixing PR 44013). This is inconsistent, and when proceeding with analysis we immediately hit

2451             t = canon_rtx (t);
2452             pending = deps->pending_read_insns;
2453             pending_mem = deps->pending_read_mems;
2454             while (pending)
2455               {
2456                 if (read_dependence (XEXP (pending_mem, 0), t)

and the line 2456 segfaults because pending is not NULL, but pending mem is NULL.

I am testing the revert of this backport for 4.6 and will commit it in about an hour or so. However, I am surprised we don't hit this either on 4.7, 4.8 or trunk. Some flush_pending_lists calls are protected from debug insns as they check CALL_P or JUMP_P, but not all of them. It looks like flush_pending_lists should not be called on debug insns at all. And indeed, the attached patch fixes Leonid's test case.

Jakub, you don't happen to remember any changes in this area that could hide the problem for 4.7 and later?

Andrey
Index: gcc/sched-deps.c
===================================================================
*** gcc/sched-deps.c	(revision 197492)
--- gcc/sched-deps.c	(working copy)
*************** sched_analyze_insn (struct deps_desc *de
*** 3044,3050 ****
  
        /* Don't flush pending lists on speculative checks for
  	 selective scheduling.  */
!       if (!sel_sched_p () || !sel_insn_is_speculation_check (insn))
  	flush_pending_lists (deps, insn, true, true);
  
        if (!deps->readonly)
--- 3044,3050 ----
  
        /* Don't flush pending lists on speculative checks for
  	 selective scheduling.  */
!       if (NONDEBUG_INSN_P (insn) && (!sel_sched_p () || !sel_insn_is_speculation_check (insn)))
  	flush_pending_lists (deps, insn, true, true);
  
        if (!deps->readonly)

Reply via email to