Implements shifted offset register addressing mode for arm.

runtime/vm/assembler_arm.h

Index: runtime/vm/assembler_arm.h
===================================================================
--- runtime/vm/assembler_arm.h	(revision 19091)
+++ runtime/vm/assembler_arm.h	(working copy)
@@ -109,6 +109,7 @@
   // Data-processing operands - Logical shift/rotate by immediate.
   ShifterOperand(Register rm, Shift shift, uint32_t shift_imm) {
     ASSERT(shift_imm < (1 << kShiftImmBits));
+    ASSERT(!(shift == LSL && shift_imm == 0));

[regis, 2013/02/26 23:39:49]

You will notice that we like redundant parenthesis to make precedence extra
clear:
 ASSERT(!((shift == LSL) && (shift_imm == 0)));

It takes some getting use to...
Why are you forbidding LSL #0? Is it a no-op?
Please, add a comment.

     type_ = 0;
     encoding_ = shift_imm << kShiftImmShift |
                 static_cast<uint32_t>(shift) << kShiftShift |
@@ -159,6 +160,7 @@
   uint32_t encoding_;
 
   friend class Assembler;
+  friend class Address;
 };
 
 
@@ -200,6 +202,11 @@
 
 class Address : public ValueObject {
  public:
+  enum OffsetKind {
+    Immediate,
+    ShiftedRegister,
+  };
+
   // Memory operand addressing mode
   enum Mode {
     // bit encoding P U W
@@ -211,15 +218,19 @@
     NegPostIndex = (0|0|0) << 21   // negative post-indexed with writeback
   };
 
-  Address(const Address& other) : ValueObject(), encoding_(other.encoding_) { }
+  Address(const Address& other) :
+    ValueObject(), encoding_(other.encoding_), kind_(other.kind_)
+  { }

[regis, 2013/02/26 23:39:49]

The pair of braces cannot stand by themselves on a continuation line. Your
options are:
XXXXXX { }
XXXXXX {
}
XXXXXX
    : YYYYYY { }
XXXXXX
    : YYYYYY {
}
As shown above, the indentation for a continuation line is 4 spaces.
We also like to see the : at the beginning of the continuation line rather than
at the end of the previous line.

  Address(const Address& other)
      : ValueObject(), encoding_(other.encoding_), kind_(other.kind_) { }

 
   Address& operator=(const Address& other) {
     encoding_ = other.encoding_;
+    kind_     = other.kind_;

[regis, 2013/02/26 23:39:49]

We generally do not align assignments, unless it really helps with reading (such
as in the enum above). Not necessary here.
 

     return *this;
   }
 
   explicit Address(Register rn, int32_t offset = 0, Mode am = Offset) {
     ASSERT(Utils::IsAbsoluteUint(12, offset));
+    kind_ = Immediate;
     if (offset < 0) {
       encoding_ = (am ^ (1 << kUShift)) | -offset;  // Flip U to adjust sign.
     } else {
@@ -228,6 +239,14 @@
     encoding_ |= static_cast<uint32_t>(rn) << kRnShift;
   }
 
+  explicit Address(Register rn, Register rm, Shift shift_t = LSL,
+                   uint32_t shift_n = 0, Mode am = Offset) {

[regis, 2013/02/26 23:39:49]

Instead of shift_t and shift_n, I would use the same names as in ShifterOperand:
Shift shift, uint32_t shift_imm

+    ShifterOperand so(rm, shift_t, shift_n);
+
+    kind_     = ShiftedRegister;

[regis, 2013/02/26 23:39:49]

No alignment.

+    encoding_ = so.encoding() | am | (static_cast<uint32_t>(rn) << kRnShift);
+  }
+
   static bool CanHoldLoadOffset(LoadOperandType type, int offset);
   static bool CanHoldStoreOffset(StoreOperandType type, int offset);
 
@@ -240,8 +259,12 @@
   // Encoding for vfp load/store addressing.
   uint32_t vencoding() const;
 
+  enum OffsetKind kind() const { return kind_; }

[regis, 2013/02/26 23:39:49]

We generally do not write "enum".

+
   uint32_t encoding_;
 
+  enum OffsetKind kind_;

[regis, 2013/02/26 23:39:49]

ditto

+
   friend class Assembler;
 };