Make sure the constant pool size is as promised.

src/arm/assembler-arm.cc

Index: src/arm/assembler-arm.cc
diff --git a/src/arm/assembler-arm.cc b/src/arm/assembler-arm.cc
index b90ff1e05b6a383cbf40276af1e762b20ebd2635..74c682a47f60799b4fecc7cbf56a7f99bddb0dce 100644
--- a/src/arm/assembler-arm.cc
+++ b/src/arm/assembler-arm.cc
@@ -3734,18 +3734,19 @@ void Assembler::CheckConstPool(bool force_emit, bool require_jump) {
   // the gap to the relocation information).
   int jump_instr = require_jump ? kInstrSize : 0;
   int size_up_to_marker = jump_instr + kInstrSize;
-  int size_after_marker = num_pending_32_bit_constants_ * kPointerSize;
+  int estimated_size_after_marker =
+      num_pending_32_bit_constants_ * kPointerSize;
   bool has_fp_values = (num_pending_64_bit_constants_ > 0);
   bool require_64_bit_align = false;
   if (has_fp_values) {
-    require_64_bit_align = (((uintptr_t)pc_ + size_up_to_marker) & 0x7);
+    require_64_bit_align = IsAligned(
+        reinterpret_cast<intptr_t>(pc_ + size_up_to_marker), kDoubleAlignment);
     if (require_64_bit_align) {
-      size_after_marker += kInstrSize;
+      estimated_size_after_marker += kInstrSize;
     }
-    size_after_marker += num_pending_64_bit_constants_ * kDoubleSize;
+    estimated_size_after_marker += num_pending_64_bit_constants_ * kDoubleSize;
   }
-
-  int size = size_up_to_marker + size_after_marker;
+  int estimated_size = size_up_to_marker + estimated_size_after_marker;
 
   // We emit a constant pool when:
   //  * requested to do so by parameter force_emit (e.g. after each function).
@@ -3759,7 +3760,7 @@ void Assembler::CheckConstPool(bool force_emit, bool require_jump) {
     DCHECK((first_const_pool_32_use_ >= 0) || (first_const_pool_64_use_ >= 0));
     bool need_emit = false;
     if (has_fp_values) {
-      int dist64 = pc_offset() + size -
+      int dist64 = pc_offset() + estimated_size -
                    num_pending_32_bit_constants_ * kPointerSize -
                    first_const_pool_64_use_;
       if ((dist64 >= kMaxDistToFPPool - kCheckPoolInterval) ||
@@ -3767,8 +3768,7 @@ void Assembler::CheckConstPool(bool force_emit, bool require_jump) {
         need_emit = true;
       }
     }
-    int dist32 =
-      pc_offset() + size - first_const_pool_32_use_;
+    int dist32 = pc_offset() + estimated_size - first_const_pool_32_use_;
     if ((dist32 >= kMaxDistToIntPool - kCheckPoolInterval) ||
         (!require_jump && (dist32 >= kMaxDistToIntPool / 2))) {
       need_emit = true;
@@ -3776,6 +3776,37 @@ void Assembler::CheckConstPool(bool force_emit, bool require_jump) {
     if (!need_emit) return;
   }
 
+  // Deduplicate constants.
+  int size_after_marker = estimated_size_after_marker;
+  for (int i = 0; i < num_pending_64_bit_constants_; i++) {
+    ConstantPoolEntry& entry = pending_64_bit_constants_[i];
+    DCHECK(!entry.is_merged());
+    for (int j = 0; j < i; j++) {
+      if (entry.value64() == pending_64_bit_constants_[j].value64()) {
+        DCHECK(!pending_64_bit_constants_[j].is_merged());
+        entry.set_merged_index(j);
+        size_after_marker -= kDoubleSize;
+        break;
+      }
+    }
+  }
+
+  for (int i = 0; i < num_pending_32_bit_constants_; i++) {
+    ConstantPoolEntry& entry = pending_32_bit_constants_[i];
+    DCHECK(!entry.is_merged());
+    if (!entry.sharing_ok()) continue;
+    for (int j = 0; j < i; j++) {
+      if (entry.value() == pending_32_bit_constants_[j].value()) {
+        DCHECK(!pending_32_bit_constants_[j].is_merged());
+        entry.set_merged_index(j);

[rmcilroy, 2015/07/03 08:58:04]

I think this the wrong way round, you should be setting
...constants[j].set_merged_index[i]. Otherwise the delta for entry will get
larger (since the j is larger than i) which means it might be out of ldr range.
I'm not sure if this is actually a problem though (I can't rememeber how the
assembler checks when it should emit the pool). Also if there was another
duplicate later then wouldnt entry j will get merged with that other dup while i
is still duped with j?

[Yang, 2015/07/03 09:07:33]

I haven't changed the merge loop at all. j is never larger than i as indicated
by the for-loop condition. For every entry i we look at prior entries (0 .. i-1)
for duplicates. If we found an entry j, we will use the found prior entry, by
setting the merge_index of i to j.

[rmcilroy, 2015/07/03 09:45:14]

You're right, oops. I didn't look at the for loop header properly and assumed
that you were doing j=i, j++ :). This looks fine then, thanks.

+        size_after_marker -= kPointerSize;
+        break;
+      }
+    }
+  }
+
+  int size = size_up_to_marker + size_after_marker;
+
   int needed_space = size + kGap;
   while (buffer_space() <= needed_space) GrowBuffer();
 
@@ -3785,6 +3816,9 @@ void Assembler::CheckConstPool(bool force_emit, bool require_jump) {
     RecordComment("[ Constant Pool");
     RecordConstPool(size);
 
+    Label size_check;
+    bind(&size_check);
+
     // Emit jump over constant pool if necessary.
     Label after_pool;
     if (require_jump) {
@@ -3805,8 +3839,6 @@ void Assembler::CheckConstPool(bool force_emit, bool require_jump) {
     for (int i = 0; i < num_pending_64_bit_constants_; i++) {
       ConstantPoolEntry& entry = pending_64_bit_constants_[i];
 
-      DCHECK(!((uintptr_t)pc_ & 0x7));  // Check 64-bit alignment.
-
       Instr instr = instr_at(entry.position());
       // Instruction to patch must be 'vldr rd, [pc, #offset]' with offset == 0.
       DCHECK((IsVldrDPcImmediateOffset(instr) &&
@@ -3815,24 +3847,19 @@ void Assembler::CheckConstPool(bool force_emit, bool require_jump) {
       int delta = pc_offset() - entry.position() - kPcLoadDelta;
       DCHECK(is_uint10(delta));
 
-      bool found = false;
-      uint64_t value = entry.value64();
-      for (int j = 0; j < i; j++) {
-        ConstantPoolEntry& entry2 = pending_64_bit_constants_[j];
-        if (value == entry2.value64()) {
-          found = true;
-          Instr instr2 = instr_at(entry2.position());
-          DCHECK(IsVldrDPcImmediateOffset(instr2));
-          delta = GetVldrDRegisterImmediateOffset(instr2);
-          delta += entry2.position() - entry.position();
-          break;
-        }
+      if (entry.is_merged()) {
+        ConstantPoolEntry& merged =
+            pending_64_bit_constants_[entry.merged_index()];
+        DCHECK(entry.value64() == merged.value64());
+        Instr merged_instr = instr_at(merged.position());
+        DCHECK(IsVldrDPcImmediateOffset(merged_instr));
+        delta = GetVldrDRegisterImmediateOffset(merged_instr);
+        delta += merged.position() - entry.position();
       }
-
       instr_at_put(entry.position(),
                    SetVldrDRegisterImmediateOffset(instr, delta));
-
-      if (!found) {
+      if (!entry.is_merged()) {
+        DCHECK(IsAligned(reinterpret_cast<intptr_t>(pc_), kDoubleAlignment));
         dq(entry.value64());
       }
     }
@@ -3844,41 +3871,31 @@ void Assembler::CheckConstPool(bool force_emit, bool require_jump) {
 
       // 64-bit loads shouldn't get here.
       DCHECK(!IsVldrDPcImmediateOffset(instr));
+      DCHECK(!IsMovW(instr));
+      DCHECK(IsLdrPcImmediateOffset(instr) &&
+             GetLdrRegisterImmediateOffset(instr) == 0);
 
-      if (IsLdrPcImmediateOffset(instr) &&
-          GetLdrRegisterImmediateOffset(instr) == 0) {
-        int delta = pc_offset() - entry.position() - kPcLoadDelta;
-        DCHECK(is_uint12(delta));
-        // 0 is the smallest delta:
-        //   ldr rd, [pc, #0]
-        //   constant pool marker
-        //   data
-
-        bool found = false;
-        if (entry.sharing_ok()) {
-          for (int j = 0; j < i; j++) {
-            ConstantPoolEntry& entry2 = pending_32_bit_constants_[j];
-
-            if (entry2.value() == entry.value()) {
-              Instr instr2 = instr_at(entry2.position());
-              if (IsLdrPcImmediateOffset(instr2)) {
-                delta = GetLdrRegisterImmediateOffset(instr2);
-                delta += entry2.position() - entry.position();
-                found = true;
-                break;
-              }
-            }
-          }
-        }
-
-        instr_at_put(entry.position(),
-                     SetLdrRegisterImmediateOffset(instr, delta));
-
-        if (!found) {
-          emit(entry.value());
-        }
-      } else {
-        DCHECK(IsMovW(instr));
+      int delta = pc_offset() - entry.position() - kPcLoadDelta;
+      DCHECK(is_uint12(delta));
+      // 0 is the smallest delta:
+      //   ldr rd, [pc, #0]
+      //   constant pool marker
+      //   data
+
+      if (entry.is_merged()) {
+        DCHECK(entry.sharing_ok());
+        ConstantPoolEntry& merged =
+            pending_32_bit_constants_[entry.merged_index()];
+        DCHECK(entry.value() == merged.value());
+        Instr merged_instr = instr_at(merged.position());
+        DCHECK(IsLdrPcImmediateOffset(merged_instr));
+        delta = GetLdrRegisterImmediateOffset(merged_instr);
+        delta += merged.position() - entry.position();
+      }
+      instr_at_put(entry.position(),
+                   SetLdrRegisterImmediateOffset(instr, delta));
+      if (!entry.is_merged()) {
+        emit(entry.value());
       }
     }
 
@@ -3889,6 +3906,8 @@ void Assembler::CheckConstPool(bool force_emit, bool require_jump) {
 
     RecordComment("]");
 
+    DCHECK_EQ(size, SizeOfCodeGeneratedSince(&size_check));
+
     if (after_pool.is_linked()) {
       bind(&after_pool);
     }