Re: Sumbitting ip2k port for review.

Re: Sumbitting ip2k port for review.

[Nick Kelsey]

Re: Sumbitting ip2k port for review.
From: Andrew Cagney <ac131313 at redhat dot com>
To: Nick Kelsey <nickk at ubicom dot com>
Cc: gdb-patches at sources dot redhat dot com
Date: Wed, 11 Dec 2002 19:01:37 -0500
Subject: Re: Sumbitting ip2k port for review.

>The other question is who contributed to this work? `Java at silicondust'
appears to have being doing some stuff >(or is that your alter ego?). I
believe David Taylor may have also worked on the file ip2k-tdep.c in the
past? I >know that the problems he encountered with the ip2k were one of the
motivations behind overhauling pseudo->registers.

jafa at silicondust is me also.

Rigisters, right. The ip2k has a lot of registers (about 384) and many of
them need to be combined and processed to give a useful user display. If you
take a took at ip2k-registers.h and remote-ip2k.c you will see the current
hack - a dummy memory address that the remote back-end builds to display
registers formatted nicely so they can be type-casted as a multi-level
structure (looks great in insight as the nodes can be colasped).

What the ip2k really needs out of the register handling is
- the ability to display a register group (there are about 12 register
groups in the ip2k).
- the ability to obtain a list of the register groups in a general way (for
use with a GUI).
- insight support.

I get the impression that insight is not under strong active development so
if we can leave the registers-structure trick in place then that would
satify the insight requirement. I am quite happy to work with whoever is
driving the changes to the register code so that the ip2k port can make use
of these features.

>- ``__'' shouldn't be avoided as a prefix to symbols. A leading `_' is
reserved for the system's namespace.

Sorry, i don't quite follow, can you give an example please. The ip2k gcc
port uses an underscore prefix for normal c-code. There are a few symbols
that the linker script creates that gdb needs if the vn is to be used. I
used a double-underscore prefix to avoid name-space issues with c-ode.

>Andrew

Thanks

Nick

Re: Sumbitting ip2k port for review.

[Andrew Cagney]

\
> What the ip2k really needs out of the register handling is
> - the ability to display a register group (there are about 12 register
> groups in the ip2k).
> - the ability to obtain a list of the register groups in a general way (for
> use with a GUI).
> - insight support.

See:

(gdb) help maint print register-groups

> I get the impression that insight is not under strong active development so
> if we can leave the registers-structure trick in place then that would
> satify the insight requirement. I am quite happy to work with whoever is
> driving the changes to the register code so that the ip2k port can make use
> of these features.

See current insight.

>>- ``__'' shouldn't be avoided as a prefix to symbols. A leading `_' is
> 
> reserved for the system's namespace.
> 
> Sorry, i don't quite follow, can you give an example please. The ip2k gcc
> port uses an underscore prefix for normal c-code. There are a few symbols
> that the linker script creates that gdb needs if the vn is to be used. I
> used a double-underscore prefix to avoid name-space issues with c-ode.

Your code contained C symbols with a leading `_'.  All GDB source code 
needs to avoid it.

Andrew

RE: Sumbitting ip2k port for review.

[Jafa]

Hi Andrew,

Thanks for taking the time to review the code and give feedback.

I agree with and follow most of your feedback and I will clean it up.

I think the only thing in tm-ip2k.h is IGNORE_HELPER_CALL which I copied
from the mips port.

When GDB does a step over it does a series of single steps until it reaches
its target ot it steps out of the current function (ie steps into a
sub-function). When it steps into a sub-function it requests the return-PC
and installs a tempory breakpoint so it can skip the function. This
IGNORE_HELPER_CALL define makes GDB call the defined function to determine
if the sub-function was a dummy helper (in which case it should just keep
stepping), or a real function (in which case it should do the breakpoint
trick).

The ip2k gcc port makes extensive use of function epilogue helpers which are
not real functions (they do not have a return address as the are jumped to,
not called), and so GDB needs to step over them as it they are part of the
same context.

I am quite happy to add this function to gdbarch and update the MIPS arch to
use it as well.

Does this make sense to do?

Nick

-----Original Message-----
From: Andrew Cagney [mailto:ac131313@redhat.com]
Sent: Wednesday, December 11, 2002 4:02 PM
To: Nick Kelsey
Cc: gdb-patches@sources.redhat.com
Subject: Re: Sumbitting ip2k port for review.

[I've replied on GDB patches. For reference the code is attached.]
> Hi all,
>
> I would like to submit for review the ip2k port of gdb that I have been
> working on for a while.
>
> A copyright assignment has been signed by Ubicom.
>
> Any feedback is welcome.
You'll probably want to look over the thread:
http://sources.redhat.com/ml/gdb/2002-02/msg00139.html
my follow up contains a check list.
The other question is who contributed to this work? `Java at
silicondust' appears to have being doing some stuff (or is that your
alter ego?). I believe David Taylor may have also worked on the file
ip2k-tdep.c in the past? I know that the problems he encountered with
the ip2k were one of the motivations behind overhauling pseudo-registers.
Having looked ip2k-tdep.c, some quick tweaks:
- the log messages should all go to gdb_stdlog, not gdb_stdout.
- instead of starting a message with "warning: ..." or similar, just
call warning(), or internal_error(), or ...
- The other targets have the command `set debug <cpu> <int>' to allow
the run-time control of the debug variable. That should be included.
Just borrow/tweak the code from mips-tdep.c say.
- it creates a tm-ip2k.h file. Per the above, all new targets need to
be pure multi-arched (with one shared library exception)
- ``__'' shouldn't be avoided as a prefix to symbols. A leading `_' is
reserved for the system's namespace.
- the registers are named ``r$80'' giving ``$r$80''. I don't know if
that is a good idea. Long term, as GDB's parser develops, you'll likely
find that this no longer works. Any reason to not simply use $r80?
- what is IGNORE_HELPER_CALL?
- take care when printing addresses. 0x%08lx won't work portably.
"0x%s", paddr (addr)" works better.
I'll look at ip2k-registers.h, remote-ip2k* later (I've got an OC target
to help with first). I've also, unfortunatly, put this out just as the
last of the frame overhaul goes in.
Andrew

Re: Sumbitting ip2k port for review.

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 1959 bytes --]

[I've replied on GDB patches.  For reference the code is attached.]

> Hi all,
> 
> I would like to submit for review the ip2k port of gdb that I have been
> working on for a while.
> 
> A copyright assignment has been signed by Ubicom.
> 
> Any feedback is welcome.

You'll probably want to look over the thread:
http://sources.redhat.com/ml/gdb/2002-02/msg00139.html
my follow up contains a check list.

The other question is who contributed to this work?  `Java at 
silicondust' appears to have being doing some stuff (or is that your 
alter ego?).  I believe David Taylor may have also worked on the file 
ip2k-tdep.c in the past?  I know that the problems he encountered with 
the ip2k were one of the motivations behind overhauling pseudo-registers.

Having looked ip2k-tdep.c, some quick tweaks:

- the log messages should all go to gdb_stdlog, not gdb_stdout.

- instead of starting a message with "warning: ..." or similar, just 
call warning(), or internal_error(), or ...

- The other targets have the command `set debug <cpu> <int>' to allow 
the run-time control of the debug variable.  That should be included. 
Just borrow/tweak the code from mips-tdep.c say.

- it creates a tm-ip2k.h file.  Per the above, all new targets need to 
be pure multi-arched (with one shared library exception)

- ``__'' shouldn't be avoided as a prefix to symbols.  A leading `_' is 
reserved for the system's namespace.

- the registers are named ``r$80'' giving ``$r$80''.  I don't know if 
that is a good idea.  Long term, as GDB's parser develops, you'll likely 
find that this no longer works.  Any reason to not simply use $r80?

- what is IGNORE_HELPER_CALL?

- take care when printing addresses.  0x%08lx won't work portably. 
"0x%s", paddr (addr)" works better.

I'll look at ip2k-registers.h, remote-ip2k* later (I've got an OC target 
to help with first).  I've also, unfortunatly, put this out just as the 
last of the frame overhaul goes in.

Andrew


[-- Attachment #2: gdb-ip2k.tar.gz --]
[-- Type: application/gzip, Size: 34128 bytes --]