Review Board 2.0.15


X86: Consolidate extra microop flags into one parameter.

Review Request #205 - Created Aug. 22, 2010 and submitted

Information
Gabe Black
gem5
Reviewers
Default
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.

   
Posted (Aug. 22, 2010, 11:59 p.m.)
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"?
  1. I thought about using the Flags class, but it seemed like overkill like you said. I also considered adding some sort of check that would prevent bad flags, contradictory flags, etc., but that seemed like it'd be hard to maintain. I chose setFlags because it's really just the flags you want to ensure are set. If there are other flags already set automatically for some reason then you wouldn't be able to clear them with that parameter.