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

- Fix the use of OS::ActivationFrameAlignment() when running with
  the simulator.
Review URL: http://codereview.chromium.org/155109

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@2401 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/arm/disasm-arm.cc b/src/arm/disasm-arm.cc
index c55a958..d193ab9 100644 (file)
--- a/src/arm/disasm-arm.cc
+++ b/src/arm/disasm-arm.cc
@@ -506,17 +506,25 @@ void Decoder::DecodeType01(Instr* instr) {
         // multiply instructions
         if (instr->Bit(23) == 0) {
           if (instr->Bit(21) == 0) {
-            // Mul calls it Rd.  Everyone else calls it Rn.
+            // 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");
           } else {
-            // In the manual the order is rd, rm, rs, rn.  But mla swaps the
-            // positions of rn and rd in the encoding.
+            // The MLA instruction description (A 4.1.28) refers to the order
+            // of registers as "Rd, Rm, Rs, Rn". But confusingly it uses the
+            // Rn field to encode the Rd register and the Rd field to encode
+            // the Rn register.
             Format(instr, "mla'cond's 'rn, 'rm, 'rs, 'rd");
           }
         } else {
-          // In the manual the order is RdHi, RdLo, Rm, Rs.
-          // RdHi is what other instructions call Rn and RdLo is Rd.
-          Format(instr, "'um'al'cond's 'rn, 'rd, 'rm, 'rs");
+          // 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 field
+          // RdHi == Rn field
+          // The order of registers is: <RdLo>, <RdHi>, <Rm>, <Rs>
+          Format(instr, "'um'al'cond's 'rd, 'rn, 'rm, 'rs");
         }
       } else {
         Unknown(instr);  // not used by V8

diff --git a/src/arm/macro-assembler-arm.cc b/src/arm/macro-assembler-arm.cc
index 3d6b8cb..47e2749 100644 (file)
--- a/src/arm/macro-assembler-arm.cc
+++ b/src/arm/macro-assembler-arm.cc
@@ -290,11 +290,24 @@ void MacroAssembler::EnterExitFrame(StackFrame::Type type) {
   // Align the stack at this point.  After this point we have 5 pushes,
   // so in fact we have to unalign here!  See also the assert on the
   // alignment immediately below.
-  if (OS::ActivationFrameAlignment() != kPointerSize) {
+#if defined(V8_HOST_ARCH_ARM)
+  // Running on the real platform. Use the alignment as mandated by the local
+  // environment.
+  // Note: This will break if we ever start generating snapshots on one ARM
+  // platform for another ARM platform with a different alignment.
+  int activation_frame_alignment = OS::ActivationFrameAlignment();
+#else  // defined(V8_HOST_ARCH_ARM)
+  // If we are using the simulator then we should always align to the expected
+  // alignment. As the simulator is used to generate snapshots we do not know
+  // if the target platform will need alignment, so we will always align at
+  // this point here.
+  int activation_frame_alignment = 2 * kPointerSize;
+#endif  // defined(V8_HOST_ARCH_ARM)
+  if (activation_frame_alignment != kPointerSize) {
     // This code needs to be made more general if this assert doesn't hold.
-    ASSERT(OS::ActivationFrameAlignment() == 2 * kPointerSize);
+    ASSERT(activation_frame_alignment == 2 * kPointerSize);
     mov(r7, Operand(Smi::FromInt(0)));
-    tst(sp, Operand(OS::ActivationFrameAlignment() - 1));
+    tst(sp, Operand(activation_frame_alignment - 1));
     push(r7, eq);  // Conditional push instruction.
   }
 

diff --git a/src/arm/simulator-arm.cc b/src/arm/simulator-arm.cc
index 1b42919..e5500aa 100644 (file)
--- a/src/arm/simulator-arm.cc
+++ b/src/arm/simulator-arm.cc
@@ -1080,25 +1080,44 @@ void Simulator::DecodeType01(Instr* instr) {
     // 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.
+            // The MLA instruction description (A 4.1.28) refers to the order
+            // of registers as "Rd, Rm, Rs, Rn". But confusingly it uses the
+            // Rn field to encode the Rd register and the Rd field to encode
+            // the Rn register.
+            Format(instr, "mla'cond's 'rn, 'rm, 'rs, 'rd");
           }
         } 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;
@@ -1117,7 +1136,7 @@ void Simulator::DecodeType01(Instr* instr) {
             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();
           }