diff mbox

debug insns in SMS (was: Re: Debug_insn)

Message ID orsjsvklu4.fsf_-_@livre.localdomain
State New
Headers show

Commit Message

Alexandre Oliva May 4, 2011, 6:42 a.m. UTC
On May  3, 2011, Revital1 Eres <ERES@il.ibm.com> wrote:

> Please let me know if you need any further info.

No, thanks, that was all I needed.

I think this will restore proper functioning to SMS in the presence of
debug insns.  A while ago, we'd never generate deps of non-debug insns
on debug insns.  I introduced them to enable sched to adjust (reset)
debug insns when non-debug insns were moved before them.  I believe it
is safe to leave them out of the SCCs.  Even though this will end up
causing some loss of debug info, that's probably unavoidable, and the
end result after this change is pobably the best we can hope for.  Your
thoughts?

Is this ok to install if it regstraps successfully?

Comments

Alexandre Oliva May 4, 2011, 8:52 a.m. UTC | #1
On May  4, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:

> I think this will restore proper functioning to SMS in the presence of
> debug insns.  A while ago, we'd never generate deps of non-debug insns
> on debug insns.  I introduced them to enable sched to adjust (reset)
> debug insns when non-debug insns were moved before them.  I believe it
> is safe to leave them out of the SCCs.  Even though this will end up
> causing some loss of debug info, that's probably unavoidable, and the
> end result after this change is pobably the best we can hope for.  Your
> thoughts?

Actually...  We can probably do better.  Leaving the debug insns out, we
have no way to adjust (reset) them when non-debug insns that depended on
them were moved across them, rendering the debug value incorrect.

I'm now trying to find out some way to add the tracking of these deps to
the DDG in a way that doesn't change the SCCs because of -g, but that
enables us to adjust (or even modulo-schedule) debug insns accurately.

Suggestions would be appreciated.
Revital1 Eres May 4, 2011, 9:31 a.m. UTC | #2
Hello Alexandre

> I think this will restore proper functioning to SMS in the presence of
> debug insns.  A while ago, we'd never generate deps of non-debug insns
> on debug insns.  I introduced them to enable sched to adjust (reset)
> debug insns when non-debug insns were moved before them.  I believe it
> is safe to leave them out of the SCCs.  Even though this will end up
> causing some loss of debug info, that's probably unavoidable, and the
> end result after this change is pobably the best we can hope for.  Your
> thoughts?

Thanks for the patch!

I actually discussed this issue with Ayal yesterday.
Ayal also suggested to reconsider the edges that are created in
the DDG between real instructions and debug_insns. Currently, we create
bidirectional anti deps edges between them. This leads to the problem you
were trying to solve in the current patch (described below) where these
extra edges influence the construction of the strongly connected component
and the code generated with and w\o -g. Your patch seems to solve this
problem.
However I can not approve it as I'm not the maintainer (Ayal is).

Another problem with the handling of debug insns in SMS is that
debug_insns instructions are been ignored while scheduling which means
that they do not delimit the scheduling window of the real instructions
they depend on. This could lead to a scenario where the dependencies
between real instruction and debug_insn could be violated as we leave
the debug_insns in their original place after scheduling. I'll try to send
you
an example of this problem as well.

Example of code gen difference when using -g and without it which this
patch tries to solve:
The following nodes are all belong to the same SCC running with -g.
node 1 is not present in this SCC when not using -g.
(the nodes marked in [] are the one that do not
exist with your patch and prevent from node 1 to be added to the SCC
when compiling with -g)

                                                                     node 0
Debug_insn  (ior(MEM(ivtmp.24),MEM(ivtmp.46))
in edges: 1->0, 2->0
out edges: [0->1], [0->4], [0->2]


node 1
Reg 220 = MEM (pre_inc (ivtmp.24))
in edges: [0->1], 1->1
out edges: 1->0, 1->1, 1->3

node 2
Reg 221= MEM (pre_inc (ivtmp.46))
in edges: [0->2], 4->2, 2->2
out edges: 2->0, 2->3, 2->4, 2->2

node 3
Reg 222= ior (220,221)
in edges: 2->3, 1->3
out edges: 3->4

node 4
MEM[pre_inc(196)] = 222
in edges: 3->4, 4->4, [0->4], 2->4
out edges: 4->4, 4->2

Thanks,
Revital
diff mbox

Patch

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

	* ddg.c (build_intra_loop_deps): Discard deps of nondebug on debug.

Index: gcc/ddg.c
===================================================================
--- gcc/ddg.c.orig	2011-05-04 03:42:20.938013725 -0300
+++ gcc/ddg.c	2011-05-04 03:42:30.202253457 -0300
@@ -452,7 +452,12 @@  build_intra_loop_deps (ddg_ptr g)
 
       FOR_EACH_DEP (dest_node->insn, SD_LIST_BACK, sd_it, dep)
 	{
-	  ddg_node_ptr src_node = get_node_of_insn (g, DEP_PRO (dep));
+	  ddg_node_ptr src_node;
+
+	  if (DEBUG_INSN_P (DEP_PRO (dep)) && !DEBUG_INSN_P (dest_node->insn))
+	    continue;
+
+	  src_node = get_node_of_insn (g, DEP_PRO (dep));
 
 	  if (!src_node)
 	    continue;