RE: Sumbitting ip2k port for review.

["Jafa" <jafa at silicondust dot com>]

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