Re: [patch] add target Score to GDB

[Jim Blandy <jimb@codesourcery.com>]

秦巍<qinwei@sunnorth.com.cn> writes:
> Jim Blandy <jimb@codesourcery.com> wrote on 2006-12-07 03:41:54:
>  
>> qinwei@sunnorth.com.cn writes:
>> > If we won't open simulator source, can we put simulator libs(11 libs) 
> and 
>> > simstub.c in the dir 'gdb-6.5/sim/score'?
>> > If we can, i will send you these libs. 
>> > Otherwise, i post the patch in the following just containing 'score' 
> gdb 
>> > but without simulator.
>> 
>> The GDB source tree cannot contain non-Free software.  We give our
>> users all the rights to modify and redistribute the software that we
>> have, on the condition that they do the same for others; that's the
>> founding principle of Project GNU.  Including non-free libraries with
>> GDB would not follow that principle.
>> 
>> We encourage you to free your simulator; if that's not possible, then
>> we'll need to leave the simulator out.
>
> Hi Jim, 
> Sorry our company SunNorth doesn't want to open simulator source, please 
> leave it out.

Okay.

>> Does the S+core architecture allow you to mix 32-bit and 16-bit
>> instructions?  If so, could score_breakpoint_from_pc be simplified to
>> always return a 16-bit breakpoint?  This would mean that a breakpoint
>> inserted at a 32-bit instruction would produce two bytes of garbage
>> after the breakpoint, but that's normal; on the i386, GDB uses a
>> one-byte breakpoint instruction regardless of the length of the
>> instruction it's set at.
>
> Yes, score supports the mixture of 32-bit and 16-bit instructions.
> But score uses the word address to fetch instruction, and then identify if 
> the 32-bits data 
> is a 32-bit instruction or 2 16-bit instructions.
> Only setting 16-bit breakpoint instruction will affect score indentifing 
> instruction types
> and cause error both on hardware and simulator.

I see --- there's much more going on here than there is in most other
processors.

>> Does score_integer_to_address do exactly the same thing that GCC does
>> when the user casts an integer to an address?  If they don't, then the
>> user could become quite confused when GDB and GCC evaluate their
>> expressions differently.
>
> I didn't see clearly about this hook when i was porting score gdb.
> I have seen your NOTES in 'value_as_address'.
> The pointer and the address are the same on score target, 
> and a pointer to a function always points to the entry point address of 
> that function.
> So we don't need to implement this hook. Can i say that?

If you don't need to set it to get clean test suite results, then I
would definitely recommend leaving it out.

>> Future revisions of GCC change the sorts of prologues GDB will
>> encounter.  If you find yourself investing more time than you expected
>> on score_analyze_prologue, consider using the framework in
>> prologue-value.h, documented there.
>
> The next step i will survey other methods such as using 
> prologue-value.[ch] or 
> using .eh_frame section and will post new patches.
>
> Can this patch be checked in first?

Using prologue-value certainly isn't required; I just wanted to
suggest it.

There should be patches to Makefile.in; could you post those?  We need
to see the complete patch in order to approve it.

All changes need a ChangeLog entry.  You don't need to say much for
score-tdep.c and score-tdep.h; here's the entry for the xtensa port:

2006-11-14  Maxim Grigoriev  <maxim@tensilica.com>

	* NEWS: New port to Xtensa.
	* Makefile.in: Add dependencies for Xtensa files.
	* configure.tgt (xtensa*, xtensa*-*-elf*): New.
	* configure.host (xtensa*-*-elf*): New.
	* config/xtensa/xtensa.mt: New file.
	* xtensa-config.c: New file.
	* xtensa-tdep.h: New file.
	* xtensa-tdep.c: New file.

(I would prefer that the Makefile.in entry actually listed the
variables and rules added.)

Please keep all lines under 80 characters in length; for example,
score_unwind_dummy_id needs to be reformatted.

I'm surprised by score_type_needs_double_align --- is it really true
that struct types only need double alignment if their *first* member
does?  Why is that?  I'd expect the code for structs and unions to be
the same.

Other than that, this looks good.  Please post a complete patch,
generated against the current CVS sources, and with a ChangeLog entry,
for a final review, and then we'll get it committed.