Re: Merging OC gdb with official gdb

[Andrew Cagney <ac131313@redhat.com>]

>> >> If you find that you do appear to need to define certain macro's then
>> >> post a question to check what is going on.
> 
>> >
>> > Ok, how should we handle following macros:
>> > (I looked at the current code, but they look like they are not yet in
>> > gdbarch)
>> >
>> > #define GDB_MULTI_ARCH 1
>> > #define CANNOT_STEP_BREAKPOINT
> 
>>
>> This will need to be multi-arched (or deleted).
> 
> What do you mean deleted? This will change functionality right?
> how can I do multi-arched.

Often people, when they examine a macro with the intent to multi-arch 
it, find they can instead delete it.

Anyway, to serve as an example, I've multi-arched CANNOT_STEP_BREAKPOINT.

>> > #define TARGET_HAS_HARDWARE_WATCHPOINTS
> 
> Is this one automatic?

I think that is now native only.

>> > #define HAVE_NONSTEPPABLE_WATCHPOINT
> 
> What about this?

Um, that will need thinking about (add to architecture or target vector)?

Sigh, for the moment, add it (separate patch) as a variable to the 
architecture vector and then create a bug report noteing that it needs a 
re-think.

>> > #define TARGET_CAN_USE_HARDWARE_WATCHPOINT(bp_type, cnt, ot) \
>> >   or1k_can_use_hardware_watchpoint(bp_type, cnt)
> 
>>
>> As part of the change:
>>
>> 2002-08-01  Grace Sainsbury  <graces@redhat.com>
>>
>>          * target.h: Add to_insert_hw_breakpoint, to_remove_hw_breakpoint,
>>          to_insert_watchpoint, to_remove_watchpoint,
>>          to_stopped_by_watchpoint, to_stopped_data_address,
>>          to_region_size_ok_for_hw_watchpoint, to_can_use_hw_breakpoint to
>>          target vecctor. Define their corresponding macros so they call
>>          them.
>>
>>          * target.c: Add default and debug versions of for
>>          to_insert_hw_breakpoint, to_remove_hw_breakpoint,
>>          to_insert_watchpoint, to_remove_watchpoint,
>>          to_stopped_by_watchpoint, to_stopped_data_address,
>>          to_region_size_ok_for_hw_watchpoint, to_can_use_hw_breakpoint.
>>
>> Much of the above was moved into the target vector.  The file
>> remote-or1k.c would need to add entries for them in its target vector.
> 
> done.
> 
> 
>> > #define STEP_SKIPS_DELAY_P (1)
>> > #define STEP_SKIPS_DELAY(pc) (or1k_step_skips_delay (pc))
> 
> what needs to be defined instead?

They need to be added to gdbarch.{sh,h,c}.  That's a separate change. 
See my post.  For this, look at one of the ``F:'' (predicate) methods - 
DO_REGISTERS_INFO.


>> The code that uses it mind:
>>
>>        if (STEP_SKIPS_DELAY_P
>>            && breakpoint_here_p (read_pc () + 4)
>>            && STEP_SKIPS_DELAY (read_pc ()))
>>          oneproc = 1;
>>
>> is pretty bogus -- what is ``4''?
> 
> that is delay slot instruction size; I changed the const to OR1K_INSTLEN.

That will also need to be added to the architecture vector (and 
gdbint.texinfo).

>> The intent is to make it as easy as possible to debug for anyone that
>> follows.  You can't step into, breakpoint, or call a macro (and
>> suprisingly it is to often the one line macro's that contain the bugs :-( )
> 
> Ok then, I will change all the macros that have some meaningfull code inside.
> 
> 
>> (A useful and generic feature would be a command to select the case of
>> register names.  Some people, apparently, like using the shift key)
> 
> Actually parsing was not case sensitive, just names in file were written in 
> hicaps.

Ah, all the targets also print them in lower case (except this one).

>> > or1k architecture has special address space of registers called Special
>> > Purpose Registers. These include cache, tick timer, supervision
>> > registers, debug registers, etc.
>> > They have to be separatedfrom General Purpose Registers, also GPRs.
>> > Due to large number of SPRs (several thousands), I did not include them
>> > into gdb register space (except program counter PC).
> 
>>
>> That's ok.  A 1000 registers is nothing!  I know of an arch with 4k (or
>> was it 8k!).
> 
> it is about 8k here also.

ok.

>> The problem with the above is that it assumes that there will never be
>> more than one outstanding register_name() call.  That assumption is wrong.
>>
>> register_name() should use (for want of a better word) permenant memory,
>> instead of a scratch array, when returning a register's name.
>>
>> Easiest might be to generate all the names once in
>> _initialize_or1k_tdep().  Another would be to generate each on-demand.
> 
> Huh, I must say I don't the idea pretty much. Some of the registers have e.g.: 
> read&clear or write&do_something functionality.
> This also involves your expression evaluator - how many times are values 
> evalueated in (and what order):
> $BAR + $FOE
> $BAR + $FOE * $BAR
> 
> I find it higly inconvenient to use them as SW variables...
> 
> If you still insist tell me what would be best way to include them in gdb 
> registers structure. Note that they should not be cached.

Even something as simple as:

static char **register_names[];

..._register_name (int regnum)

	if (register_names == NULL)
	  register_names = big array...
	if (register_names[regnum] == NULL)
	  generate register name
	return register_name[i];

>> > How do I get relevant frame? Is there a target already doing what you are
>> > proposing?
> 
>>
>> The more up-to-date multi-arch method print_registers_info() takes the
>> architecture and frame as parameters.
> 
> ok, done.

Thanks!

>> No, I wasn't
>>
> 
>> >> >   if (inv1)
>> >> >     printf_filtered (" %-5s flt: <invalid float>", REGISTER_NAME
>> >> > (regnum)); else
>> >> >     printf_filtered (" %-5s flt:%-17.9g", REGISTER_NAME (regnum),
>> >> > flt); printf_filtered (inv3 ? " dbl: <invalid double> " :
>> >> > 		   " dbl: %-24.17g ", doub);
> 
>>
>> (Er, but thinking about it).
>>
>> The ``-17.9g'' isn't going to be portable.  DOUBLEST can either be
>> ``double'' or ``long double''.
>>
>> Does something like:
>> print_floating (raw_buffer, builtin_type_ieee_single_big, gdb_stdout)
>> or
>> print_floating (raw_buffer, REGISTER_VIRTUAL_TYPE(i), gdb_stdout)
>> do what you want?
> 
> Do I have a choice? :)

Your target will it portability problems otherwize :-(  Ensuring that 
anyone can use a GDB target is important.  The other, less plesant 
option is:
...%g ...", (double) doub
(because you don't know what the type of DOUBLEST is.)


>> Can you summarize what the limitations were and post this, separatly, to
>> gdb@.  If there is some sort of limitation, people need to understand it
>> and determine if fixing it, or living with it, is best.
> 
> ok.

Thanks!  I think the conclusion is that you code gets to be called the 
``prototype''.

>> > I am reposting the files. Changes to config.gdb stays the same.
> 
>>
>> Lets just get or1k-tdep.c in.  I'm currently ignoring the others.
>>
>> How is your texinfo?
> 
> You really know how to set the questions ;)
> BTW: The question I asked you week ago :)))

I think, with the above done, you're clear to commit or1k-tdep.c and 
tweaks to the config (see xstormy16) so that that architecture is pure 
multi-arch.

Once that file is in, two things:

- someone should confirm that this base system (--target=or1k-elf?) 
building ok on a second random system with ,-Werror

- you'll want to clean up coding problems noticed by the ari (these are 
obvious).

Then, with those resolved, I think its fair you get to add your self and 
this arch to the MAINTAINERS file.

Andrew