Re: [patch] MIPS16e support in simulator.

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [patch] MIPS16e support in simulator.

Daniel Jacobowitz-2
On Thu, Jun 09, 2005 at 05:52:13PM +0100, David Ung wrote:

>
> This patch adds MIPS16e support in the GNU simulator.  I've configure
> targets mipsisa32*-*-*, mipsisa32r2*-*-*, mipsisa64*-*-*,
> mipsisa64r2*-*-* to generate both normal and mips16e code.
>
> David.
>
> 2005-06-09  David Ung  <[hidden email]>
>    Nigel Stephens  <[hidden email]>
>
> * mips.igen: New mips16e model and include m16e.igen.
> (check_u64): Add mips16e tag.
> * m16e.igen: New file for MIPS16e instructions.
> * configure.ac (mipsisa32*-*-*, mipsisa32r2*-*-*, mipsisa64*-*-*,
> mipsisa64r2*-*-*): Change sim_gen to M16, add mips16 and mips16e
> models.
> * configure: Regenerate.

So um... unfortunately, GDB does not have an active MIPS maintainer at
the moment.  I do not see anything (other than your followup about the
mipsisa32 configure stanza) obviously wrong with these changes, but I
can't provide any useful review of the mips-specific bits.  I don't
suppose someone on binutils@ could lend a hand?

--
Daniel Jacobowitz
CodeSourcery, LLC
Reply | Threaded
Open this post in threaded view
|

Re: [patch] MIPS16e support in simulator.

Eric Christopher

> So um... unfortunately, GDB does not have an active MIPS maintainer at
> the moment.  I do not see anything (other than your followup about the
> mipsisa32 configure stanza) obviously wrong with these changes, but I
> can't provide any useful review of the mips-specific bits.  I don't
> suppose someone on binutils@ could lend a hand?
>

I will, url to the original patch?

-eric

Reply | Threaded
Open this post in threaded view
|

Re: [patch] MIPS16e support in simulator.

Daniel Jacobowitz-2
On Mon, Jun 13, 2005 at 10:02:45AM -0700, Eric Christopher wrote:
>
> > So um... unfortunately, GDB does not have an active MIPS maintainer at
> > the moment.  I do not see anything (other than your followup about the
> > mipsisa32 configure stanza) obviously wrong with these changes, but I
> > can't provide any useful review of the mips-specific bits.  I don't
> > suppose someone on binutils@ could lend a hand?
> >
>
> I will, url to the original patch?

http://sourceware.org/ml/gdb-patches/2005-06/msg00078.html
http://sourceware.org/ml/gdb-patches/2005-06/msg00079.html

Thanks!

--
Daniel Jacobowitz
CodeSourcery, LLC
Reply | Threaded
Open this post in threaded view
|

Re: [patch] MIPS16e support in simulator.

Eric Christopher
On Mon, 2005-06-13 at 13:03 -0400, Daniel Jacobowitz wrote:

> On Mon, Jun 13, 2005 at 10:02:45AM -0700, Eric Christopher wrote:
> >
> > > So um... unfortunately, GDB does not have an active MIPS maintainer at
> > > the moment.  I do not see anything (other than your followup about the
> > > mipsisa32 configure stanza) obviously wrong with these changes, but I
> > > can't provide any useful review of the mips-specific bits.  I don't
> > > suppose someone on binutils@ could lend a hand?
> > >
> >
> > I will, url to the original patch?
>
> http://sourceware.org/ml/gdb-patches/2005-06/msg00078.html
> http://sourceware.org/ml/gdb-patches/2005-06/msg00079.html

Looks like the jump instructions aren't executing the instructions in
the delay slot? See the definition of jalr in m16.igen.

I guess a good question would be asking how this patch was tested?

Otherwise it looks fine.

-eric

Reply | Threaded
Open this post in threaded view
|

Re: [patch] MIPS16e support in simulator.

David Ung
On Mon, 2005-06-13 at 16:22 -0700, Eric Christopher wrote:

> On Mon, 2005-06-13 at 13:03 -0400, Daniel Jacobowitz wrote:
> > On Mon, Jun 13, 2005 at 10:02:45AM -0700, Eric Christopher wrote:
> > >
> > > > So um... unfortunately, GDB does not have an active MIPS maintainer at
> > > > the moment.  I do not see anything (other than your followup about the
> > > > mipsisa32 configure stanza) obviously wrong with these changes, but I
> > > > can't provide any useful review of the mips-specific bits.  I don't
> > > > suppose someone on binutils@ could lend a hand?
> > > >
> > >
> > > I will, url to the original patch?
> >
> > http://sourceware.org/ml/gdb-patches/2005-06/msg00078.html
> > http://sourceware.org/ml/gdb-patches/2005-06/msg00079.html
>
> Looks like the jump instructions aren't executing the instructions in
> the delay slot? See the definition of jalr in m16.igen.

This is correct.  The MIPS16e jrc, jalrc etc does not have a delay slot.

>
> I guess a good question would be asking how this patch was tested?
>
> Otherwise it looks fine.

We've actually had MIPS16e simulator support for quite awhile at MIPS
for our internal use.  We've been doing nightly regressions test on
MIPS16e for gcc 3.4 and gdb for more that half year now.

David.

Reply | Threaded
Open this post in threaded view
|

Re: [patch] MIPS16e support in simulator.

Eric Christopher

> >
> > Looks like the jump instructions aren't executing the instructions in
> > the delay slot? See the definition of jalr in m16.igen.
>
> This is correct.  The MIPS16e jrc, jalrc etc does not have a delay slot.
>

OK. The docs I have don't call this out explicitly in the instruction
description and probably should given that other mips (and mips16)
instructions have delay slots.

> >
> > I guess a good question would be asking how this patch was tested?
> >
> > Otherwise it looks fine.
>
> We've actually had MIPS16e simulator support for quite awhile at MIPS
> for our internal use.  We've been doing nightly regressions test on
> MIPS16e for gcc 3.4 and gdb for more that half year now.

You should say that sort of thing when you submit patches, e.g.:

"Tested on mips-elf using internal mips16e port. No regressions."

or some such.

-eric

Reply | Threaded
Open this post in threaded view
|

Re: [patch] MIPS16e support in simulator.

Daniel Jacobowitz-2
In reply to this post by Eric Christopher
On Mon, Jun 13, 2005 at 04:22:36PM -0700, Eric Christopher wrote:

> On Mon, 2005-06-13 at 13:03 -0400, Daniel Jacobowitz wrote:
> > On Mon, Jun 13, 2005 at 10:02:45AM -0700, Eric Christopher wrote:
> > >
> > > > So um... unfortunately, GDB does not have an active MIPS maintainer at
> > > > the moment.  I do not see anything (other than your followup about the
> > > > mipsisa32 configure stanza) obviously wrong with these changes, but I
> > > > can't provide any useful review of the mips-specific bits.  I don't
> > > > suppose someone on binutils@ could lend a hand?
> > > >
> > >
> > > I will, url to the original patch?
> >
> > http://sourceware.org/ml/gdb-patches/2005-06/msg00078.html
> > http://sourceware.org/ml/gdb-patches/2005-06/msg00079.html
>
> Looks like the jump instructions aren't executing the instructions in
> the delay slot? See the definition of jalr in m16.igen.
>
> I guess a good question would be asking how this patch was tested?
>
> Otherwise it looks fine.

Given this, and the followups, the patch is OK.  It looks like David
doesn't have commit access; is that right?  Let me know and I'll take
care of the patch.

--
Daniel Jacobowitz
CodeSourcery, LLC
Reply | Threaded
Open this post in threaded view
|

Re: [patch] MIPS16e support in simulator.

Maciej W. Rozycki
In reply to this post by Eric Christopher
On Tue, 14 Jun 2005, Eric Christopher wrote:

> > This is correct.  The MIPS16e jrc, jalrc etc does not have a delay slot.
> >
>
> OK. The docs I have don't call this out explicitly in the instruction
> description and probably should given that other mips (and mips16)
> instructions have delay slots.

 Perhaps you have wrong docs? ;-)  Vol. IV-a of the MIPS32 ISA document
set (downloadable from the web site) is quite explicit on that in the
detailed instruction descriptions -- see "Programming Notes" for the
relevant ones.

  Maciej
Reply | Threaded
Open this post in threaded view
|

Re: [patch] MIPS16e support in simulator.

Eric Christopher

>  Perhaps you have wrong docs? ;-)  Vol. IV-a of the MIPS32 ISA document
> set (downloadable from the web site) is quite explicit on that in the
> detailed instruction descriptions -- see "Programming Notes" for the
> relevant ones.

Hey, yup, there it is. Right there at the bottom of the page.


-eric

Reply | Threaded
Open this post in threaded view
|

Re: [patch] MIPS16e support in simulator.

David Ung
In reply to this post by Daniel Jacobowitz-2
On Tue, 2005-06-14 at 13:48 -0400, Daniel Jacobowitz wrote:

> On Mon, Jun 13, 2005 at 04:22:36PM -0700, Eric Christopher wrote:
> > On Mon, 2005-06-13 at 13:03 -0400, Daniel Jacobowitz wrote:
> > > On Mon, Jun 13, 2005 at 10:02:45AM -0700, Eric Christopher wrote:
> > > >
> > > > > So um... unfortunately, GDB does not have an active MIPS maintainer at
> > > > > the moment.  I do not see anything (other than your followup about the
> > > > > mipsisa32 configure stanza) obviously wrong with these changes, but I
> > > > > can't provide any useful review of the mips-specific bits.  I don't
> > > > > suppose someone on binutils@ could lend a hand?
> > > > >
> > > >
> > > > I will, url to the original patch?
> > >
> > > http://sourceware.org/ml/gdb-patches/2005-06/msg00078.html
> > > http://sourceware.org/ml/gdb-patches/2005-06/msg00079.html
> >
> > Looks like the jump instructions aren't executing the instructions in
> > the delay slot? See the definition of jalr in m16.igen.
> >
> > I guess a good question would be asking how this patch was tested?
> >
> > Otherwise it looks fine.
>
> Given this, and the followups, the patch is OK.  It looks like David
> doesn't have commit access; is that right?  Let me know and I'll take
> care of the patch.

no, I don't have write access.  Could you commit it please.

David.