Review Board 2.0.15


ruby: change function signature for functional_read/write

Review Request #3422 - Created April 4, 2016 and updated

Information
Brandon Potter
gem5
default
Reviewers
Default 
Changeset 11424:6901103f7f04
---------------------------
ruby: change function signature for functional_read/write

The functional_read and function_write functions within Ruby will
occasionally use methods which rely on knowing the block size so we add
the block size as a parameter.

To avoid special casing large sections of the SLICC code, we propagate the
change to all instances of these functions (even when the parameter is
not used).

   

  


 


 

  

  









  
 


  

 
















 



 

  


 


  



   

   
Jason Lowe-Power

   
Posted  (April 11, 2016, 1:51 p.m.)


  


  


  

   
Could you point out where this parameter is used? I'm having a hard time finding it in this patch.


Without seeing where the parameter is used, I feel like there has to be a better way! Surely we can find a way to modify the few places this information is needed rather than changing every file.



  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    Seems I jumped the gun here. This, /r/3423 and /r/3424 will be combined? To double check that I understand, the reason this is needed is for makeLineAddress, etc? Could you provide a list of these functions?
    

    Maybe we should take this off reviewboard. Could you send an email with the underlying reason you need to pass this new parameter around everywhere. Since it's split across so many patches, I'm having a hard time following.
    

    I really don't like the idea of passing around this parameter everywhere. I'd like to understand better the problem so I can try my hand at a cleaner solution.
    
    
    
   
    2. 


   
  2. 
    
    
     
    
      
     
    
     
    Right, I broke the patches apart so that it was easier to isolate the changes. The goal was to split the patches so that they compile/run seperately while maintaining some distinction between them; this is especially true for the block size patches which amount to thousands of patch lines if they're all combined.
    

    The total ordering of all of the Ruby patches is as follows:
    
1)  http://reviews.gem5.org/r/3413/ - DPRINTF newline
    
2)  http://reviews.gem5.org/r/3414/ - unnecessary !!
    
3)  http://reviews.gem5.org/r/3415/ - #ifdef 0 removal
    
4)  http://reviews.gem5.org/r/3416/ - intermediate TBE transitions (MESI_{Three,Two}_Level, MOESI_CMP_directory, MOESI_hammer)
    
5)  http://reviews.gem5.org/r/3417/ - for TBETable, use ENTRY* instead of Entry
    
6)  http://reviews.gem5.org/r/3418/ - SLICC pointer naming convention change
    
7)  http://reviews.gem5.org/r/3419/ - SubBlock removal
    
8)  http://reviews.gem5.org/r/3420/ - filter's parameter list change (string -> types)
    
9)  http://reviews.gem5.org/r/3421/ - preliminary addition of block_size_{bits,bytes} to objects
    
10) http://reviews.gem5.org/r/3422/ - functional read/write parameter changes
    
11) http://reviews.gem5.org/r/3423/ - add block_size_bits to testAnd* functions
    
12) http://reviews.gem5.org/r/3424/ - add block_size_{bytes, bits} to the address functions
    
13) http://reviews.gem5.org/r/3425/ - write mask initialization
    
14) http://reviews.gem5.org/r/3426/ - data block initialization
    
15) http://reviews.gem5.org/r/3427/ - SLICC grammer addition (new w/ constructor for new ENTRY(block_size))
    
16) http://reviews.gem5.org/r/3428/ - pass block_size into ENTRY constructors
    
17) http://reviews.gem5.org/r/3429/ - change PerfectCache to use ENTRY* instead of ENTRY
    
18) http://reviews.gem5.org/r/3430/ - remove RubySystem static for block_size
    
19) http://reviews.gem5.org/r/3431/ - convert global stats variables to locals
    
20) http://reviews.gem5.org/r/3432/ - add helper method for stats printing
    
21) http://reviews.gem5.org/r/3433/ - component mapping changes
    
22) http://reviews.gem5.org/r/3434/ - remove unused L3Cntrl in Python files
    
23) http://reviews.gem5.org/r/3435/ - consolidate CntrlBase definitions and add clear method to CntrlBase
    
24) http://reviews.gem5.org/r/3436/ - topology - avoid Machine* functions
    
25) http://reviews.gem5.org/r/3437/ - network/netdest - avoid Machine* functions
    
26) http://reviews.gem5.org/r/3438/ - allows used of uninitialized sets for NetDest
    
27) http://reviews.gem5.org/r/3439/ - Packet* to PacketPtr change
    
28) http://reviews.gem5.org/r/3440/ - add L0 machine type to machine type enumeration class (for MESI_Three_Level)
    
29) http://reviews.gem5.org/r/3441/ - MESI_Three_Level - make the L0 cache visible to the Ruby network
    
30) http://reviews.gem5.org/r/3442/ - add scripts to invoke multi-instance and left-over miscellaneous fixes
    
31) http://reviews.gem5.org/r/3443/ - garnet fixed model bug fix
    

    The block size related changes are from 9-18. With patches 9-12, I add in block_size parameters in a structured way that doesn't prevent compilation/running. It should also help readability tremendously; the alternative is to have them put into a single patch which might be hard to understand.
    
    
    
   
    3. 


   
  3. 
    
    
     
    
      
     
    
     
    To follow the block_size parameter changes, I'd look at 9-12 definitely and perhaps 13 and 14. The key to understanding the changes is that the functions used in 12 are used alot.
    
    
    
   
    4.