Review Board 2.0.15


ruby: improved component mapping

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

Information
Brandon Potter
gem5
default
Reviewers
Default 
Changeset 11435:9b3faddc9c0b
---------------------------
ruby: improved component mapping

This patch aims are improving how addresses are mapped to different entities
in the memory system.  The main issue was having a static member function in
the DirectoryMemory class and several different globally visible functions
almost doing the same thing.  The functions map_Address_to_DirectoryNode,
map_Address_to_Directory, and broadcast() are being dropped. There is no
replacement provided for broadcast(), but one should be able to use the one
already available provided in the NetDest class.  The functions
map_Address_to_DirectoryNode and map_Address_to_Directory should be replaced
with mapAddressToRange().

   

  


 


 

  

  






 
 
Issue Summary

 

  

   
    
    1
   
   
    
    0
   
   
    
    0
   
   
    
    1
   
  

  

   

    
    
   

   

    
    
   

  

 

 

  

   
    

     Description
     From
     Last Updated
     Status
    

   
   

















    







   
  

 









  
 


  

 


















 

  


 


  



   

   
Andreas Hansson

   
Posted  (April 6, 2016, 2:43 p.m.)


  


  


  

   
No objections, but a high-level question: Should the long-term play here be to use the "normal" gem5 address mapping functionality embedded in the AddrRange class? This class (along with the XBar) already has support for interleaving and hashing.



  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    With these patches that handle routing based on certain ranges of the address, it seems that this is well aligned for that adoption. I'm not too familiar with how classic does it's mappings, but I did briefly look at the AddrRange class and I suspect that I understand what you mean. It remains to be seen if this patch will go through, but it's what I am pushing right now.  The address based mapping with bitSelect seems like a good way to handle the routing problems.
    

    You can thank Nilay for this solution if you see him on the boards anytime soon; kudos go to him for this.
    
    
    
   
    2. 


   
  2. 
    
    
     
    
      
     
    
     
    Are you suggesting it's not worth merging the two at this point?
    
    
    
   
    3. 


   
  3. 
    
    
     
    
      
     
    
     
    We are discussing the AddrRange proposal internally. I think it can be done (and probably should be done); though, we haven't tried to do it yet.
    

    However, I don't think that it should become a prerequisite for accepting this change into the source. If that change works out, it will come as a subsequent patch.
    
    
    
   
    4. 


   
  4. 
    
    
     
    
      
     
    
     
    Agreed. I was mostly hoping that we can get on a trajectory that leads to less overlap
    
    
    
   
    5. 

   


  





  

 


  

 

















 



 

  


 


  



   

   
Joel Hestness

   
Posted  (April 14, 2016, 8:32 a.m.)


  


  


  

   
I'm really not a fan of the way we're trying to do this. To summarize, this change distributes address mapping parameters through all of the Ruby controllers, just so that they can call a global function that takes those as parameters and returns the mapping.


This leads to a few problems. First, we'll have to change all of the protocol files, and get the correct parameters to the correct controllers in config files. This will be painful for users with protocols outside of mainline, because they need to reason about how to calculate correct parameters. Second, it's a confusing way to distribute mapping parameters; they are only used in calls to mapAddressToRange, which is a global function. The fact that mapAddressToRange is global indicates that there a number of ways we could organize this, some of which would likely avoid spreading parameters throughout protocol files. Finally, this is a very fragile way to handle component mapping, so it won't be extensible. What do I do if I notice that I need more complicated parameterization of the directory mapping function? It looks like I'd have to extend the function signature for mapAddressToRange to take more parameters, and then distribute more parameters through the relevent protocol files. This sounds very disruptive.


I think we should be aiming to implement this by moving mapAddressToRange into the RubySystem. Then, we can just give each RubySystem instance the number of controllers that it needs to map, and it can configure all of the appropriate mapping functions and parameters. The existing mapping functions used in protocol files can indirectly call the RubySystem's mapping functions as appropriate, so we shouldn't need to change protocol files (or if we do, it would be a 1-time change). This also encapsulates the mapping functionality in a single location, so modifying mapping functions and parameters should be far more extensible.



  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    I think that it might be reasonable to move the component mapping functionality into RubySystem because the controllers already have a handle for RubySystem. However, I have a couple concerns:
    

    "What do I do if I notice that I need more complicated parameterization of the directory mapping function? It looks like I'd have to extend the function signature for mapAddressToRange to take more parameters, and then distribute more parameters through the relevent protocol files. This sounds very disruptive."
    

    Can you provide an example of what you're suggesting for the directory mapping? I'm not saying that it's impossible/unreasonable, but I am not aware of what you'd want to use as the other mapping methods. Currently, we should be able to support the address interleaving method using mapAddressToRange (which looks like a banked cache/memory) and a direct method where a private cache sends to a downstream private cache. We could extend it further if needed so that we have a private cache send to a second private cache that is banked. I believe that would match the functionality that the classic cache model provides. (Andreas, feel free to correct me if I'm mistaken; I'm not very familiar with classic.)
    

    "Then, we can just give each RubySystem instance the number of controllers that it needs to map, and it can configure all of the appropriate mapping functions and parameters."
    

    By default, RubySystem does not track the controllers; the controllers were previously tracked in a global. With my first attempt at this problem, I moved that global into RubySystem and the controllers were added into it as they were initialized. The RubySystem was aware of the controllers and could handle queries to with the Machine* functions because they were being routed through the RubySystem into it's controller vector. The controller vector was a 2-D array which consisted of a vector of the MachineTypes each of which held a vector of controllers for that given MachineType. So, it's possible to do as you're suggesting, although it's not as simple as you're making it out to be.
    

    END CONCERNS
    

    If and only if we find that the mapAddressToRange is not flexible enough, I'm willing to try an alternative that you suggest. First, please look at http://reviews.gem5.org/r/3113/ again. Before I do any more coding on this, I want a clear-cut path that everyone agrees on for a component mapping. I am not going to develop another implementation unless there's total agreement on how the solution looks. Andreas, a more flexible implementation probably won't be using AddrRange. If you really believe that Ruby should use it, you might speak up here. Also, you might have an opinion on why it's sufficient for classic (if I understand everything correctly).
    
    
    
   
    2. 


   
  2. 
    
    
     
    
      
     
    
     
    

    

    

    

    

    

    "What do I do if I notice that I need more complicated parameterization of the directory mapping function? It looks like I'd have to extend the function signature for mapAddressToRange to take more parameters, and then distribute more parameters through the relevent protocol files. This sounds very disruptive."
    

    

    

    

    

    

    Can you provide an example of what you're suggesting for the directory mapping? I'm not saying that it's impossible/unreasonable, but I am not aware of what you'd want to use as the other mapping methods. Currently, we should be able to support the address interleaving method using mapAddressToRange (which looks like a banked cache/memory) and a direct method where a private cache sends to a downstream private cache. We could extend it further if needed so that we have a private cache send to a second private cache that is banked. I believe that would match the functionality that the classic cache model provides. (Andreas, feel free to correct me if I'm mistaken; I'm not very familiar with classic.)
    

    

    As a practical example, in gem5-gpu, we have a really hacky solution for complicated bit transformations to avoid pathological memory access patterns. A common issue is when GPU threads process rows of a matrix: Suppose a row is 4096 bytes (12 address bits between row starts), and the directory address mapping high bit is the 11th bit (0-indexed). Then, all GPU memory accesses in a small time window can end up going to the same directory and cause huge contention. We deal with this in gem5-gpu by swizzling bits above and below the mapping "high bit", but parameterizing and modifying the current directory mapping scheme was exceptionally error prone, and I don't want to ever modify the mapping again.
    

    More abstractly, we should be aiming to augment Ruby so that we can define and swap out nearly arbitrary mapping functions. If we home the address mapping functions in a single place, like the RubySystem, then we can easily parameterize the mapping function selection (e.g. similar to passing a cache replacement policy to a cache). As Andreas suggests, I too believe the AddrRange functionality can be eventually used in the mapping functionality (note: AddrRanges have 4 parameters that define the mapping, indicating it is desirable to have more flexibility than Ruby mappings). To me, this is very desirable functionality.
    

    

    

    

    

    

    

    

    "Then, we can just give each RubySystem instance the number of controllers that it needs to map, and it can configure all of the appropriate mapping functions and parameters."
    

    

    

    

    

    

    By default, RubySystem does not track the controllers; the controllers were previously tracked in a global. With my first attempt at this problem, I moved that global into RubySystem and the controllers were added into it as they were initialized. The RubySystem was aware of the controllers and could handle queries to with the Machine* functions because they were being routed through the RubySystem into it's controller vector. The controller vector was a 2-D array which consisted of a vector of the MachineTypes each of which held a vector of controllers for that given MachineType. So, it's possible to do as you're suggesting, although it's not as simple as you're making it out to be.
    

    

    I don't think any of my proposed solutions will require the RubySystem to have pointers to the separate controllers. To handle address mapping, it will just need the count of each controller type for address striping across them. The address mapping functions can still pass MachineIDs or NetDests back to the calling controllers.
    

    

    

    

    

    If and only if we find that the mapAddressToRange is not flexible enough, I'm willing to try an alternative that you suggest. First, please look at http://reviews.gem5.org/r/3113/ again. Before I do any more coding on this, I want a clear-cut path that everyone agrees on for a component mapping. I am not going to develop another implementation unless there's total agreement on how the solution looks. Andreas, a more flexible implementation probably won't be using AddrRange. If you really believe that Ruby should use it, you might speak up here. Also, you might have an opinion on why it's sufficient for classic (if I understand everything correctly).
    

    

    I believe everything I'm describing in this current set of reviews is consistent with my recommendations in 3113. We can aim for a solution that uses AddrRanges in later iterations - we need more flexible address mapping structure first.
    
    
    
   
    3. 

   


  




   







    

     
     

      

       
        

         
          configs/ruby/GPU_RfO.py
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
Please maintain spacing around '='. It's really hard to read something like tcc_low_bit=block_size_bits, and it's inconsistent with spacing through the rest of the file (e.g. Cluster constructors)


     





 

  
  
  
  
  
  Show all issues
 




     




  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    Maybe I am interpreting it wrong but doesn't this style guide rule apply here?
    
http://www.m5sim.org/Coding_Style - "no space around '=' when used in parameter/argument lists, either to bind default parameter values (in Python or C++) or to bind keyword arguments (in Python)"
    
    
    
   
    2. 


   
  2. 
    
    
     
    
      
     
    
     
    Ah, apologies. I wasn't aware we had adopted the Python/Google style for this. I dropped this issue.
    

    Eventually, we'll need to make '=' spacing consistent within this file, but note that our use of '=' spacing in named parameters lists is quite inconsistent throughout gem5.
    
    
    
   
    3.