On Wed, Jun 24, 2009 at 12:47:59AM -0700, Gabe Black wrote: > I've looked at these more carefully, and comments are inline. I intend > to commit these (with the adjustments explained below) using your name > as the committer. Although I know that can be done since it's been done > for other people, I don't know how to do it offhand. It might take a day > or two to get that sorted out.
Thanks. > This was almost right. The LR needs to be "updated" to its original > value so that the writeback doesn't set it to uninitialized junk, but it > should do so only for linked branches. If you always put that in there > it's functionally correct for linked branches, but it artificially makes > LR a source and destination operand for regular branches. Good point. > Rm and Rs start life as 32 bit unsigned integers (see > src/arch/arm/isa/operands.isa), so there's no reason to cast them as > such unless I'm missing something. The place where it matters is for the > signed multiply where you need to make sure you're sign extending things > properly. That can be accomplished with the operand types or casting. The fix was for signed multiply; I must have wrongly applied it to unsigned multiply as well. My mistake. > The new loads look fine, although I didn't check where they fall in the > decoder. For a number of loads it looks like you reversed the order of > the address base register update and the write to the destination > register. I have no idea if that's correct or not (I was wondering about > that the other day, actually), but if it is, it might be better to apply > one ordering rule consistently across all such loads. Do you know if > that would be correct? I notice that you didn't mention that in your > description of your patch. Is that something you tried for debugging and > forgot to take back out? Please take this out, it is there by accident. I added this in the hope of forcing the trace output to report the loaded data (D=). The trace output only reports the results of a single ALU operation, and this is Rn = Rn + disp instead of Rd = Mem. The load operations that update two registers use the "post-indexed" addressing mode. You do not need to worry about the case where both registers are the same (e.g. "LDR r0, [r0], #4"), because ARM does not define what happens in this case (I checked using the official ARM assembler; that instruction is invalid). Similarly, ldm* instructions have undefined behaviour if the base register is one of the registers being loaded. -- Jack Whitham [email protected] _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
