gdb/sh-tdep.c: need to eliminate target-dependent static variables

[Joern Rennecke <joern dot rennecke at st dot com>]

Joern Rennecke wrote:
> 
> ezannoni@redhat.com wrote:
> > Now, looking at the current mapping, the 2 sets are not disjoint, and
> > this is the reason for which gdb is failing some tests currently. I
> > hadn't closely looked at this mapping before, unfortunately.  In case
> > of general registers gdb gets very confused between compact and media
> > mode. It doesn't know how long the register is when trying to read the
> > value of a variable stored in a general register because the debug
> > information is exactly the same for the 2 modes. The only way to get
> > around this, which your re-mapping would also require, is to have
> > *always* available the PC of the frame relative to which we are
> > reading the register, because the PC being odd/even tells us which
> > ABI, and therefore which register size, is in use.  At this time I
> > have identified 2 places in GDB where I have a hack in place to work
> > around this overlap of register sets. However I haven't submitted the
> > hacks publicly, because they are pretty bad. Anyway, there are plenty
> > of other places where a PC value (i.e. a frame parameter) would need
> > to be added to gdb internal functions, to get this right.
> 
> Right, so we do need the general purpose register to have different
> numbers, because of their different size.  The size shouldn't really
> matter for single integer values inside a register, but it does when you
> need more than one register for SHcompact, or you are describing register
> saves / restores.
> 
> > As far as the sh-tdep.c code goes, this also would have to be
> > extensively rewritten if the assumption that the register sets are
> > disjoint is dropped.
> >
> > So, I would be opposed to such a change.
> 
> It needs to be extensively rewritten anyways to accomodate the disjoint
> register numbers for SH1-4 / SHmedia simulators, and to fix numerous bugs.
> This rewrite becomes indeed simpler if we make the interface a bit more
> straightforward.
> 
> > I don't think renumbering the simulator registers would have such a
> > devastating effect.
> 
> sh-tdep has a lot of hardcoded 'knowledge' how simulator numbers relate
> to the layout of th register cache.  Moreover, we should loose all the
> tdep->FOO_REGNUM stuff where the value is actually a constant according
> to the simulator interface.
> 
> > Anyway, I agree with you that the current version of the
> > SH_DBX_REGISTER_NUMBER is not the most logic arrangement.  The
> > floating point registers are mapped to a separate subset, but the
> > general registers are not.
> >
> > All the following registers should be mapped to a register
> > set disjoint from the sh media one.
> > Right now they are not.
> >
> > SH COMPACT MODE (ISA 16)
> > 221     r0_c, r1_c, r2_c,  r3_c,  r4_c,  r5_c,  r6_c,  r7_c,
> > 229     r8_c, r9_c, r10_c, r11_c, r12_c, r13_c, r14_c, r15_c,
The sh-tdep.c file contains three file scope static variables that are set
when a new architecture is initialized: sh_show_regs, skip_prologue_hard_way
and do_pseudo_register.  The problem with that when architectures are switched,
these variables are not changed if the architecture that is switched to has
been used before.  This effect is not visible with the current CVS sources
because there is only one value for each of these variables to take, but it
does show with the Sh64 patches.
I think sh_show_regs and skip_prologue_pard_way should be made members of
the gdbarch_tdep structure defined in config/sh/tm-sh.h .

Whereas I see no point in having do_pseudo_register in the first place.
You should be allowed to read SHmedia registers no matter if you are
in SHmedia or SHcompact mode, and a register should not be displayed
in a different format just because it is in compact format.
Moreover, there is unnecessary code duplication between the
*do_pseudo_register and *pseudo_register_read functions.

I propose to:
- add another parameter to the *pseudo_register_read functions to do
  the underlying reading (in place of the hardcoded calls to regcache_read);
  the *register_read functions pass in regcache_read, while the for
  'info reg' read_relative_register_raw_bytes is passed in.
- put the address of the appropriate *pseudo_register_read function into
  a new gdbarch_tdep member.
- sh_print_register uses either read_relative_register_raw_bytes or
  tdep->pseudo_register_read to read the value into a buffer, and
  then proceeds to print it according to its type.
- sh_print_register is used for printing all registers as well as printing
  a particular register.
  The ability to print a particular register only depends on the processor
  type, not the mode the program is currently in.
-- 
--------------------------
SuperH
2430 Aztec West / Almondsbury / BRISTOL / BS32 4AQ
T:+44 1454 462330