Implements shifted offset register addressing mode for arm.

runtime/vm/assembler_arm.h

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #ifndef VM_ASSEMBLER_ARM_H_
 #define VM_ASSEMBLER_ARM_H_
 
 #ifndef VM_ASSEMBLER_H_
 #error Do not include assembler_arm.h directly; use assembler.h instead.
 #endif
@@ skipping 91 matching lines @@
 
   // Data-processing operands - Register.
   explicit ShifterOperand(Register rm) {
     type_ = 0;
     encoding_ = static_cast<uint32_t>(rm);
   }
 
   // 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 |
                 static_cast<uint32_t>(rm);
   }
 
   // Data-processing operands - Logical shift/rotate by register.
   ShifterOperand(Register rm, Shift shift, Register rs) {
     type_ = 0;
     encoding_ = static_cast<uint32_t>(rs) << kShiftRegisterShift |
@@ skipping 30 matching lines @@
 
   uint32_t encoding() const {
     ASSERT(is_valid());
     return encoding_;
   }
 
   uint32_t type_;  // Encodes the type field (bits 27-25) in the instruction.
   uint32_t encoding_;
 
   friend class Assembler;
+  friend class Address;
 };
 
 
 enum LoadOperandType {
   kLoadSignedByte,
   kLoadUnsignedByte,
   kLoadSignedHalfword,
   kLoadUnsignedHalfword,
   kLoadWord,
   kLoadWordPair,
@@ skipping 21 matching lines @@
   IB           = (8|4|0) << 21,  // increment before
   DA_W         = (0|0|1) << 21,  // decrement after with writeback to base
   IA_W         = (0|4|1) << 21,  // increment after with writeback to base
   DB_W         = (8|0|1) << 21,  // decrement before with writeback to base
   IB_W         = (8|4|1) << 21   // increment before with writeback to base
 };
 
 
 class Address : public ValueObject {
  public:
+  enum OffsetKind {
+    Immediate,
+    ShiftedRegister,
+  };
+
   // Memory operand addressing mode
   enum Mode {
     // bit encoding P U W
     Offset       = (8|4|0) << 21,  // offset (w/o writeback to base)
     PreIndex     = (8|4|1) << 21,  // pre-indexed addressing with writeback
     PostIndex    = (0|4|0) << 21,  // post-indexed addressing with writeback
     NegOffset    = (8|0|0) << 21,  // negative offset (w/o writeback to base)
     NegPreIndex  = (8|0|1) << 21,  // negative pre-indexed with writeback
     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 {
       encoding_ = am | offset;
     }
     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);
 
  private:
   uint32_t encoding() const { return encoding_; }
 
   // Encoding for addressing mode 3.
   uint32_t encoding3() const;
 
   // 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;
 };
 
 
 class FieldAddress : public Address {
  public:
   FieldAddress(Register base, int32_t disp)
       : Address(base, disp - kHeapObjectTag) { }
 
   FieldAddress(const FieldAddress& other) : Address(other) { }
@@ skipping 383 matching lines @@
     return *reg1 - *reg2;
   }
 
   DISALLOW_ALLOCATION();
   DISALLOW_COPY_AND_ASSIGN(Assembler);
 };
 
 }  // namespace dart
 
 #endif  // VM_ASSEMBLER_ARM_H_