Patchwork debug insns in SMS

login
register
mail settings
Submitter Alexandre Oliva
Date April 9, 2012, 6:53 a.m.
Message ID <or7gxpe6qt.fsf@livre.localdomain>
Download mbox | patch
Permalink /patch/151413/
State New
Headers show

Comments

Alexandre Oliva - April 9, 2012, 6:53 a.m.
On May  4, 2011, Revital1 Eres <ERES@il.ibm.com> wrote:

> 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).

Ping?

(Retested on x86_64-linux-gnu and i686-pc-linux-gnu)
Alexandre Oliva - June 13, 2012, 8:05 a.m.
On Apr  9, 2012, 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?

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

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

> Ping?

http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00419.html
Alexandre Oliva - June 13, 2012, 10 p.m.
Apologies for the duplicate ping, this one is now properly addressed to
the pass maintainer.

On Apr  9, 2012, Alexandre Oliva <aoliva@redhat.com> wrote:

> On May  4, 2011, Revital1 Eres <ERES@il.ibm.com> wrote:
>> 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).

> Ping?

> (Retested on x86_64-linux-gnu and i686-pc-linux-gnu)


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

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

Ping?  http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00419.html
Ayal Zaks - June 14, 2012, 4:54 p.m.
Thanks for the duplicate ping. This is fine.
So this indeed solves the discrepancy between running SMS w/ and w/o debugging?
Please include a comment next to the code stating why it's important
not to create such deps.
You may also want to store the result of "DEP_PRO (dep)" in
src_<something> and use it twice, for clarity.
Thanks,
Ayal.


On Thu, Jun 14, 2012 at 1:00 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
>
> Apologies for the duplicate ping, this one is now properly addressed to
> the pass maintainer.
>
> On Apr  9, 2012, Alexandre Oliva <aoliva@redhat.com> wrote:
>
> > On May  4, 2011, Revital1 Eres <ERES@il.ibm.com> wrote:
> >> 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).
>
> > Ping?
>
> > (Retested on x86_64-linux-gnu and i686-pc-linux-gnu)
>
>
> > for  gcc/ChangeLog
> > from  Alexandre Oliva  <aoliva@redhat.com>
>
> >       * ddg.c (build_intra_loop_deps): Discard deps of nondebug on
> > debug.
>
> Ping?  http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00419.html
>
> --
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer

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	2012-01-04 21:06:38.000000000 -0200
+++ gcc/ddg.c	2012-04-08 02:10:44.711511989 -0300
@@ -532,7 +532,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;