Patchwork fixed two issues in handling debug_insn.

login
register
mail settings
Submitter Bingfeng Mei
Date July 16, 2010, 12:43 p.m.
Message ID <7FB04A5C213E9943A72EE127DB74F0ADA6897AD008@SJEXCHCCR02.corp.ad.broadcom.com>
Download mbox | patch
Permalink /patch/59102/
State New
Headers show

Comments

Bingfeng Mei - July 16, 2010, 12:43 p.m.
Sorry, I am a bit careless. Thanks.

Bingfeng


> -----Original Message-----
> From: Jakub Jelinek [mailto:jakub@redhat.com]
> Sent: 16 July 2010 13:38
> To: Bingfeng Mei
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] fixed two issues in handling debug_insn.
> 
> On Fri, Jul 16, 2010 at 05:27:40AM -0700, Bingfeng Mei wrote:
> > Well, I don't know that function. Here is the updated patch. OK for
> > trunk & 4.5?
> 
> > +      rtx prev_insn = prev_nondebug_insn (doloop_pat);
> >
> >        /* We expect the decrement to immediately precede the branch.
> */
> >
> >        if ((PREV_INSN (doloop_pat) == NULL_RTX)
> > -          || !INSN_P (PREV_INSN (doloop_pat)))
> > +          || !INSN_P (prev_insn))
> >          return 0;
> 
> This will crash if PREV_INSN is non-NULL, but prev_nondebug_insn
> returns
> NULL.  And, please remove the redundant parens and unnecessary
> splitting
> of the condition on two lines, i.e. what I wrote:
> 
>        if (prev_insn == NULL_RTX || !INSN_P (prev_insn))
> 
> It looks good to me with those changes (also the ddg.c change), but I
> can't
> approve this.
> 
> 	Jakub
Richard Henderson - July 16, 2010, 4:36 p.m.
On 07/16/2010 05:43 AM, Bingfeng Mei wrote:
> Sorry, I am a bit careless. Thanks.
> 
> Bingfeng
> 
> Index: ddg.c
> ===================================================================
> --- ddg.c       (revision 162258)
> +++ ddg.c       (working copy)
> @@ -488,7 +488,7 @@
>      }
> 
>    /* There is nothing to do for this BB.  */
> -  if (num_nodes <= 1)
> +  if ((num_nodes - g->num_debug) <= 1)
>      {
>        free (g);
>        return NULL;
> Index: loop-doloop.c
> ===================================================================
> --- loop-doloop.c       (revision 162258)
> +++ loop-doloop.c       (working copy)
> @@ -104,11 +104,11 @@
>    if (GET_CODE (pattern) != PARALLEL)
>      {
>        rtx cond;
> +      rtx prev_insn = prev_nondebug_insn (doloop_pat);
> 
>        /* We expect the decrement to immediately precede the branch.  */
> 
> -      if ((PREV_INSN (doloop_pat) == NULL_RTX)
> -          || !INSN_P (PREV_INSN (doloop_pat)))
> +      if (prev_insn == NULL_RTX || !INSN_P (prev_insn))
>          return 0;

Ok.


r~
Bingfeng Mei - July 19, 2010, 9:59 a.m.
Commit to trunk at 162300 & 4.5 at 162302


2010-07-19  Bingfeng Mei  <bmei@broadcom.com>
	* ddg.c (create_ddg): Exclude nodes of debug_insn in counting nodes
        of a loop.
        * loop-doloop.c (doloop_condition_get): Skip possible debug_insn.

> -----Original Message-----
> From: Richard Henderson [mailto:rth@redhat.com]
> Sent: 16 July 2010 17:37
> To: Bingfeng Mei
> Cc: Jakub Jelinek; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] fixed two issues in handling debug_insn.
> 
> On 07/16/2010 05:43 AM, Bingfeng Mei wrote:
> > Sorry, I am a bit careless. Thanks.
> >
> > Bingfeng
> >
> > Index: ddg.c
> > ===================================================================
> > --- ddg.c       (revision 162258)
> > +++ ddg.c       (working copy)
> > @@ -488,7 +488,7 @@
> >      }
> >
> >    /* There is nothing to do for this BB.  */
> > -  if (num_nodes <= 1)
> > +  if ((num_nodes - g->num_debug) <= 1)
> >      {
> >        free (g);
> >        return NULL;
> > Index: loop-doloop.c
> > ===================================================================
> > --- loop-doloop.c       (revision 162258)
> > +++ loop-doloop.c       (working copy)
> > @@ -104,11 +104,11 @@
> >    if (GET_CODE (pattern) != PARALLEL)
> >      {
> >        rtx cond;
> > +      rtx prev_insn = prev_nondebug_insn (doloop_pat);
> >
> >        /* We expect the decrement to immediately precede the branch.
> */
> >
> > -      if ((PREV_INSN (doloop_pat) == NULL_RTX)
> > -          || !INSN_P (PREV_INSN (doloop_pat)))
> > +      if (prev_insn == NULL_RTX || !INSN_P (prev_insn))
> >          return 0;
> 
> Ok.
> 
> 
> r~

Patch

Index: ddg.c
===================================================================
--- ddg.c       (revision 162258)
+++ ddg.c       (working copy)
@@ -488,7 +488,7 @@ 
     }

   /* There is nothing to do for this BB.  */
-  if (num_nodes <= 1)
+  if ((num_nodes - g->num_debug) <= 1)
     {
       free (g);
       return NULL;
Index: loop-doloop.c
===================================================================
--- loop-doloop.c       (revision 162258)
+++ loop-doloop.c       (working copy)
@@ -104,11 +104,11 @@ 
   if (GET_CODE (pattern) != PARALLEL)
     {
       rtx cond;
+      rtx prev_insn = prev_nondebug_insn (doloop_pat);

       /* We expect the decrement to immediately precede the branch.  */

-      if ((PREV_INSN (doloop_pat) == NULL_RTX)
-          || !INSN_P (PREV_INSN (doloop_pat)))
+      if (prev_insn == NULL_RTX || !INSN_P (prev_insn))
         return 0;

       cmp = pattern;