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.
Jack Whitham wrote: > Hi, > > I've found a few bugs in the M5 ARM implementation. > Here are some fixes for the latest (06/23/09) development version. > > macromem-fix - added unimplemented load/store multiple instructions. > Looks good. > lr-fix - the link register is incorrectly overwritten by > non-executed branch and link operations, e.g. "blne" > when "Z = 1". > 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. > longmul-loads - long (64bit) multiplication instructions should > ensure that only 32 bits of the input registers are > actually used; added some unimplemented load instructions > 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 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? Gabe _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
