ARM: Clean up literal pool generation....

src/arm/assembler-arm.cc

Index: src/arm/assembler-arm.cc
===================================================================
--- src/arm/assembler-arm.cc	(revision 8269)
+++ src/arm/assembler-arm.cc	(working copy)
@@ -324,7 +324,7 @@
   next_buffer_check_ = 0;
   const_pool_blocked_nesting_ = 0;
   no_const_pool_before_ = 0;
-  last_const_pool_end_ = 0;
+  first_const_pool_use_ = 0;

[Søren Thygesen Gjesse, 2011/06/14 07:50:56]

Maybe initialize to -1, as the value is only valid when num_prinfo_ is > 0.

[Rodolph Perfetta, 2011/06/14 22:52:20]

Done.

   last_bound_pos_ = 0;
   ast_id_for_reloc_info_ = kNoASTId;
 }
@@ -873,7 +873,7 @@
   emit(instr | rn.code()*B16 | rd.code()*B12);
   if (rn.is(pc) || x.rm_.is(pc)) {
     // Block constant pool emission for one instruction after reading pc.
-    BlockConstPoolBefore(pc_offset() + kInstrSize);
+    BlockConstPoolFor(1);
   }
 }
 
@@ -997,7 +997,7 @@
 
   // Block the emission of the constant pool, since the branch instruction must
   // be emitted at the pc offset recorded by the label.
-  BlockConstPoolBefore(pc_offset() + kInstrSize);
+  BlockConstPoolFor(1);
   return target_pos - (pc_offset() + kPcLoadDelta);
 }
 
@@ -1493,15 +1493,17 @@
 void Assembler::stop(const char* msg, Condition cond, int32_t code) {
 #ifndef __arm__
   ASSERT(code >= kDefaultStopCode);
-  // The Simulator will handle the stop instruction and get the message address.
-  // It expects to find the address just after the svc instruction.
-  BlockConstPoolFor(2);
-  if (code >= 0) {
-    svc(kStopCode + code, cond);
-  } else {
-    svc(kStopCode + kMaxStopCode, cond);
+  {
+    // The Simulator will handle the stop instruction and get the message
+    // address. It expects to find the address just after the svc instruction.
+    BlockConstPoolScope block_const_pool(this);
+    if (code >= 0) {
+      svc(kStopCode + code, cond);
+    } else {
+      svc(kStopCode + kMaxStopCode, cond);
+    }
+    emit(reinterpret_cast<Instr>(msg));
   }
-  emit(reinterpret_cast<Instr>(msg));
 #else  // def __arm__
 #ifdef CAN_USE_ARMV5_INSTRUCTIONS
   if (cond != al) {
@@ -2406,11 +2408,6 @@
 }
 
 
-void Assembler::BlockConstPoolFor(int instructions) {
-  BlockConstPoolBefore(pc_offset() + instructions * kInstrSize);
-}
-
-
 // Debugging.
 void Assembler::RecordJSReturn() {
   positions_recorder()->WriteRecordedPositions();
@@ -2518,10 +2515,13 @@
     // These modes do not need an entry in the constant pool.
   } else {
     ASSERT(num_prinfo_ < kMaxNumPRInfo);
+    if (num_prinfo_ == 0) {
+      first_const_pool_use_ = pc_offset();
+    }
     prinfo_[num_prinfo_++] = rinfo;
     // Make sure the constant pool is not emitted in place of the next
     // instruction for which we just recorded relocation info.
-    BlockConstPoolBefore(pc_offset() + kInstrSize);
+    BlockConstPoolFor(1);
   }
   if (rinfo.rmode() != RelocInfo::NONE) {
     // Don't record external references unless the heap will be serialized.
@@ -2548,111 +2548,109 @@
 }
 
 
-void Assembler::CheckConstPool(bool force_emit, bool require_jump) {
-  // Calculate the offset of the next check. It will be overwritten
-  // when a const pool is generated or when const pools are being
-  // blocked for a specific range.
-  next_buffer_check_ = pc_offset() + kCheckConstInterval;
+void Assembler::BlockConstPoolFor(int instructions) {
+  int pc_limit = pc_offset() + instructions * kInstrSize;
+  if (no_const_pool_before_ < pc_limit) {
+    // If there are some pending entries, the constant pool cannot be blocked
+    // further than first_const_pool_use_ + kMaxDistToPool
+    ASSERT((num_prinfo_ == 0) ||
+           (pc_limit < (first_const_pool_use_ + kMaxDistToPool)));
+    no_const_pool_before_ = pc_limit;
+  }
 
-  // There is nothing to do if there are no pending relocation info entries.
-  if (num_prinfo_ == 0) return;
-
-  // We emit a constant pool at regular intervals of about kDistBetweenPools
-  // or when requested by parameter force_emit (e.g. after each function).
-  // We prefer not to emit a jump unless the max distance is reached or if we
-  // are running low on slots, which can happen if a lot of constants are being
-  // emitted (e.g. --debug-code and many static references).
-  int dist = pc_offset() - last_const_pool_end_;
-  if (!force_emit && dist < kMaxDistBetweenPools &&
-      (require_jump || dist < kDistBetweenPools) &&
-      // TODO(1236125): Cleanup the "magic" number below. We know that
-      // the code generation will test every kCheckConstIntervalInst.
-      // Thus we are safe as long as we generate less than 7 constant
-      // entries per instruction.
-      (num_prinfo_ < (kMaxNumPRInfo - (7 * kCheckConstIntervalInst)))) {
-    return;
+  if (next_buffer_check_ < no_const_pool_before_) {
+    next_buffer_check_ = no_const_pool_before_;
   }
+}
 
-  // If we did not return by now, we need to emit the constant pool soon.
 
-  // However, some small sequences of instructions must not be broken up by the
-  // insertion of a constant pool; such sequences are protected by setting
-  // either const_pool_blocked_nesting_ or no_const_pool_before_, which are
-  // both checked here. Also, recursive calls to CheckConstPool are blocked by
-  // no_const_pool_before_.
-  if (const_pool_blocked_nesting_ > 0 || pc_offset() < no_const_pool_before_) {
-    // Emission is currently blocked; make sure we try again as soon as
-    // possible.
-    if (const_pool_blocked_nesting_ > 0) {
-      next_buffer_check_ = pc_offset() + kInstrSize;
-    } else {
-      next_buffer_check_ = no_const_pool_before_;
-    }
-
+void Assembler::CheckConstPool(bool force_emit, bool require_jump) {
+  // Some short sequence of instruction mustn't be broken up by constant pool
+  // emission, such sequences are protected by calls to BlockConstPoolFor and
+  // BlockConstPoolScope.
+  if (is_const_pool_blocked()) {
     // Something is wrong if emission is forced and blocked at the same time.
     ASSERT(!force_emit);
     return;
   }
 
-  int jump_instr = require_jump ? kInstrSize : 0;
+  // There is nothing to do if there are no pending constant pool entries.
+  if (num_prinfo_ == 0)  {
+    // Calculate the offset of the next check.
+    next_buffer_check_ = pc_offset() + kCheckPoolInterval;
+    return;
+  }
 
+  // We emit a constant pool when:
+  //  * requested to do so by parameter force_emit (e.g. after each function).
+  //  * the distance to the first instruction accessing the constant pool is
+  //    kAvgDistToPool or more.
+  //  * no jump is required and the distance to the first instruction accessing
+  //    the constant pool is at least kMaxDistToPool / 2.

[Søren Thygesen Gjesse, 2011/06/14 07:50:56]

And then add ASSERT(first_const_pool_use_ >= 0).

[Rodolph Perfetta, 2011/06/14 22:52:20]

Done.

+  int dist = pc_offset() - first_const_pool_use_;
+  if (!force_emit && dist < kAvgDistToPool &&
+      (require_jump || (dist < (kMaxDistToPool / 2)))) {
+    return;
+  }
+
   // Check that the code buffer is large enough before emitting the constant
-  // pool and relocation information (include the jump over the pool and the
-  // constant pool marker).
-  int max_needed_space =
-      jump_instr + kInstrSize + num_prinfo_*(kInstrSize + kMaxRelocSize);
-  while (buffer_space() <= (max_needed_space + kGap)) GrowBuffer();
+  // pool (include the jump over the pool and the constant pool marker and
+  // the gap to the relocation information).
+  int jump_instr = require_jump ? kInstrSize : 0;
+  int needed_space = jump_instr + kInstrSize + num_prinfo_ * kInstrSize + kGap;
+  while (buffer_space() <= needed_space) GrowBuffer();
 
-  // Block recursive calls to CheckConstPool.
-  BlockConstPoolBefore(pc_offset() + jump_instr + kInstrSize +
-                       num_prinfo_*kInstrSize);
-  // Don't bother to check for the emit calls below.
-  next_buffer_check_ = no_const_pool_before_;
+  {
+    // Block recursive calls to CheckConstPool.
+    BlockConstPoolScope block_const_pool(this);
 
-  // Emit jump over constant pool if necessary.
-  Label after_pool;
-  if (require_jump) b(&after_pool);
+    // Emit jump over constant pool if necessary.
+    Label after_pool;
+    if (require_jump) {
+      b(&after_pool);
+    }
 
-  RecordComment("[ Constant Pool");
+    RecordComment("[ Constant Pool");
 
-  // Put down constant pool marker "Undefined instruction" as specified by
-  // A5.6 (ARMv7) Instruction set encoding.
-  emit(kConstantPoolMarker | num_prinfo_);
+    // Put down constant pool marker "Undefined instruction" as specified by
+    // A5.6 (ARMv7) Instruction set encoding.
+    emit(kConstantPoolMarker | num_prinfo_);
 
-  // Emit constant pool entries.
-  for (int i = 0; i < num_prinfo_; i++) {
-    RelocInfo& rinfo = prinfo_[i];
-    ASSERT(rinfo.rmode() != RelocInfo::COMMENT &&
-           rinfo.rmode() != RelocInfo::POSITION &&
-           rinfo.rmode() != RelocInfo::STATEMENT_POSITION);
-    Instr instr = instr_at(rinfo.pc());
+    // Emit constant pool entries.
+    for (int i = 0; i < num_prinfo_; i++) {
+      RelocInfo& rinfo = prinfo_[i];
+      ASSERT(rinfo.rmode() != RelocInfo::COMMENT &&
+             rinfo.rmode() != RelocInfo::POSITION &&
+             rinfo.rmode() != RelocInfo::STATEMENT_POSITION);
 
-    // Instruction to patch must be a ldr/str [pc, #offset].
-    // P and U set, B and W clear, Rn == pc, offset12 still 0.
-    ASSERT((instr & (7*B25 | P | U | B | W | 15*B16 | kOff12Mask)) ==
-           (2*B25 | P | U | pc.code()*B16));
-    int delta = pc_ - rinfo.pc() - 8;
-    ASSERT(delta >= -4);  // instr could be ldr pc, [pc, #-4] followed by targ32
-    if (delta < 0) {
-      instr &= ~U;
-      delta = -delta;
+      Instr instr = instr_at(rinfo.pc());
+      // Instruction to patch must be 'ldr rd, [pc, #offset]' with offset == 0.
+      ASSERT(IsLdrPcImmediateOffset(instr) && (instr & kOff12Mask) == 0);

[Søren Thygesen Gjesse, 2011/06/14 07:50:56]

How about adding GetLdrPcImmediateOffsetOffset?

[Rodolph Perfetta, 2011/06/14 22:52:20]

Used GetLdrRegImmediateOffset which was already implemented.

+
+      int delta = pc_ - rinfo.pc() - kPcLoadDelta;
+      // 0 is the smallest delta:
+      //   ldr rd, [pc, #0]
+      //   constant pool marker
+      //   data
+      ASSERT(is_uint12(delta));
+
+      instr_at_put(rinfo.pc(), instr + delta);

[Søren Thygesen Gjesse, 2011/06/14 07:50:56]

How about adding SetLdrPcImmediateOffsetOffset?

[Rodolph Perfetta, 2011/06/14 22:52:20]

Used SetLdrRegImmediateOffset which was already implemented.

+      emit(rinfo.data());
     }
-    ASSERT(is_uint12(delta));
-    instr_at_put(rinfo.pc(), instr + delta);
-    emit(rinfo.data());
-  }
-  num_prinfo_ = 0;
-  last_const_pool_end_ = pc_offset();
 
-  RecordComment("]");
+    num_prinfo_ = 0;
+    first_const_pool_use_ = 0;

[Søren Thygesen Gjesse, 2011/06/14 07:50:56]

0 -> -1?

[Rodolph Perfetta, 2011/06/14 22:52:20]

Done.

 
-  if (after_pool.is_linked()) {
-    bind(&after_pool);
+    RecordComment("]");
+
+    if (after_pool.is_linked()) {
+      bind(&after_pool);
+    }
   }
 
   // Since a constant pool was just emitted, move the check offset forward by
   // the standard interval.
-  next_buffer_check_ = pc_offset() + kCheckConstInterval;
+  next_buffer_check_ = pc_offset() + kCheckPoolInterval;
 }