----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/669/#review1181 -----------------------------------------------------------
src/arch/arm/nativetrace.cc <http://reviews.m5sim.org/r/669/#comment1619> You should divide by two here instead of shifting by 1. It's more obvious what you're doing, and the compiler will be smart enough to use a shift it it's faster. src/arch/arm/nativetrace.cc <http://reviews.m5sim.org/r/669/#comment1618> spaces around the + util/statetrace/arch/arm/tracechild.cc <http://reviews.m5sim.org/r/669/#comment1621> Hmm... I wonder how that got there? Good catch. util/statetrace/arch/arm/tracechild.cc <http://reviews.m5sim.org/r/669/#comment1622> I don't know how easy this would be to accommodate, but you're going to be sending a bunch of extra zeros for int regs that aren't 64 bits wide. Can you make it so you send full 64 bit values only when the source is actually 64 bits wide? util/statetrace/arch/arm/tracechild.cc <http://reviews.m5sim.org/r/669/#comment1623> The idea is to verify that you're not falling off of uregs. Maybe you could do something more flexible like sizeof(myregs.uregs) / sizeof (myregs.uregs[0]). util/statetrace/arch/arm/tracechild.cc <http://reviews.m5sim.org/r/669/#comment1624> The same comment applies as in getRegs, except that you have to deal with an offset. It would probably be a good idea to define something in the enum to mark the start of the FP regs. You can move the assert to after the if and subtract out the offset right before indexing fpregs. util/statetrace/arch/arm/tracechild.cc <http://reviews.m5sim.org/r/669/#comment1625> Just because libc would use a macro doesn't mean we have to. You should replace this with a constant of the appropriate type. util/statetrace/arch/arm/tracechild.cc <http://reviews.m5sim.org/r/669/#comment1626> Is the cast actually necessary here? I can believe it is to avoid a warning, but you could try leaving it out if you haven't already. - Gabe On 2011-05-02 15:41:28, Ali Saidi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/669/ > ----------------------------------------------------------- > > (Updated 2011-05-02 15:41:28) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > ARM: Add vfpv3 support to native trace. > > > Diffs > ----- > > src/arch/arm/nativetrace.hh 3f49ed206f46 > src/arch/arm/nativetrace.cc 3f49ed206f46 > util/statetrace/arch/arm/tracechild.hh 3f49ed206f46 > util/statetrace/arch/arm/tracechild.cc 3f49ed206f46 > > Diff: http://reviews.m5sim.org/r/669/diff > > > Testing > ------- > > > Thanks, > > Ali > > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
