----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/205/#review286 -----------------------------------------------------------
Overall, I have to say that this is a much better way to do things. Did you consider using the Flags class in src/base/flags.hh? There are some examples of how it's used in sim/eventq.hh and mem/packet.hh. I'm not sure if it's overkill or not. I'm also not sure if adding any checking to the setting/getting of flags is worth it, but I thought I'd point it out. Also, setFlags as a parameter name seems strange. Why not just call it "flags"? - Nathan On 2010-08-22 18:53:02, Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/205/ > ----------------------------------------------------------- > > (Updated 2010-08-22 18:53:02) > > > Review request for Default. > > > Summary > ------- > > X86: Consolidate extra microop flags into one parameter. > > This single parameter replaces the collection of bools that set up various > flavors of microops. A flag parameter also allows other flags to be set like > the serialize before/after flags, etc., without having to change the > constructor. > > > Diffs > ----- > > src/arch/x86/insts/microfpop.hh 405f840c4ae1 > src/arch/x86/insts/microldstop.hh 405f840c4ae1 > src/arch/x86/insts/micromediaop.hh 405f840c4ae1 > src/arch/x86/insts/microop.hh 405f840c4ae1 > src/arch/x86/insts/microregop.hh 405f840c4ae1 > src/arch/x86/isa/macroop.isa 405f840c4ae1 > src/arch/x86/isa/microops/base.isa 405f840c4ae1 > src/arch/x86/isa/microops/debug.isa 405f840c4ae1 > src/arch/x86/isa/microops/fpop.isa 405f840c4ae1 > src/arch/x86/isa/microops/ldstop.isa 405f840c4ae1 > src/arch/x86/isa/microops/limmop.isa 405f840c4ae1 > src/arch/x86/isa/microops/mediaop.isa 405f840c4ae1 > src/arch/x86/isa/microops/regop.isa 405f840c4ae1 > src/arch/x86/isa/microops/seqop.isa 405f840c4ae1 > src/arch/x86/isa/microops/specop.isa 405f840c4ae1 > > Diff: http://reviews.m5sim.org/r/205/diff > > > Testing > ------- > > > Thanks, > > Gabe > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
