Review Board 2.0.15


ARM: Allow conditional quiesce instructions.

Review Request #569 - Created March 11, 2011 and submitted

Information
Ali Saidi
gem5
Reviewers
Default
ali, gblack, nate, stever 
ARM: Allow conditional quiesce instructions.

This patch prevents not executed conditional instructions marked as
IsQuiesce from stalling the pipeline indefinitely. If the instruction
is not executed the quiesceSkip psuedoinst is called which schedules a
wakes up call to the fetch stage.

   

  


 


 

  

  









  
 


  

 
















 



 

  


 


  



   

   
Gabe Black

   
Posted  (March 11, 2011, 10:58 a.m.)


  


  


  

   
Why are you fixing up the fact that the CPU shouldn't have quiesced instead of preventing it from doing it in the first place? If the instruction is being executed speculatively, you should mark it as non-speculative. If O3 is basing its behavior off of the instruction's flags instead of its behavior (ie. if it actually called quiesce), I'd argue it shouldn't be doing that.



  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    Because for the CPU to quiesce it needs to stall early (in fetch) and it does so by testing the IsQuiesce flag of the instruction. Even if it's not speculative that doesn't solve the problem for us if the instruction is predicated false. So after we end up processing the instruction if it was predicated false we need to wake up the fetch stage which had just quiesced. 
    
    
    
   
    2. 


   
  2. 
    
    
     
    
      
     
    
     
    So, why does it need to quiesce in fetch? Can't quiesce act like a fault (sort of) and throw out non-committed state?
    
    
    
   
    3. 


   
  3. 
    
    
     
    
      
     
    
     
    It's just the way the O3 cpu quiesces. we didn't add that feature, we're just using it. 
    
    
    
   
    4. 


   
  4. 
    
    
     
    
      
     
    
     
    That's probably true, but fixing it in the "wrong" way just spreads the problem somewhere else rather than getting rid of it.
    
    
    
   
    5. 

   


  




   







    

     
     

      

       
        

         
          src/arch/arm/isa/insts/misc.isa
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
This should be on the same line as the }




  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    k
    
    
    
   
    2. 

   


  


    


    

     
     

      

       
        

         
          src/sim/pseudo_inst.hh
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
This isn't really a pseudo instruction at all, it's a generic call to fix up a pseudo instruction that should never have been called.




  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    well, it's the most logical place for the code given what it does. 
    
    
    
   
    2. 


   
  2. 
    
    
     
    
      
     
    
     
    Maybe it would be better to call it something like cancelQuiesce? To me that sounds more like it's subordinate to quiesce and not a separate peer to it. I'm sure that's mostly a matter of opinion.
    
    
    
   
    3. 

   


  


    


    

     
     

      

       
        

         
          src/sim/pseudo_inst.cc
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
The year is wrong, unless this is an older patch.