Re: [PATCH] [SH] Prologue skipping if there is none

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi!

On Thu, 16 Feb 2012 18:25:44 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
> On Thu, 16 Feb 2012 17:32:18 +0100
> Thomas Schwinge <thomas@codesourcery.com> wrote:
> 
> > I now have (on a SH7785-based board).  My patch fixes a few more failures
> > than yours.  ;-P
> 
> This will require more study and discussion then.

Heh, indeed.  :-)

> I tested against
> the simulator using the default multilib.  I compared results using
> each of our patches to an unpatched sh-tdep.c.

Thanks, and I will likewise do such testing in the next days.

> Here are the FAILs that my patch fixed.  There are 246 of them.  [...]
> 
> Here is a list of FAILs that my patch introduced (regressions).  There
> are 9 of them: [...]

> Here are the FAILs that your patch fixed.  There are 127 of them.  [...]

> Your patch did not introduce any regressions.

> So my testing showed that my patch fixed more failures, but introduced
> regressions.  I find it conceivable, however, that my patch might not
> fare as well on some other target.  (That's what your testing
> demonstrates, right?)

Yeah, apparently.

> >     tbreak add_charest
> >     Temporary breakpoint 10 at 0x400720: file /scratch/tschwing/FM_sh-linux-gnu/src/gdb-mainline/gdb/testsuite/gdb.base/store.c, line 14.

> > So the ``tbreak add_charest'' chose line 14 instead of 15.
> 
> I took a look at this regression.  [...]

Many thanks for the analysis (omitted here) as well as your comments to
my comments; I will go throught this in the next days.


So, we do seem to agree that something like the patch I posted initially
(and as it is incorporated in a similar fashion in your patch, too) does
already move us forward; is it reasonable that we commit that one now,
and then continue to look on how to further improve the situation based
on your patch?  This will let us point out more easily which are the
additional cases your patch improves/regresses on.  (I'd offer to port
your patch over to the new baseline, if you want me to.)


GrÃÃe,
 Thomas

Attachment: pgp00000.pgp
Description: PGP signature