diff mbox

don't force debug insns after their PREV_INSNs

Message ID orbon1e6v2.fsf@livre.localdomain
State New
Headers show

Commit Message

Alexandre Oliva April 9, 2012, 6:50 a.m. UTC 
On Jun  3, 2011, Bernd Schmidt <bernds@codesourcery.com> wrote:

> On 06/03/2011 04:47 PM, Alexandre Oliva wrote:
>> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=677681 can be
>> “fixed” by disabling the artificial dependency of a debug insn on its
>> previous insn.
>> 
>> Debug insns will often depend on their prevs anyway, in a use/def
>> relationship, but if the def was (re)moved or the use was reset, this
>> artificial dep helped keep the debug insn “in place”.
>> 
>> Being a very imperfect heuristic, it's not clear that it helps more than
>> it harms.  Thoughts?  Regstrapped on x86_64-linux-gnu and
>> i686-linux-gnu.

> Can you explain a little more clearly what the problem is? The RH
> bugzilla isn't really clear.

The problem here is that a nondebug insn may be moved ahead of a useful
debug insn and clobber one of its inputs, rendering it useless, when
there's no good reason for the debug insn to be kept in place, other
than an accidental dependency on the previous insn when it happens to be
unrelated with the debug insn.

Removing the extraneous dependency, that was thought to be a way to
reduce movement of debug insns, improves on this problem.  It's not
clear that this artificial dependency really does any good, since odds
are that that previous insn may be pulled ahead anyway, in which case so
will debug insn (unless that would fail other of its deps, of course)

Retested.  Ok?

Comments

Alexandre Oliva June 13, 2012, 8:06 a.m. UTC | #1 
On Apr  9, 2012, Alexandre Oliva <aoliva@redhat.com> wrote:

> The problem here is that a nondebug insn may be moved ahead of a useful
> debug insn and clobber one of its inputs, rendering it useless, when
> there's no good reason for the debug insn to be kept in place, other
> than an accidental dependency on the previous insn when it happens to be
> unrelated with the debug insn.

> Removing the extraneous dependency, that was thought to be a way to
> reduce movement of debug insns, improves on this problem.  It's not
> clear that this artificial dependency really does any good, since odds
> are that that previous insn may be pulled ahead anyway, in which case so
> will debug insn (unless that would fail other of its deps, of course)

> Retested.  Ok?

> for  gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>

> 	* sched-deps.c (sched_analyze_insn): Don't force debug insns
> 	to follow their original predecessors.

Ping?  http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00418.html
diff mbox

Patch

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* sched-deps.c (sched_analyze_insn): Don't force debug insns
	to follow their original predecessors.

Index: gcc/sched-deps.c
===================================================================
--- gcc/sched-deps.c.orig	2012-02-25 09:45:31.749795611 -0200
+++ gcc/sched-deps.c	2012-04-08 02:10:39.710573253 -0300
@@ -2988,18 +2988,6 @@  sched_analyze_insn (struct deps_desc *de
 	    reg_last->uses = alloc_INSN_LIST (insn, reg_last->uses);
 	}
       CLEAR_REG_SET (reg_pending_uses);
-
-      /* Quite often, a debug insn will refer to stuff in the
-	 previous instruction, but the reason we want this
-	 dependency here is to make sure the scheduler doesn't
-	 gratuitously move a debug insn ahead.  This could dirty
-	 DF flags and cause additional analysis that wouldn't have
-	 occurred in compilation without debug insns, and such
-	 additional analysis can modify the generated code.  */
-      prev = PREV_INSN (insn);
-
-      if (prev && NONDEBUG_INSN_P (prev))
-	add_dependence (insn, prev, REG_DEP_ANTI);
     }
   else
     {