Fix issue 962.

SplitBetween (formely known as Split with 3 arguments) should select split position from [start, end] instead of [start, end[. This should also improve allocation quality (remove certain redundant move patterns).

Also some minor renaming and refactoring to make register allocator code more readable.

BUG=v8:962
TEST=test/mjsunit/regress/regress-962.js

Review URL: http://codereview.chromium.org/5720001

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

src/lithium-allocator.h

@@ -55,6 +55,7 @@ class LPointerMap;
 class LStackSlot;
 class LRegister;
 
+
 // This class represents a single point of a LOperand's lifetime.
 // For each lithium instruction there are exactly two lifetime positions:
 // the beginning and the end of the instruction. Lifetime positions for
@@ -121,7 +122,13 @@ class LifetimePosition {
   // instruction.
   bool IsValid() const { return value_ != -1; }
 
-  static LifetimePosition Invalid() { return LifetimePosition(); }
+  static inline LifetimePosition Invalid() { return LifetimePosition(); }
+
+  static inline LifetimePosition MaxPosition() {
+    // We have to use this kind of getter instead of static member due to
+    // crash bug in GDB.
+    return LifetimePosition(kMaxInt);
+  }
 
  private:
   static const int kStep = 2;
@@ -135,6 +142,13 @@ class LifetimePosition {
 };
 
 
+enum RegisterKind {
+  NONE,
+  GENERAL_REGISTERS,
+  DOUBLE_REGISTERS
+};
+
+
 class LOperand: public ZoneObject {
  public:
   enum Kind {
@@ -594,8 +608,8 @@ class LiveRange: public ZoneObject {
   explicit LiveRange(int id)
       : id_(id),
         spilled_(false),
-        assigned_double_(false),
         assigned_register_(kInvalidAssignment),
+        assigned_register_kind_(NONE),
         last_interval_(NULL),
         first_interval_(NULL),
         first_pos_(NULL),
@@ -620,10 +634,10 @@ class LiveRange: public ZoneObject {
   LOperand* CreateAssignedOperand();
   int assigned_register() const { return assigned_register_; }
   int spill_start_index() const { return spill_start_index_; }
-  void set_assigned_register(int reg, bool double_reg) {
+  void set_assigned_register(int reg, RegisterKind register_kind) {
     ASSERT(!HasRegisterAssigned() && !IsSpilled());
     assigned_register_ = reg;
-    assigned_double_ = double_reg;
+    assigned_register_kind_ = register_kind;
     ConvertOperands();
   }
   void MakeSpilled() {
@@ -652,9 +666,13 @@ class LiveRange: public ZoneObject {
   // Can this live range be spilled at this position.
   bool CanBeSpilled(LifetimePosition pos);
 
+  // Split this live range at the given position which must follow the start of
+  // the range.
+  // All uses following the given position will be moved from this
+  // live range to the result live range.
   void SplitAt(LifetimePosition position, LiveRange* result);
 
-  bool IsDouble() const { return assigned_double_; }
+  bool IsDouble() const { return assigned_register_kind_ == DOUBLE_REGISTERS; }
   bool HasRegisterAssigned() const {
     return assigned_register_ != kInvalidAssignment;
   }
@@ -721,8 +739,8 @@ class LiveRange: public ZoneObject {
 
   int id_;
   bool spilled_;
-  bool assigned_double_;
   int assigned_register_;
+  RegisterKind assigned_register_kind_;
   UseInterval* last_interval_;
   UseInterval* first_interval_;
   UsePosition* first_pos_;
@@ -774,8 +792,8 @@ class LAllocator BASE_EMBEDDED {
   // Checks whether the value of a given virtual register is tagged.
   bool HasTaggedValue(int virtual_register) const;
 
-  // Checks whether the value of a given virtual register is a double.
-  bool HasDoubleValue(int virtual_register) const;
+  // Returns the register kind required by the given virtual register.
+  RegisterKind RequiredRegisterKind(int virtual_register) const;
 
   // Begin a new instruction.
   void BeginInstruction();
@@ -814,12 +832,6 @@ class LAllocator BASE_EMBEDDED {
 #endif
 
  private:
-  enum OperationMode {
-    NONE,
-    CPU_REGISTERS,
-    XMM_REGISTERS
-  };
-
   void MeetRegisterConstraints();
   void ResolvePhis();
   void BuildLiveRanges();
@@ -871,17 +883,38 @@ class LAllocator BASE_EMBEDDED {
   // Helper methods for allocating registers.
   bool TryAllocateFreeReg(LiveRange* range);
   void AllocateBlockedReg(LiveRange* range);
-  void SplitAndSpillIntersecting(LiveRange* range);
+
+  // Live range splitting helpers.
+
+  // Split the given range at the given position.
+  // If range starts at or after the given position then the
+  // original range is returned.
+  // Otherwise returns the live range that starts at pos and contains
+  // all uses from the original range that follow pos. Uses at pos will
+  // still be owned by the original range after splitting.
+  LiveRange* SplitAt(LiveRange* range, LifetimePosition pos);
+
+  // Split the given range in a position from the interval [start, end].
+  LiveRange* SplitBetween(LiveRange* range,
+                          LifetimePosition start,
+                          LifetimePosition end);
+
+  // Find a lifetime position in the interval [start, end] which
+  // is optimal for splitting: it is either header of the outermost
+  // loop covered by this interval or the latest possible position.
   LifetimePosition FindOptimalSplitPos(LifetimePosition start,
                                        LifetimePosition end);
-  LiveRange* Split(LiveRange* range,
-                   LifetimePosition start,
-                   LifetimePosition end);
-  LiveRange* Split(LiveRange* range, LifetimePosition split_pos);
-  void SplitAndSpill(LiveRange* range,
-                     LifetimePosition start,
-                     LifetimePosition end);
-  void SplitAndSpill(LiveRange* range, LifetimePosition at);
+
+  // Spill the given life range after position pos.
+  void SpillAfter(LiveRange* range, LifetimePosition pos);
+
+  // Spill the given life range after position start and up to position end.
+  void SpillBetween(LiveRange* range,
+                    LifetimePosition start,
+                    LifetimePosition end);
+
+  void SplitAndSpillIntersecting(LiveRange* range);
+
   void Spill(LiveRange* range);
   bool IsBlockBoundary(LifetimePosition pos);
   void AddGapMove(int pos, LiveRange* prev, LiveRange* next);
@@ -914,6 +947,8 @@ class LAllocator BASE_EMBEDDED {
   HPhi* LookupPhi(LOperand* operand) const;
   LGap* GetLastGap(HBasicBlock* block) const;
 
+  const char* RegisterName(int allocation_index);
+
   LChunk* chunk_;
   ZoneList<InstructionSummary*> summaries_;
   InstructionSummary* next_summary_;
@@ -938,7 +973,7 @@ class LAllocator BASE_EMBEDDED {
   // Next virtual register number to be assigned to temporaries.
   int next_virtual_register_;
 
-  OperationMode mode_;
+  RegisterKind mode_;
   int num_registers_;
 
   HGraph* graph_;

test/mjsunit/regress/regress-962.js

+// Copyright 2010 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+function L(scope) { this.s = new Object(); }
+
+L.prototype.c = function() { return true; }
+
+function F() {
+  this.l = [new L, new L];
+}
+
+F.prototype.foo = function () {
+    var f, d = arguments,
+        e, b = this.l,
+        g;
+    for (e = 0; e < b.length; e++) {
+        g = b[e];
+        f = g.c.apply(g.s, d);
+        if (f === false) {
+            break
+        }
+    }
+    return f
+}
+
+
+var ctx = new F;
+
+for (var i = 0; i < 10000000; i++) ctx.foo();