Review Board 2.0.15


The ARM decoder should not panic when decoding undefined holes in the

Review Request #346 - Created Dec. 6, 2010 and submitted

Information
Ali Saidi
gem5
Reviewers
Default
ali, gblack, nate, stever 
The ARM decoder should not panic when decoding undefined holes in the
Architecture, as this can abort simulations when the fetch unit runs
ahead and speculatively decodes instructions that are off the execution
path or that are generated dynamically.

   

  


 


 

  

  









  
 


  

 
















 



 

  


 


  



   

   
Gabe Black

   
Posted  (Dec. 8, 2010, 3:56 p.m.)


  


  


  

   
Malformed commit message again.

It would be nice not to lose the information in the commit message. Ideally it would pop out if the fault was invoked in SE mode, but more trivially it could be moved into a comment above the new version.



  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    Well, it didn't come out in review board, but the description is:
The ARM decoder should not panic when decoding undefined holes<newline>
rest of message. You get the the best of both worlds. This doesn't work so well in the case of SE mode either, because te O3 CPU can still speculate and cause the same issues.

Fixed rest of the issued mentioned. 
    
    
    
   
    2. 


   
  2. 
    
    
     
    
      
     
    
     
    Woops, that should have said "not lose the information in the panics", ie that the size was invalid, something wasn't recognized, etc. It would be great if there was still a message when the fault happened and that ends the simulation (basically SE, I think), but moving the panic message into a comment at least preserves the intent if someone finds where a fault came from.
    
    
    
   
    3. 

   


  




   







    

     
     

      

       
        

         
          src/arch/arm/isa/formats/fp.isa
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
The style of this comment looks unusual, although I don't think I can say it's wrong. I think the line with /* should at least be otherwise blank, but really // style comments would work better. Gramatically this is actually two sentences. I think the comment in general is probably not necessary since the name immValid makes it pretty clear what's going on.




  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    cleaned up with //
    
    
    
   
    2. 

   


  


    


    

     
     

      

       
        

         
          src/arch/arm/isa/formats/fp.isa
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
Misplaced brace




  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    done
    
    
    
   
    2. 

   


  


    


    

     
     

      

       
        

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

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
delete, don't comment out




  

   

   
    
    
   


  


    


    

     
     

      

       
        

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

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
delete, don't comment out




  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    all fixed
    
    
    
   
    2.