> 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.

The ARM regressions pass with this patch. Feel free to do further testing.


- 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

Reply via email to