I think there's been a lot of aversion to virtual functions in the past because they add some performance overhead. You've got the indirection through what's basically a function pointer, and you can't really inline the function because you don't know what it's going to be ahead of time. The set*Operand functions are pretty frequently called, too. For more complex CPUs like O3 and InOrder it might all get lost in the noise, but please at least measure what the impact is performance wise.
Gabe On 03/03/11 21:06, Korey Sewell wrote: > >> 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. > 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). > > > - Korey > > > ----------------------------------------------------------- > 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 _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
