[v8-dev] ARM: Improve Lithium register constraints.

src/arm/lithium-arm.cc

Index: src/arm/lithium-arm.cc
diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc
index 998b73b62e9242a6dd2393a919f199f2af5c1859..5f03ec2a7dc59f83bb9f7adcd21bcf1c4b895928 100644
--- a/src/arm/lithium-arm.cc
+++ b/src/arm/lithium-arm.cc
@@ -483,13 +483,21 @@ LOperand* LChunkBuilder::UseTempRegister(HValue* value) {
 
 
 LOperand* LChunkBuilder::Use(HValue* value) {
-  return Use(value, new(zone()) LUnallocated(LUnallocated::NONE));
+  if (value->UseCount() == 1) {

[danno, 2013/08/20 14:53:20]

What is the concrete motivation behind this change and the one below? "Use" mean
precisely that it makes no performance difference where the live range of a
virtual is allocated, stack or register. This change changes this semantic in a
way I'm not convinced is a overall win. Take how we lazily rematerialize the
context register before runtime calls in deferred code (we don't do this in arm
yet, only on ia32, but it's something that is on our TODO list). I expect this
change artificially increases register pressure in such a situation, where what
we should do is eagerly spill the contant context register even though it's used
frequently in deferred code because the deferred code isn't called often.

I _think_ your motivation is to allocate constants into registers with a single
contiguous live range if they are used more than once (not sure). If so, there
is a better and cleaner (but harder) way to do this which involves changes in
the register allocator itself. Basically, we need to teach the register
allocator that it should by default try to allocate constants into registers
that have multiple uses, but split the constant range eagerly if register
pressure is too high and teach it how to rematerlialize those split constants
without spilling/reloading from the stack. Without that fundamental rework, I am
uncomfortable with this part of your change.

[vincent.belliard.fr, 2013/08/22 12:35:48]

The register allocation has been made for IA32. The constraints with ARM are not
the same.
With ARM, the performances will never be the same with a stack slot or with a
register. The operation will always be faster with a register because, with a
stack slot, we must first load the stack slot into a register.

Our first idea was to remove the "Use" function and to always call
"UseRegister". This gave some overall improvements but, when the value was used
only once, it was sometimes a little slower because it asked the allocator to
reserve a register. The need of an extra register cause the allocator to store
some other values on the stack.

For some instructions, it's possible to load a stack slot into the IP register
(it was the meaning of the EmitLoadRegister) but it's only faster if the value
is only used once (if the value is used twice, it would be load twice and this
is slower with ARM).

I think that we don't change too much the meaning of use. The new meaning is
now:
If the value is used once, use either a register or a stack slot, else always
use a register.

It's sometimes difficult for ARM to share the IA32 code. This modification was
done to continue to share the IA32 code but with the maximum of performances for
ARM.

One last point, we removed the EmitLoadRegister function because IP should't be
used outside the macro assembler. We add some functions (XXXMemOperand) which
have the same advantage but which keep the IP use inside the macro assembler.

+    return Use(value, new(zone()) LUnallocated(LUnallocated::NONE));
+  } else {
+    return UseRegister(value);
+  }
 }
 
 
 LOperand* LChunkBuilder::UseAtStart(HValue* value) {
-  return Use(value, new(zone()) LUnallocated(LUnallocated::NONE,
-                                             LUnallocated::USED_AT_START));
+  if (value->UseCount() == 1) {
+    return Use(value, new(zone()) LUnallocated(LUnallocated::NONE,
+                                               LUnallocated::USED_AT_START));
+  } else {
+    return UseRegisterAtStart(value);
+  }
 }
 
 
@@ -756,8 +764,8 @@ LInstruction* LChunkBuilder::DoArithmeticD(Token::Value op,
   ASSERT(instr->left()->representation().IsDouble());
   ASSERT(instr->right()->representation().IsDouble());
   ASSERT(op != Token::MOD);
-  LOperand* left = UseRegisterAtStart(instr->left());
-  LOperand* right = UseRegisterAtStart(instr->right());
+  LOperand* left = UseRegisterAtStart(instr->BetterLeftOperand());
+  LOperand* right = UseAtStart(instr->BetterRightOperand());
   LArithmeticD* result = new(zone()) LArithmeticD(op, left, right);
   return DefineAsRegister(result);
 }
@@ -1084,7 +1092,7 @@ LInstruction* LChunkBuilder::DoApplyArguments(HApplyArguments* instr) {
 
 LInstruction* LChunkBuilder::DoPushArgument(HPushArgument* instr) {
   ++argument_count_;
-  LOperand* argument = Use(instr->argument());
+  LOperand* argument = UseRegisterAtStart(instr->argument());
   return new(zone()) LPushArgument(argument);
 }
 
@@ -1227,7 +1235,7 @@ LInstruction* LChunkBuilder::DoMathTan(HUnaryMathOperation* instr) {
 LInstruction* LChunkBuilder::DoMathExp(HUnaryMathOperation* instr) {
   ASSERT(instr->representation().IsDouble());
   ASSERT(instr->value()->representation().IsDouble());
-  LOperand* input = UseTempRegister(instr->value());
+  LOperand* input = UseRegister(instr->value());
   LOperand* temp1 = TempRegister();
   LOperand* temp2 = TempRegister();
   LOperand* double_temp = FixedTemp(d3);  // Chosen by fair dice roll.
@@ -1358,8 +1366,8 @@ LInstruction* LChunkBuilder::DoDiv(HDiv* instr) {
       ASSERT(!instr->CheckFlag(HValue::kCanBeDivByZero));
       LOperand* value = UseRegisterAtStart(instr->left());
       LDivI* div =
-          new(zone()) LDivI(value, UseOrConstant(instr->right()), NULL);
-      return AssignEnvironment(DefineSameAsFirst(div));
+          new(zone()) LDivI(value, UseConstant(instr->right()), NULL);
+      return AssignEnvironment(DefineAsRegister(div));
     }
     LOperand* dividend = UseRegister(instr->left());
     LOperand* divisor = UseRegister(instr->right());
@@ -1427,15 +1435,16 @@ HValue* LChunkBuilder::SimplifiedDivisorForMathFloorOfDiv(HValue* divisor) {
 
 LInstruction* LChunkBuilder::DoMathFloorOfDiv(HMathFloorOfDiv* instr) {
   HValue* right = instr->right();
-  LOperand* dividend = UseRegister(instr->left());
-  LOperand* divisor = CpuFeatures::IsSupported(SUDIV)
-      ? UseRegister(right)
-      : UseOrConstant(right);
-  LOperand* remainder = TempRegister();
   ASSERT(CpuFeatures::IsSupported(SUDIV) ||
          (right->IsConstant() &&
           HConstant::cast(right)->HasInteger32Value() &&
           HasMagicNumberForDivisor(HConstant::cast(right)->Integer32Value())));
+
+  LOperand* dividend = UseRegister(instr->left());
+  LOperand* divisor = CpuFeatures::IsSupported(SUDIV)
+      ? UseRegister(right)
+      : UseConstant(right);
+  LOperand* remainder = TempRegister();
   return AssignEnvironment(DefineAsRegister(
           new(zone()) LMathFloorOfDiv(dividend, divisor, remainder)));
 }
@@ -1450,7 +1459,7 @@ LInstruction* LChunkBuilder::DoMod(HMod* instr) {
     if (instr->HasPowerOf2Divisor()) {
       ASSERT(!right->CanBeZero());
       LModI* mod = new(zone()) LModI(UseRegisterAtStart(left),
-                                     UseOrConstant(right));
+                                     UseConstant(right));
       LInstruction* result = DefineAsRegister(mod);
       return (left->CanBeNegative() &&
               instr->CheckFlag(HValue::kBailoutOnMinusZero))
@@ -1505,20 +1514,37 @@ LInstruction* LChunkBuilder::DoMul(HMul* instr) {
   if (instr->representation().IsSmiOrInteger32()) {
     ASSERT(instr->left()->representation().Equals(instr->representation()));
     ASSERT(instr->right()->representation().Equals(instr->representation()));
-    LOperand* left;
-    LOperand* right = UseOrConstant(instr->BetterRightOperand());
-    LOperand* temp = NULL;
-    if (instr->CheckFlag(HValue::kBailoutOnMinusZero) &&
-        (instr->CheckFlag(HValue::kCanOverflow) ||
-        !right->IsConstantOperand())) {
-      left = UseRegister(instr->BetterLeftOperand());
-      temp = TempRegister();
+    HValue* left = instr->BetterLeftOperand();
+    HValue* right = instr->BetterRightOperand();
+    LOperand* left_op;
+    LOperand* right_op;
+    bool can_overflow = instr->CheckFlag(HValue::kCanOverflow);
+    bool bailout_on_minus_zero = instr->CheckFlag(HValue::kBailoutOnMinusZero);
+
+    if (can_overflow) {
+      if (bailout_on_minus_zero) {
+        // If the result is zero, the sign of left and right will be checked,
+        // so they need to be kept alive.
+        left_op = UseRegister(left);
+        right_op = UseRegister(right);
+      } else {
+        left_op =  UseRegisterAtStart(left);
+        right_op = UseRegisterAtStart(right);
+      }
     } else {
-      left = UseRegisterAtStart(instr->BetterLeftOperand());
+      if (bailout_on_minus_zero && !right->IsConstant()) {
+        // If the result is zero, the sign of left and right will be checked,
+        // so they need to be kept alive. Multiplication by a constant is
+        // handled differently.
+        left_op = UseRegister(left);
+        right_op = UseRegister(right);
+      } else {
+        left_op = UseRegisterAtStart(left);
+        right_op = UseRegisterOrConstantAtStart(right);
+      }
     }
-    LMulI* mul = new(zone()) LMulI(left, right, temp);
-    if (instr->CheckFlag(HValue::kCanOverflow) ||
-        instr->CheckFlag(HValue::kBailoutOnMinusZero)) {
+    LMulI* mul = new(zone()) LMulI(left_op, right_op);
+    if (can_overflow || bailout_on_minus_zero) {
       AssignEnvironment(mul);
     }
     return DefineAsRegister(mul);
@@ -1557,8 +1583,10 @@ LInstruction* LChunkBuilder::DoSub(HSub* instr) {
     ASSERT(instr->left()->representation().Equals(instr->representation()));
     ASSERT(instr->right()->representation().Equals(instr->representation()));
 
-    if (instr->left()->IsConstant()) {
-      // If lhs is constant, do reverse subtraction instead.
+    if (instr->left()->IsConstant() ||
+        ((instr->left()->UseCount() == 1) &&
+         (instr->right()->UseCount() > 1))) {
+      // If lhs is better right, do reverse subtraction instead.
       return DoRSub(instr);
     }
 
@@ -1657,13 +1685,13 @@ LInstruction* LChunkBuilder::DoMathMinMax(HMathMinMax* instr) {
     ASSERT(instr->left()->representation().Equals(instr->representation()));
     ASSERT(instr->right()->representation().Equals(instr->representation()));
     left = UseRegisterAtStart(instr->BetterLeftOperand());
-    right = UseOrConstantAtStart(instr->BetterRightOperand());
+    right = UseRegisterOrConstantAtStart(instr->BetterRightOperand());
   } else {
     ASSERT(instr->representation().IsDouble());
     ASSERT(instr->left()->representation().IsDouble());
     ASSERT(instr->right()->representation().IsDouble());
-    left = UseRegisterAtStart(instr->left());
-    right = UseRegisterAtStart(instr->right());
+    left = UseRegister(instr->BetterLeftOperand());
+    right = Use(instr->BetterRightOperand());
   }
   return DefineAsRegister(new(zone()) LMathMinMax(left, right));
 }
@@ -1758,7 +1786,7 @@ LInstruction* LChunkBuilder::DoIsStringAndBranch(HIsStringAndBranch* instr) {
 
 LInstruction* LChunkBuilder::DoIsSmiAndBranch(HIsSmiAndBranch* instr) {
   ASSERT(instr->value()->representation().IsTagged());
-  return new(zone()) LIsSmiAndBranch(Use(instr->value()));
+  return new(zone()) LIsSmiAndBranch(UseRegisterAtStart(instr->value()));
 }
 
 
@@ -1844,11 +1872,11 @@ LInstruction* LChunkBuilder::DoDateField(HDateField* instr) {
 
 LInstruction* LChunkBuilder::DoSeqStringSetChar(HSeqStringSetChar* instr) {
   LOperand* string = UseRegister(instr->string());
-  LOperand* index = UseRegister(instr->index());
-  LOperand* value = UseTempRegister(instr->value());
+  LOperand* index = UseRegisterOrConstant(instr->index());
+  LOperand* value = UseRegister(instr->value());
   LSeqStringSetChar* result =
       new(zone()) LSeqStringSetChar(instr->encoding(), string, index, value);
-  return DefineAsRegister(result);
+  return result;
 }
 
 
@@ -1991,7 +2019,7 @@ LInstruction* LChunkBuilder::DoChange(HChange* instr) {
             new(zone()) LUint32ToDouble(UseRegister(instr->value())));
       } else {
         return DefineAsRegister(
-            new(zone()) LInteger32ToDouble(Use(instr->value())));
+            new(zone()) LInteger32ToDouble(UseRegister(instr->value())));
       }
     }
   }
@@ -2181,13 +2209,9 @@ LInstruction* LChunkBuilder::DoLoadKeyed(HLoadKeyed* instr) {
   LLoadKeyed* result = NULL;
 
   if (!instr->is_external()) {
-    LOperand* obj = NULL;
-    if (instr->representation().IsDouble()) {
-      obj = UseTempRegister(instr->elements());
-    } else {
-      ASSERT(instr->representation().IsSmiOrTagged());
-      obj = UseRegisterAtStart(instr->elements());
-    }
+    ASSERT(instr->representation().IsSmiOrTagged() ||
+           instr->representation().IsDouble());
+    LOperand* obj = UseRegisterAtStart(instr->elements());
     result = new(zone()) LLoadKeyed(obj, key);
   } else {
     ASSERT(
@@ -2221,7 +2245,9 @@ LInstruction* LChunkBuilder::DoLoadKeyedGeneric(HLoadKeyedGeneric* instr) {
 
 
 LInstruction* LChunkBuilder::DoStoreKeyed(HStoreKeyed* instr) {
+#ifdef DEBUG
   ElementsKind elements_kind = instr->elements_kind();
+#endif
 
   if (!instr->is_external()) {
     ASSERT(instr->elements()->representation().IsTagged());
@@ -2232,15 +2258,19 @@ LInstruction* LChunkBuilder::DoStoreKeyed(HStoreKeyed* instr) {
 
     if (instr->value()->representation().IsDouble()) {
       object = UseRegisterAtStart(instr->elements());
-      val = UseTempRegister(instr->value());
+      val = UseRegisterAtStart(instr->value());
       key = UseRegisterOrConstantAtStart(instr->key());
     } else {
       ASSERT(instr->value()->representation().IsSmiOrTagged());
-      object = UseTempRegister(instr->elements());
-      val = needs_write_barrier ? UseTempRegister(instr->value())
-          : UseRegisterAtStart(instr->value());
-      key = needs_write_barrier ? UseTempRegister(instr->key())
-          : UseRegisterOrConstantAtStart(instr->key());
+      if (needs_write_barrier) {
+        object = UseTempRegister(instr->elements());
+        val = UseTempRegister(instr->value());
+        key = UseTempRegister(instr->key());
+      } else {
+        object = UseRegisterAtStart(instr->elements());
+        val = UseRegisterAtStart(instr->value());
+        key = UseRegisterOrConstantAtStart(instr->key());
+      }
     }
 
     return new(zone()) LStoreKeyed(object, key, val);
@@ -2254,11 +2284,7 @@ LInstruction* LChunkBuilder::DoStoreKeyed(HStoreKeyed* instr) {
        ((elements_kind == EXTERNAL_FLOAT_ELEMENTS) ||
         (elements_kind == EXTERNAL_DOUBLE_ELEMENTS))));
   ASSERT(instr->elements()->representation().IsExternal());
-  bool val_is_temp_register =
-      elements_kind == EXTERNAL_PIXEL_ELEMENTS ||
-      elements_kind == EXTERNAL_FLOAT_ELEMENTS;
-  LOperand* val = val_is_temp_register ? UseTempRegister(instr->value())
-      : UseRegister(instr->value());
+  LOperand* val = UseRegister(instr->value());
   LOperand* key = UseRegisterOrConstantAtStart(instr->key());
   LOperand* external_pointer = UseRegister(instr->elements());
   return new(zone()) LStoreKeyed(external_pointer, key, val);
@@ -2380,9 +2406,7 @@ LInstruction* LChunkBuilder::DoStringCharFromCode(HStringCharFromCode* instr) {
 
 LInstruction* LChunkBuilder::DoAllocate(HAllocate* instr) {
   info()->MarkAsDeferredCalling();
-  LOperand* size = instr->size()->IsConstant()
-      ? UseConstant(instr->size())
-      : UseTempRegister(instr->size());
+  LOperand* size = UseRegisterOrConstant(instr->size());
   LOperand* temp1 = TempRegister();
   LOperand* temp2 = TempRegister();
   LAllocate* result = new(zone()) LAllocate(size, temp1, temp2);
@@ -2458,15 +2482,8 @@ LInstruction* LChunkBuilder::DoCapturedObject(HCapturedObject* instr) {
 LInstruction* LChunkBuilder::DoAccessArgumentsAt(HAccessArgumentsAt* instr) {
   info()->MarkAsRequiresFrame();
   LOperand* args = UseRegister(instr->arguments());
-  LOperand* length;
-  LOperand* index;
-  if (instr->length()->IsConstant() && instr->index()->IsConstant()) {
-    length = UseRegisterOrConstant(instr->length());
-    index = UseOrConstant(instr->index());
-  } else {
-    length = UseTempRegister(instr->length());
-    index = UseRegisterAtStart(instr->index());
-  }
+  LOperand* length = UseRegisterOrConstantAtStart(instr->length());
+  LOperand* index = UseRegisterOrConstantAtStart(instr->index());
   return DefineAsRegister(new(zone()) LAccessArgumentsAt(args, length, index));
 }
 
@@ -2485,7 +2502,7 @@ LInstruction* LChunkBuilder::DoTypeof(HTypeof* instr) {
 
 
 LInstruction* LChunkBuilder::DoTypeofIsAndBranch(HTypeofIsAndBranch* instr) {
-  return new(zone()) LTypeofIsAndBranch(UseTempRegister(instr->value()));
+  return new(zone()) LTypeofIsAndBranch(UseRegister(instr->value()));
 }