mips ptrace.S fix
Neelkanth Natu
neelnatu at yahoo.com
Thu Feb 4 02:56:22 UTC 2010
Hi JC,
--- On Wed, 2/3/10, C. Jayachandran <c.jayachandran at gmail.com> wrote:
> From: C. Jayachandran <c.jayachandran at gmail.com>
> Subject: Re: mips ptrace.S fix
> To: "Neelkanth Natu" <neelnatu at yahoo.com>
> Cc: "Rui Paulo" <rpaulo at freebsd.org>, freebsd-mips at freebsd.org
> Date: Wednesday, February 3, 2010, 6:09 PM
> On Thu, Feb 4, 2010 at 1:01 AM,
> Neelkanth Natu <neelnatu at yahoo.com>
> wrote:
> > Your patch looks good. I have a few comments though.
> See inline:
> >
> > Index: lib/libc/mips/sys/ptrace.S
> >
> ===================================================================
> > --- lib/libc/mips/sys/ptrace.S (revision 203379)
> > +++ lib/libc/mips/sys/ptrace.S (working copy)
> > @@ -42,14 +42,26 @@
> > #endif /* LIBC_SCCS and not lint */
> >
> > LEAF(ptrace)
> > + .frame sp,40,ra
> >
> >>> space missing after the ','
> >
> > + .mask 0x80000000, -8
> > #ifdef __ABICALLS__
> > .set noreorder
> > .cpload t9
> > .set reorder
> > #endif
> > + subu sp, sp, 40
> > + sw ra, 32(sp)
> > +#ifdef __ABICALLS__
> > + .cprestore 16
> > +#endif
> > la t9, _C_LABEL(__error) #
> locate address of errno
> > - jalr t9
> > + jalr t9
> >
> >>> this change is not required - the newly added
> line has a tab at the end.
> >
> > +#ifdef __ABICALLS__
> > + lw gp, 16(sp)
> > +#endif
> >
> >>> this is redundant - the assembler will
> generate exactly the same line of
> >>> code due to the .cprestore directive above.
>
> Are you sure about this? I think the assembler
> generates the gp load
> only for the 'jal' and 'bal' not for 'jalr'.
>
> Probably the two lines(la and jalr) can be replaced by a
> jal.
>
You are right. My eyes glossed over between the 'jal' and 'jalr'. I think
combining the two instructions into a single 'jal' is the way to go.
best
Neel
> Regards,
> JC.
>
More information about the freebsd-mips
mailing list