Message ID | orsjsvklu4.fsf_-_@livre.localdomain |
---|---|
State | New |
Headers | show |
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.
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
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;
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?