Review Board 2.0.15


sparc: compilation fixes for inorder

Review Request #583 - Created March 14, 2011 and submitted

Information
Korey Sewell
gem5
default
Reviewers
Default
ali, gblack, nate, stever
sparc: compilation fixes for inorder
Add a few constants and functions that the InOrder model wants for SPARC.
* * *
sparc: add eaComp function
InOrder separates the address generation from the actual access so give
Sparc that functionality
* * *
sparc: add control flags for branches
branch predictors and other cpu model functions need to know specific information
about branches, so add the necessary flags here

   
Review request changed
Updated (March 14, 2011, 10:39 a.m.)

Description:

   

sparc: compilation fixes for inorder

    Add a few constants and functions that the InOrder model wants for SPARC.

   
   
   
   

sparc: add eaComp function

    InOrder separates the address generation from the actual access so give
    Sparc that functionality

   
   
   
   

sparc: add control flags for branches

    branch predictors and other cpu model functions need to know specific information
    about branches, so add the necessary flags here

-  
-  

(this is really 3 patches in one, for some reason, reviewboard isnt letting me post the patches consecutively)

Diff:

Revision 2 (+134 -4)

Show changes

Posted (March 16, 2011, 8:17 a.m.)
There are a number of style issues to clean up, but otherwise this seems mostly ok. I'm still not crazy about having an mt.hh that's only used in InOrder and MIPS, as far as I'm aware. That's ISA specific behavior masquerading as something generic, but that's probably a separate issue from this change.
  1. Agreed on the mt.hh file, I've been thinking of ways to mask that out. I need to double check, but if those are just per-thread RegFile calls are just triggered through instruction calls then maybe the ISA description can handle it.
src/arch/sparc/isa/decoder.isa (Diff revision 2)
 
 
Needs a space between the comma and None. Should this be marked as IsUncondControl as well?
You shouldn't add this. This isn't really a C++ file anyway.
src/arch/sparc/isa/formats/mem/swap.isa (Diff revision 2)
 
 
This line is too long. It needs to be split up. Better yet, exec_output should be appended before the return.
src/arch/sparc/isa/formats/mem/util.isa (Diff revision 2)
 
 
Checking for faults here (for SPARC at least) isn't necessary as far as I remember.
src/arch/sparc/isa/formats/mem/util.isa (Diff revision 2)
 
 
Checking for a fault here is probably also unnecessary.
src/arch/sparc/mt.hh (Diff revision 2)
 
 
Some of these header files don't need to be included.
src/arch/sparc/mt.hh (Diff revision 2)
 
 
Never put "using namespace" in a header file.
src/arch/sparc/mt.hh (Diff revision 2)
 
 
This isn't really a user configuration problem since there's no way to even request multithreaded behavior like this in SPARC. I think these should be panics.
src/arch/sparc/registers.hh (Diff revision 2)
 
 
This seems redundant. Can't the CPU model add them up just as easily?
  1. The CPU Model could calculate these but it would be the same line wherever you put it. 
    
    I'm not sure it's redundant though, since there isn't necessarily a constant that just encapsulates all the registers available and there other places throughout the code where we are adding constants together to make a easy generic term to use for other objects to draw form.
    
    Overall, I thought this was the right place because all the register dependency tracking and sizing of Register Files basically uses this file's constants.
  2. Ok. It's not a huge deal where exactly it ends up, although it would be nice to put it in a common place since it'll (most likely) be the same everywhere. Maybe in cpu/base.hh or something like that. That's not necessarily the perfect place since it's not really a constant that belongs to the CPU. It's ok to leave it here at least for now.