This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Unreviewed patches


Joern Rennecke writes:
 > Elena Zannoni wrote:
 > > For the gdb sh-tdep.c part, I have a few comments.
 > > There is no need for a prototype. I removed all of those a few months back.
 > 
 > Ok, I'll place the new function so that it needs no prototype.
 > But you have to keep in mind that that makes the call graph more
 > rigid, unless you like to move code around a lot.
 > 

Thanks. I did a major clean up recently, and things should be ok for a while.
If I find out that I have to introduce a prototype in the future because
there is no other reasonable choice, I'll revisit the decision then.

 > > You should just move the new function to before the gdbarch_init one.
 > > Instead of adding another enum, you should use
 > > 
 > >   struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
 > >  and then
 > >   tdep-><...>_REGNUM
 > > 
 > > If you need new register numbers for the dsp case, just add
 > > them to the tdep structure.
 > 
 > I think the stashing of constants into the tdep structure is basically
 > wrong.  You separate the register names arrays from the literals
 > that describe their positions, and you replicate the literals
 > up to four times.  The tdep structure and the sh_gdbarch_init
 > function are so large that you have lost track of the things that
 > really belong in tdep, like sh_show_regs, skip_prologue_hard_way,
 > and do_pseudo_register.  If you look at other gdb ports, you'll
 > see that they put only variable stuff in tdep, and use enums
 > for constants.  The sh gdb register naming scheme also doesn't
 > scale well, the names are again duplicated multiple times.
 > 	

Tdep contains the things that change between different flavors of the
architecture, when you say 'set architecture blah'. The register
numbers belong to that category. ppc does the same.

 > I have a scheme that names each register just once, and enumerates
 > it only once - in an enum, of course.  But I thought we should first
 > sort out the simulator <-> gdb and gcc -> gdb interfaces before we
 > start on the gdb internals.
 > 	

Definitely, focussing on the interface issues is time better spent
right now.

Elena


 > -- 
 > --------------------------
 > SuperH
 > 2430 Aztec West / Almondsbury / BRISTOL / BS32 4AQ
 > T:+44 1454 462330


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]