Fixed two register allocator bugs (off-by-one error/failure propagation).

Minor cleanups on the way, e.g. making sure that we never use something after an allocation failed. Style question: Should we switch to some kind of MUST_USE_RESULT-style to ensure that we handle failures consistently? Not sure...

BUG=v8:2576

Review URL: https://codereview.chromium.org/12867002

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

diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc
index 6961a0d..7272f54 100644 (file)
--- a/src/arm/lithium-arm.cc
+++ b/src/arm/lithium-arm.cc
@@ -675,8 +675,12 @@ LInstruction* LChunkBuilder::AssignPointerMap(LInstruction* instr) {
 LUnallocated* LChunkBuilder::TempRegister() {
   LUnallocated* operand =
       new(zone()) LUnallocated(LUnallocated::MUST_HAVE_REGISTER);
-  operand->set_virtual_register(allocator_->GetVirtualRegister());
-  if (!allocator_->AllocationOk()) Abort("Not enough virtual registers.");
+  int vreg = allocator_->GetVirtualRegister();
+  if (!allocator_->AllocationOk()) {
+    Abort("Out of virtual registers while trying to allocate temp register.");
+    return NULL;
+  }
+  operand->set_virtual_register(vreg);
   return operand;
 }
 

diff --git a/src/ia32/lithium-ia32.cc b/src/ia32/lithium-ia32.cc
index f98e6c0..4a380ef 100644 (file)
--- a/src/ia32/lithium-ia32.cc
+++ b/src/ia32/lithium-ia32.cc
@@ -725,10 +725,12 @@ LInstruction* LChunkBuilder::AssignPointerMap(LInstruction* instr) {
 LUnallocated* LChunkBuilder::TempRegister() {
   LUnallocated* operand =
       new(zone()) LUnallocated(LUnallocated::MUST_HAVE_REGISTER);
-  operand->set_virtual_register(allocator_->GetVirtualRegister());
+  int vreg = allocator_->GetVirtualRegister();
   if (!allocator_->AllocationOk()) {
-    Abort("Not enough virtual registers (temps).");
+    Abort("Out of virtual registers while trying to allocate temp register.");
+    return NULL;
   }
+  operand->set_virtual_register(vreg);
   return operand;
 }
 

diff --git a/src/lithium-allocator.cc b/src/lithium-allocator.cc
index e3b9a88..6ee9ef2 100644 (file)
--- a/src/lithium-allocator.cc
+++ b/src/lithium-allocator.cc
@@ -839,8 +839,9 @@ void LAllocator::MeetConstraintsBetween(LInstruction* first,
         ASSERT(!cur_input->IsUsedAtStart());
 
         LUnallocated* input_copy = cur_input->CopyUnconstrained(zone());
-        cur_input->set_virtual_register(GetVirtualRegister());
+        int vreg = GetVirtualRegister();
         if (!AllocationOk()) return;
+        cur_input->set_virtual_register(vreg);
 
         if (RequiredRegisterKind(input_copy->virtual_register()) ==
             DOUBLE_REGISTERS) {
@@ -1924,6 +1925,7 @@ void LAllocator::AllocateBlockedReg(LiveRange* current) {
     LiveRange* tail = SplitBetween(current,
                                    current->Start(),
                                    block_pos[reg].InstructionStart());
+    if (!AllocationOk()) return;
     AddToUnhandledSorted(tail);
   }
 
@@ -1954,6 +1956,7 @@ void LAllocator::SplitAndSpillIntersecting(LiveRange* current) {
       } else {
         SpillBetween(range, split_pos, next_pos->pos());
       }
+      if (!AllocationOk()) return;
       ActiveToHandled(range);
       --i;
     }
@@ -1972,6 +1975,7 @@ void LAllocator::SplitAndSpillIntersecting(LiveRange* current) {
           next_intersection = Min(next_intersection, next_pos->pos());
           SpillBetween(range, split_pos, next_intersection);
         }
+        if (!AllocationOk()) return;
         InactiveToHandled(range);
         --i;
       }
@@ -1997,8 +2001,9 @@ LiveRange* LAllocator::SplitRangeAt(LiveRange* range, LifetimePosition pos) {
   ASSERT(pos.IsInstructionStart() ||
          !chunk_->instructions()->at(pos.InstructionIndex())->IsControl());
 
-  LiveRange* result = LiveRangeFor(GetVirtualRegister());
+  int vreg = GetVirtualRegister();
   if (!AllocationOk()) return NULL;
+  LiveRange* result = LiveRangeFor(vreg);
   range->SplitAt(pos, result, zone_);
   return result;
 }
@@ -2075,6 +2080,7 @@ void LAllocator::SpillBetween(LiveRange* range,
         second_part,
         second_part->Start().InstructionEnd(),
         end.PrevInstruction().InstructionEnd());
+    if (!AllocationOk()) return;
 
     ASSERT(third_part != second_part);
 

diff --git a/src/lithium-allocator.h b/src/lithium-allocator.h
index 2ca8537..0e24d54 100644 (file)
--- a/src/lithium-allocator.h
+++ b/src/lithium-allocator.h
@@ -427,8 +427,10 @@ class LAllocator BASE_EMBEDDED {
   Zone* zone() const { return zone_; }
 
   int GetVirtualRegister() {
-    if (next_virtual_register_ > LUnallocated::kMaxVirtualRegisters) {
+    if (next_virtual_register_ >= LUnallocated::kMaxVirtualRegisters) {
       allocation_ok_ = false;
+      // Maintain the invariant that we return something below the maximum.
+      return 0;
     }
     return next_virtual_register_++;
   }

diff --git a/src/mips/lithium-mips.cc b/src/mips/lithium-mips.cc
index 757ae1c..f04a462 100644 (file)
--- a/src/mips/lithium-mips.cc
+++ b/src/mips/lithium-mips.cc
@@ -668,8 +668,12 @@ LInstruction* LChunkBuilder::AssignPointerMap(LInstruction* instr) {
 LUnallocated* LChunkBuilder::TempRegister() {
   LUnallocated* operand =
       new(zone()) LUnallocated(LUnallocated::MUST_HAVE_REGISTER);
-  operand->set_virtual_register(allocator_->GetVirtualRegister());
-  if (!allocator_->AllocationOk()) Abort("Not enough virtual registers.");
+  int vreg = allocator_->GetVirtualRegister();
+  if (!allocator_->AllocationOk()) {
+    Abort("Out of virtual registers while trying to allocate temp register.");
+    return NULL;
+  }
+  operand->set_virtual_register(vreg);
   return operand;
 }
 

diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc
index 394637c..d9c2f95 100644 (file)
--- a/src/x64/lithium-x64.cc
+++ b/src/x64/lithium-x64.cc
@@ -683,8 +683,12 @@ LInstruction* LChunkBuilder::AssignPointerMap(LInstruction* instr) {
 LUnallocated* LChunkBuilder::TempRegister() {
   LUnallocated* operand =
       new(zone()) LUnallocated(LUnallocated::MUST_HAVE_REGISTER);
-  operand->set_virtual_register(allocator_->GetVirtualRegister());
-  if (!allocator_->AllocationOk()) Abort("Not enough virtual registers.");
+  int vreg = allocator_->GetVirtualRegister();
+  if (!allocator_->AllocationOk()) {
+    Abort("Out of virtual registers while trying to allocate temp register.");
+    return NULL;
+  }
+  operand->set_virtual_register(vreg);
   return operand;
 }
 

diff --git a/test/cctest/test-heap-profiler.cc b/test/cctest/test-heap-profiler.cc
index 6962484..2277c3d 100644 (file)
--- a/test/cctest/test-heap-profiler.cc
+++ b/test/cctest/test-heap-profiler.cc
@@ -1731,9 +1731,9 @@ TEST(MapHasDescriptorsAndTransitions) {
 TEST(ManyLocalsInSharedContext) {
   v8::HandleScope scope;
   LocalContext env;
-  int num_objects = 5000;
+  int num_objects = 6000;
   CompileRun(
-      "var n = 5000;"
+      "var n = 6000;"
       "var result = [];"
       "result.push('(function outer() {');"
       "for (var i = 0; i < n; i++) {"