> On 2010-11-09 11:50:33, Gabe Black wrote: > > Instead of packing/unpacking 32 bit values in 64 bit registers, why don't > > we template the base class on the type of register it's going to hold. I > > think the changes to the base class would be minimal, and it would simplify > > this ARM code nicely with almost no impact on the other ISAs.
I sort of agree, but I don't think it's worth the time. The remote gdb code is already a disaster area and putting a bandaid on it doesn't really help much. It was quite a pain to make sure this was correct and test every register, so I'd prefer to leave it this way until some overall restructuring of the remote gdb code occurs. > On 2010-11-09 11:50:33, Gabe Black wrote: > > src/arch/arm/remote_gdb.hh, line 51 > > <http://reviews.m5sim.org/r/293/diff/1/?file=5061#file5061line51> > > > > If you're not going to indent the constants above (which seems to be > > the preferred way to do things) then you should unindent this. I think it's > > better to be consistent that inconsistently correct. Fixed. > On 2010-11-09 11:50:33, Gabe Black wrote: > > src/arch/arm/remote_gdb.cc, line 165 > > <http://reviews.m5sim.org/r/293/diff/1/?file=5062#file5062line165> > > > > This is already happening in the base class constructor. Fixed. - Ali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/293/#review446 ----------------------------------------------------------- On 2010-11-08 15:35:16, Ali Saidi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/293/ > ----------------------------------------------------------- > > (Updated 2010-11-08 15:35:16) > > > Review request for Default. > > > Summary > ------- > > imported patch ext/arm_gdb.patch > > > Diffs > ----- > > src/arch/arm/SConscript f61e079ad05e > src/arch/arm/remote_gdb.hh f61e079ad05e > src/arch/arm/remote_gdb.cc PRE-CREATION > src/arch/arm/utility.hh f61e079ad05e > src/arch/arm/utility.cc f61e079ad05e > > Diff: http://reviews.m5sim.org/r/293/diff > > > Testing > ------- > > > Thanks, > > Ali > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
