Review Board 2.0.15


ARM: make predicated-false instruction to move data from a old register.

Review Request #198 - Created Aug. 13, 2010 and discarded

Information
Ali Saidi
gem5
Reviewers
Default
Min
ARM: make predicated-false instruction to move data from a old register.
Thus a predicted flase instruction turns into a move from old register
to new register. This doesn't matter on the simple CPU, but is required
because of the physical registers on the o3 cpu.

   
Posted (Aug. 13, 2010, 5:45 p.m.)
The ISA parser is foundational for a LOT of code and shouldn't be changed unless it's absolutely necessary or the end goal itself. There are a lot of changes to it here, and without going through them it's likely most of them could/should be in the ISA description itself. Could you please explain why that can't be done and this has to go into the parser?
  1. I agree, I think this issue should be dealt with in the O3 model and not in the ISA parser.  Can't the O3 model just look at the dest reg(s) in the StaticInst and deal with it from there?
  2. I needed to spit out a code that reads from a register, and writes to it again. The thing is arch reg indices are renamed (reg renaming and shadow reg file), so many structures are needed to be looked up to find a previous/current physical register for the given arch reg name. These indirections are handled in ISA-specific code and encapsulated, and the operand section of the ISA description specifies which one to use. 
    
    Simplest way was to reuse makeRead() and makeWrite(), so when we need to move we simply 'read' from the destination reg (hence the previous phy reg) and 'write' to the destination reg (newly renamed phy reg). This is 'resetcode' and all changes in the isa parser is an addition to generate this 'resetcode'.
    
    I guess the O3 model can do the job, but it would need to go though ISA to figure out the mapping, and most of the code in the O3 would remain ARM-specific. I think it would be a better isolation to contain the code that 'executes' an instruction remains in the execute() function of the StaticInst.
    
    Ali, doesn't my patch files have a more detailed description than what's posted in the header? I thought I tried to document the logic behind any major code change.