Review Board 2.0.15


O3: Make all instructions that write a misc register not perform the write until commit.

Review Request #298 - Created Nov. 8, 2010 and submitted

Information
Ali Saidi
gem5
Reviewers
Default 
O3: Make all instructions that write a misc register not perform the write until commit.

ARM Instructions updating cumulative flags (ARM FP exceptions and saturation
flags) are not serialized.

Added aliases for ARM FP exceptions and saturation flags in FPSCR.

   

  


 


 

  

  









  
 


  

 


















 

  


 


  



   

   
Gabe Black

   
Posted  (Nov. 9, 2010, 5:05 a.m.)


  


  


  

   
Generally this seems ok, but there are some relatively minor issues to look at. This is O3, but still have you checked how this affects performance? I'd imagine it's not huge since there's so much else going on, but it would happen with every inst so it might add up.



  

   

   
    
    
   


  




   







    

     
     

      

       
        

         
          src/arch/arm/isa.cc
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
I defined types for these (specifically FPSCR in miscregs.hh) so you can use that instead of manual bitmasks and oring.




  

   

   
    
    
   


  


    


    

     
     

      

       
        

         
          src/arch/arm/isa/insts/fp.isa
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
You could use the operands list to make anything writing to Fpscr nonspeculative/serialize after/whatever. That might be safer since you'd be protected from accidentally leaving it off.




  

   

   
    
    
   


  


    


    

     
     

      

       
        

         
          src/arch/arm/isa/insts/neon.isa
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
Is casting to FPSCR actually necessary? Part of the idea is that the bitunion types are (to the first order) interchangable with the bare underlying types. I didn't manage to make it seamless, granted, but it should handle the assignment. If not that may be a correctable bug.




  

   

   
    
    
   


  


    


   



  

 


  

 



















 

  


 


  



   

   
Gabe Black

   
Posted  (Nov. 9, 2010, 5:05 a.m.)


  


  


  

   
Generally this seems ok, but there are some relatively minor issues to look at. This is O3, but still have you checked how this affects performance? I'd imagine it's not huge since there's so much else going on, but it would happen with every inst so it might add up.



  

   

   
    
    
   


  





  

 


  

 



















 

  


 


  



   

   
Gabe Black

   
Posted  (Nov. 9, 2010, 5:06 a.m.)


  


  


  

   
Generally this seems ok, but there are some relatively minor issues to look at. This is O3, but still have you checked how this affects performance? I'd imagine it's not huge since there's so much else going on, but it would happen with every inst so it might add up.



  

   

   
    
    
   


  





  

 


  

 



















 

  


 


  



   

   
Gabe Black

   
Posted  (Nov. 9, 2010, 5:06 a.m.)


  


  


  

   



  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    Sorry for the review spam (assuming this is actually visible). reviewboard stopped responding and then started having internal errors for everything I did. Apparently it went through anyway.
    
    
    
   
    2. 

   


  





  

 


  

 

















 



 

  


 


  



   

   
Steve Reinhardt

   
Posted  (Nov. 10, 2010, 12:07 a.m.)


  


  


  

   
So it looks like whether you call setMiscReg() or setMiscRegNoEffect() then you buffer the update and call setMiscReg() later... i.e., even if you called setMiscRegNoEffect() originally you end up calling setMiscReg() at commit.

Are there ever cases where we call setMiscRegNoEffect() from an instruction?  I thought it was strictly for things like system initialization and restoring from checkpoints anyway... maybe we don't need it in the ExecContext interface.

Also, I see that these predate your change, but why do we need both the setMiscReg() calls and the setMiscRegOperand() calls?  Is one of them deprecated?  Can we take this opportunity to update the comments to distinguish them?  We shouldn't have two different functions for which the descriptive comment is completely identical IMO.




  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    You're right... The only place i can find an xc using setMiscRegNoEffect() is locked_mem.hh for alpha and what is the exact copy of it for MIPS. It seems like that could be setMiscReg(). The implementation for LOCKED_ADDR is identical (there are no effects). Perhaps we should remove it from the xc. My guess was that it was used with the intention of making it cheaper by not having the cpu squash when it happens, but in reality that didn't work out. 

The difference between the operand and no operand version is that the operand version gets the dest misc register from the instruction while the non-operand one expects it passed in as a parameter. One should be implemented in terms of the other, but both seem to be used.
    
    
    
   
    2. 


   
  2. 
    
    
     
    
      
     
    
     
    Yeah, I think generally setMiscRegOperand is going to be called from generated code and setMiscReg is going to be used by handwritten code where the index is provided directly.
    
    
    
   
    3. 


   
  3. 
    
    
     
    
      
     
    
     
    Ok... I removed the NoEffect versions from the exec context and changed the *Operand versions to just call their non-operand versions which cleans up the code some. Alpha has no problem with these changes, but they do uncover a bug with SPARC that somehow worked, but it's amazing that it did. The ASI register changes the type of instruction that is executing (e.g. one ASI is a type of prefetch vs. load vs. load with a different addressing mode etc). The types are decoded from the ASI bits in the ExtMachInst which are inserted by the pre-decoder. The trouble is that these bits are inserted in "fetch" which is before rename so the IsSerializing flags don't prevent it from happening. Thus, while the wrasi instruction can be marked as Serializing fetch already has read the old value and created the instructions based on the old ASI. When wrasi commits the old instructions flow through the rest of the pipeline and that isn't great. It's amazing that all the code sequences that Gabe used to run on the sparc/o3 model never ran into this issue. With the current code if you executed a alternating wrasi and load w/asi instructions you would get some very odd results. Anyway, the question becomes how do we fix this??? My current solution is to add an instruction flag that after commit flushes the CPU. Although, I don't know if this is enough and what other surprises I'll run into.

Thoughts?

Ali


    
    
    
   
    4.