Review Board 2.0.15


CPU: Add functions to get the number of executed instructions and set the

Review Request #51 - Created July 9, 2010 and updated

Information
Timothy Jones
gem5
Reviewers
Default 
CPU: Add functions to get the number of executed instructions and set the
maximum number of instructions to execute to the CPUs and allow them to be
called from python.

   

  


 


 

  

  









  
 


  

 
















 



 

  


 


  



   

   
Steve Reinhardt

   
Posted  (July 10, 2010, 1:52 a.m.)


  


  


  

   



  

   

   
    
    
   


  




   







    

     
     

      

       
        

         
          src/cpu/base.cc
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
Is there a reason to allocate this dynamically instead of having a single static counter per CPU object?




  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    No, not at all.  I'm not sure why I did that but I can alter it.
    
    
    
   
    2. 

   


  


    


    

     
     

      

       
        

         
          src/sim/eventq.hh
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
I don't like getting rid of this assertion... it's actually pretty useful in knowing when something's not right.  You should add some code upstream somewhere to skip adding the event if we're already past it.




  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    So the main problem with this assertion is that the event I'm trying to add is triggered by an instruction count, not a certain Tick.  I'm happy to either add code somewhere else to avoid adding old events or to put this assertion back in and find a new way of adding instruction-count-based events.  Which would be the preferred option?
    
    
    
   
    2. 


   
  2. 
    
    
     
    
      
     
    
     
    I understand that... but even with instructions, I think you'd have the same problem, which is that you're scheduling an event that's never going to get triggered.  So if you want a callback after 1000 insts are executed, but 2000 insts have already been executed, it seems like you'd still need to check somewhere before you call schedule() and realize that there's no point in calling schedule() because this event is not going to happen.  I don't think instruction-based events should be treated differently.
    
    
    
   
    3. 


   
  3. 
    
    
     
    
      
     
    
     
    I ran into this problem a few weeks ago and have just removed the assertion in my tree. It's not ideal, but i had no idea how to solve it in any generic fashion. The best I could come up with was adding a parameter to the event queue constructor that specified it wasn't tick based, although getting to current count to compare it to is still problematic.

    
    
    
   
    4. 


   
  4. 
    
    
     
    
      
     
    
     
    I see... the problem is that it's comparing with curTick and not the actual thing that's being used to drive the queue processing.  Duh, I missed that before.  We'll run into a similar problem when we parallelize the simulation and have per-thread definitions of curTick.  Seems like we need to have the EventQueue class have a pointer to the curTick-equivalent variable for that queue... is it reasonably straightforward to provide that at the point where each queue is created?  Is there any scenario in which that wouldn't work, such as a queue would switch variables midstream?  I don't think so.
    
    
    
   
    5. 


   
  5. 
    
    
     
    
      
     
    
     
    I actually believe that curTick shouldn't be a global variable.  The class itself should have an internal curTick variable that can be accessed.  (Probably better if it were a curTick() function).  That said, if we leave it as a variable, we could make curTick a Tick & to mainEventQueue.current.  As Steve alluded to though, we're going to run into issues will parallelization.
    
    
    
   
    6. 


   
  6. 
    
    
     
    
      
     
    
     
    So I think the only question is whether EventQueue should contain the tick variable or just have a pointer/reference to it... for things like instruction-count-based queues in particular it makes more sense for the counter to live outside and have the queue just reference it.
    
    
    
   
    7. 

   


  


    


    

     
     

      

       
        

         
          src/sim/sim_object.cc
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
I'd prefer a more informative message like "Error: setMaxInsts called on non-CPU" (and same with the following function).

There's a broader question of whether this is how we want to expose subclass-specific methods to Python... I don't have any great ideas for alternatives, but Nate might.




  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    I can definitely alter the error message here and can make alterations to the way this is exposed to python, if necessary.
    
    
    
   
    2. 


   
  2. 
    
    
     
    
      
     
    
     
    Thanks... changing the error message is all I need to be happy.  I'd like to hear Nate's opinion on whether it's good to end up with a bunch of class-specific methods on SimObject, and what the alternatives might be, but until he speaks up you can leave the rest of it like it is.
    
    
    
   
    3. 


   
  3. 
    
    
     
    
      
     
    
     
    Actually, I don't think these should be done that way.  System.py, and src/python/swig/system.i Illustrate another way.  It's somewhat messy if we want to do that a lot though.  We could try to generalize it.
    
    
    
   
    4. 


   
  4. 
    
    
     
    
      
     
    
     
    We still have this in SimObject.py:

    def getMemoryMode(self):
        if not isinstance(self, m5.objects.System):
            return None

        return self._ccObject.getMemoryMode()

    def changeTiming(self, mode):
        if isinstance(self, m5.objects.System):
            # i don't know if there's a better way to do this - calling
            # setMemoryMode directly from self._ccObject results in calling
            # SimObject::setMemoryMode, not the System::setMemoryMode
            self._ccObject.setMemoryMode(mode)

I don't see why those methods are necessary; assuming they're not, and that we get rid of them, and that there are only a limited number of classes that define their own python methods, then I think this approach is not too bad.  How would you generalize it?

    
    
    
   
    5.