This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: line insn notes cleanups (haifa-sched and modulo-sched)


Tehila Meyzels <TEHILA@il.ibm.com> writes:

> Those hunks use the function 'find_line_note', which content is:
> find_line_note (rtx insn)
>  {
>    if (no_line_numbers)
>      return 0;
> 
>    for (; insn; insn = PREV_INSN (insn))
>      if (NOTE_P (insn)
>          && NOTE_LINE_NUMBER (insn) >= 0)
>        break;
> 
>    return insn;
>  }
> 
> Since it's returning the insn that is  a line number note (NOTE_LINE_NUMBER
> (insn) >= 0), we decided to eliminate it.
> Therefore, I think I'll keep it eliminated.

My question was not about the code related to find_line_note.  I agree
that should go.  However, this code prints "SMS loop many exits" and
"SMS loop many BBs" regardless of whether a line note is found or not.
There is similar code in the other hunks I mentioned.  Why should
these fprintf statements go?  That seems like an unrelated change.

Ian

> >    if (!loop->single_exit)
> > -    {
> > -      if (dump_file)
> > -     {
> > -       rtx line_note = find_line_note (BB_END (loop->header));
> > +      {
> > +       return false;
> > +      }
> >
> > -       fprintf (dump_file, "SMS loop many exits ");
> > -       if (line_note)
> > -         {
> > -           expanded_location xloc;
> > -           NOTE_EXPANDED_LOCATION (xloc, line_note);
> > -           fprintf (dump_file, " %s %d (file, line)\n",
> > -                  xloc.file, xloc.line);
> > -         }
> > -     }
> > -      return false;
> > -    }
> > -
> >    if (! SIMPLE_SMS_LOOP_P (loop) && ! loop_single_full_bb_p (loop))
> >      {
> > -      if (dump_file)
> > -     {
> > -       rtx line_note = find_line_note (BB_END (loop->header));
> > -
> > -       fprintf (dump_file, "SMS loop many BBs. ");
> > -       if (line_note)
> > -         {
> > -           expanded_location xloc;
> > -           NOTE_EXPANDED_LOCATION (xloc, line_note);
> > -           fprintf (dump_file, " %s %d (file, line)\n",
> > -                  xloc.file, xloc.line);
> > -         }
> > -     }
> >        return false;
> >      }
> >
> 
> In these hunks, and two others later in the same file, you are
> removing debugging output that doesn't have anything to do with line
> notes.  Is this intentional?
> 
> 
> This patch looks OK to me.  However, it is not stage 3 material.  So
> I'll approve it for stage 1.
> 
> But before checking it in, please run the gdb testsuite before and
> after the patch, and make sure there are no new regressions.  I'm
> pretty sure we are OK in terms of debug info, but we don't test for it
> very much in the gcc testsuite.  The gdb testsuite is not
> comprehensive for gcc debug info generation, but it will catch any
> gross errors.
> 
> Thanks.
> 
> Ian


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]