- Update comments and fix a small glitch in the ARM disassembler....

src/arm/simulator-arm.cc

 // Copyright 2009 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 1062 matching lines @@
 // Handle execution based on instruction types.
 
 // Instruction types 0 and 1 are both rolled into one function because they
 // only differ in the handling of the shifter_operand.
 void Simulator::DecodeType01(Instr* instr) {
   int type = instr->TypeField();
   if ((type == 0) && instr->IsSpecialType0()) {
     // multiply instruction or extra loads and stores
     if (instr->Bits(7, 4) == 9) {
       if (instr->Bit(24) == 0) {
-        // Multiply instructions have Rd in a funny place.
-        int rd = instr->RnField();
+        // Raw field decoding here. Multiply instructions have their Rd in
+        // funny places.
+        int rn = instr->RnField();
         int rm = instr->RmField();
         int rs = instr->RsField();
         int32_t rs_val = get_register(rs);
         int32_t rm_val = get_register(rm);
         if (instr->Bit(23) == 0) {
           if (instr->Bit(21) == 0) {
+            // The MUL instruction description (A 4.1.33) refers to Rd as being
+            // the destination for the operation, but it confusingly uses the
+            // Rn field to encode it.
             // Format(instr, "mul'cond's 'rn, 'rm, 'rs");
+            int rd = rn;  // Remap the rn field to the Rd register.
             int32_t alu_out = rm_val * rs_val;
             set_register(rd, alu_out);
             if (instr->HasS()) {
               SetNZFlags(alu_out);
             }
           } else {
-            UNIMPLEMENTED();  // mla is not used by V8.
+            // mla is not currently being used by V8.
+            Format(instr, "mla'cond's 'rd, 'rm, 'rs, 'rn");

[Erik Corry, 2009/07/07 08:14:16]

Should be 'rn 'rm 'rs 'rd for similar reasons to the above.

[iposva, 2009/07/08 21:33:26]

Fixed by copying the whole block out of the disassembler (including the updated
comment.)

           }
         } else {
-          // Format(instr, "'um'al'cond's 'rn, 'rd, 'rs, 'rm");
+          // The signed/long multiply instructions use the terms RdHi and RdLo
+          // when referring to the target registers. They are mapped to the Rn
+          // and Rd fields as follows:
+          // RdLo == Rd
+          // RdHi == Rn (This is confusingly stored in variable rd here
+          //             because the mul instruction from above uses the
+          //             Rn field to encode the Rd register. Good luck figuring
+          //             this out without reading the ARM instruction manual
+          //             at a very detailed level.)
+          // Format(instr, "'um'al'cond's 'rd, 'rn, 'rs, 'rm");
+          int rd_hi = rn;  // Remap the rn field to the RdHi register.
           int rd_lo = instr->RdField();
           int32_t hi_res = 0;
           int32_t lo_res = 0;
           if (instr->Bit(22) == 1) {
             int64_t left_op  = static_cast<int32_t>(rm_val);
             int64_t right_op = static_cast<int32_t>(rs_val);
             uint64_t result = left_op * right_op;
             hi_res = static_cast<int32_t>(result >> 32);
             lo_res = static_cast<int32_t>(result & 0xffffffff);
           } else {
             // unsigned multiply
             uint64_t left_op  = static_cast<uint32_t>(rm_val);
             uint64_t right_op = static_cast<uint32_t>(rs_val);
             uint64_t result = left_op * right_op;
             hi_res = static_cast<int32_t>(result >> 32);
             lo_res = static_cast<int32_t>(result & 0xffffffff);
           }
           set_register(rd_lo, lo_res);
-          set_register(rd, hi_res);
+          set_register(rd_hi, hi_res);
           if (instr->HasS()) {
             UNIMPLEMENTED();
           }
         }
       } else {
         UNIMPLEMENTED();  // not used by V8
       }
     } else {
       // extra load/store instructions
       int rd = instr->RdField();
@@ skipping 663 matching lines @@
   set_register(r10, r10_val);
   set_register(r11, r11_val);
 
   int result = get_register(r0);
   return reinterpret_cast<Object*>(result);
 }
 
 } }  // namespace assembler::arm
 
 #endif  // !defined(__arm__)