> On 2011-03-03 20:41:09, Ali Saidi wrote: > > Please don't ship this until I have a chance to try it, I just want to make > > sure it doesn't break ARM_FS/O3. > > Korey Sewell wrote: > Sure, I'd welcome a go of things from some other folks to test if I > haven't introduced something quirky. > > After there is some commentary, I'll make sure to run the full > regressions before committing this because as we all know BaseDynInst is a > pretty fundamental part of M5. > > Also, I'll be posting an update to this diff soon that will make the > set<Int/Float>RegOperand pure virtual (I mistakenly thought those were > templated member functions in the first go-round). > > Ali Saidi wrote: > Anything happen with your update diff? If you could verify it passes the > arm/o3 full system regression I just committed and then I'll give it a go on > a bunch more tests. > > Korey Sewell wrote: > Gabe made a good point about the virtual function overhead on the > commonly used set*Operand functions and I've just been waffling on whether to > even make those pure virtual or not. > > However, I'll go ahead and take a hard look at this again , run the > regressions, and post an update tomorrow so we can move on with this > potential change. > > Korey Sewell wrote: > The ARM regressions pass with this patch. Feel free to do further testing.
What about the virtual function overhead? - Ali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/529/#review932 ----------------------------------------------------------- On 2011-03-01 13:49:24, Korey Sewell wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/529/ > ----------------------------------------------------------- > > (Updated 2011-03-01 13:49:24) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > cpu: split o3-specific parts out of BaseDynInst > The bigger picture goal is that I want to get the InorderDynInst class > derived from the > BaseDynInst, since there is no need to replicate a lot of useful code already > defined > in BaseDynInst (e.g. microcode identification, etc.) and Inorder can take > advantage > of common code that handles microcode and other features that other ISAs need. > > But to do this, there are a lot of o3-specific things that are in > BaseDynInst, that I pushed to > O3DynInst in this patch. Book-keeping variables that handle the IQ,LSQ,ROB > are unnecessary in > the base class but generic variables that will work across CPUs (IsSquashed, > IsCompleted, etc.) > are kept in the base class. > > The upside is more consistency across the simple models (branch prediction > and instruction > identification are all in one common place). > > I really wanted to define pure virtual functions for read/write(to memory) > and the > set<Int/Float>RegOperand, but virtual functions in a templated class is a > no-no and > I couldn't get around that (suggestions?). > > Also, I'd rather not use the "this->" pointer all over the place to access > member variables of > the templated Base class, but it had to be done. > > Other than those quirks, simulator functionality should stay the same as the > O3 Model always references > the O3DynInst pointer and the InOrder model doesnt currently make use of the > base dyn inst. class. > (but it will be easier to derive from now...) > > > Diffs > ----- > > src/cpu/base_dyn_inst.hh cf1afc88070f > src/cpu/base_dyn_inst_impl.hh cf1afc88070f > src/cpu/o3/dyn_inst.hh cf1afc88070f > src/cpu/o3/dyn_inst_impl.hh cf1afc88070f > > Diff: http://reviews.m5sim.org/r/529/diff > > > Testing > ------- > > > Thanks, > > Korey > > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev