----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/616/#review1038 -----------------------------------------------------------
This is a great change. I saw one style mistake, and also I think some code you moved could be simplified further. src/arch/arm/types.hh <http://reviews.m5sim.org/r/616/#comment1409> The type should be on its own line. src/arch/arm/types.hh <http://reviews.m5sim.org/r/616/#comment1410> You could add new fields to the ITSTATE bitunion that would make this easier. cond and mask could be SubBitUnions which can be treated as values on their own or have internal bitfields (syntactically, they still have access to everything). This would then look more like: it.cond.bottom = it.mask.top; it.mask = it.mask << 1; if (it.mask == 0) it.cond = 0; You could use _itstate directly as well which would save a few more lines. - Gabe On 2011-03-30 09:05:10, Ali Saidi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/616/ > ----------------------------------------------------------- > > (Updated 2011-03-30 09:05:10) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > ARM: Cleanup implementation of ITSTATE and put important code in PCState. > > Consolidate all code to handle ITSTATE in the PCState object rather than > touching a variety of structures/objects. > > > Diffs > ----- > > src/arch/alpha/predecoder.hh d54b7775a6b0 > src/arch/arm/faults.cc d54b7775a6b0 > src/arch/arm/isa.cc d54b7775a6b0 > src/arch/arm/isa/insts/data.isa d54b7775a6b0 > src/arch/arm/isa/insts/macromem.isa d54b7775a6b0 > src/arch/arm/isa/insts/misc.isa d54b7775a6b0 > src/arch/arm/isa/operands.isa d54b7775a6b0 > src/arch/arm/isa/templates/macromem.isa d54b7775a6b0 > src/arch/arm/isa/templates/mem.isa d54b7775a6b0 > src/arch/arm/isa/templates/misc.isa d54b7775a6b0 > src/arch/arm/isa/templates/neon.isa d54b7775a6b0 > src/arch/arm/isa/templates/pred.isa d54b7775a6b0 > src/arch/arm/miscregs.hh d54b7775a6b0 > src/arch/arm/predecoder.hh d54b7775a6b0 > src/arch/arm/predecoder.cc d54b7775a6b0 > src/arch/arm/types.hh d54b7775a6b0 > src/arch/mips/predecoder.hh d54b7775a6b0 > src/arch/power/predecoder.hh d54b7775a6b0 > src/arch/sparc/predecoder.hh d54b7775a6b0 > src/arch/x86/predecoder.hh d54b7775a6b0 > src/cpu/o3/fetch_impl.hh d54b7775a6b0 > > Diff: http://reviews.m5sim.org/r/616/diff > > > Testing > ------- > > > Thanks, > > Ali > > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
