Add fake data dependencies (instead of disabling GVN) to fix code motion

for HInstructions used in polymorphic array access code.

TEST=existing tests pass.

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

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

diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc
index d282f37..6c316cd 100644 (file)
--- a/src/hydrogen-instructions.cc
+++ b/src/hydrogen-instructions.cc
@@ -1798,11 +1798,6 @@ void HSimulate::Verify() {
 }
 
 
-void HBoundsCheck::Verify() {
-  HInstruction::Verify();
-}
-
-
 void HCheckSmi::Verify() {
   HInstruction::Verify();
   ASSERT(HasNoUses());
@@ -1815,18 +1810,6 @@ void HCheckNonSmi::Verify() {
 }
 
 
-void HCheckInstanceType::Verify() {
-  HInstruction::Verify();
-  ASSERT(HasNoUses());
-}
-
-
-void HCheckMap::Verify() {
-  HInstruction::Verify();
-  ASSERT(HasNoUses());
-}
-
-
 void HCheckFunction::Verify() {
   HInstruction::Verify();
   ASSERT(HasNoUses());

diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index 15186ff..63ee683 100644 (file)
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -1663,12 +1663,14 @@ class HCallRuntime: public HCall<1> {
 };
 
 
-class HJSArrayLength: public HUnaryOperation {
+class HJSArrayLength: public HTemplateInstruction<2> {
  public:
-  explicit HJSArrayLength(HValue* value) : HUnaryOperation(value) {
+  HJSArrayLength(HValue* value, HValue* typecheck) {
     // The length of an array is stored as a tagged value in the array
     // object. It is guaranteed to be 32 bit integer, but it can be
     // represented as either a smi or heap number.
+    SetOperandAt(0, value);
+    SetOperandAt(1, typecheck);
     set_representation(Representation::Tagged());
     SetFlag(kUseGVN);
     SetFlag(kDependsOnArrayLengths);
@@ -1679,6 +1681,8 @@ class HJSArrayLength: public HUnaryOperation {
     return Representation::Tagged();
   }
 
+  HValue* value() { return OperandAt(0); }
+
   DECLARE_CONCRETE_INSTRUCTION(JSArrayLength)
 
  protected:
@@ -1909,10 +1913,6 @@ class HCheckMap: public HUnaryOperation {
   virtual void PrintDataTo(StringStream* stream);
   virtual HType CalculateInferredType();
 
-#ifdef DEBUG
-  virtual void Verify();
-#endif
-
   Handle<Map> map() const { return map_; }
 
   DECLARE_CONCRETE_INSTRUCTION(CheckMap)
@@ -1980,10 +1980,6 @@ class HCheckInstanceType: public HUnaryOperation {
     return Representation::Tagged();
   }
 
-#ifdef DEBUG
-  virtual void Verify();
-#endif
-
   virtual HValue* Canonicalize();
 
   bool is_interval_check() const { return check_ <= LAST_INTERVAL_CHECK; }
@@ -2458,10 +2454,6 @@ class HBoundsCheck: public HTemplateInstruction<2> {
     return Representation::Integer32();
   }
 
-#ifdef DEBUG
-  virtual void Verify();
-#endif
-
   HValue* index() { return OperandAt(0); }
   HValue* length() { return OperandAt(1); }
 

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 7e62ec4..69a060d 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -3274,8 +3274,8 @@ void HGraphBuilder::VisitArrayLiteral(ArrayLiteral* expr) {
 
     // Load the elements array before the first store.
     if (elements == NULL)  {
-     elements = new(zone()) HLoadElements(literal);
-     AddInstruction(elements);
+      elements = new(zone()) HLoadElements(literal);
+      AddInstruction(elements);
     }
 
     HValue* key = AddInstruction(
@@ -3908,12 +3908,16 @@ HInstruction* HGraphBuilder::BuildMonomorphicElementAccess(HValue* object,
                     : BuildLoadKeyedGeneric(object, key);
   }
   AddInstruction(new(zone()) HCheckNonSmi(object));
-  AddInstruction(new(zone()) HCheckMap(object, map));
-  HInstruction* elements = new(zone()) HLoadElements(object);
+  HInstruction* mapcheck = AddInstruction(new(zone()) HCheckMap(object, map));
+  HInstruction* elements = AddInstruction(new(zone()) HLoadElements(object));
+  bool fast_double_elements = map->has_fast_double_elements();
+  if (is_store && !fast_double_elements) {
+    AddInstruction(new(zone()) HCheckMap(
+        elements, isolate()->factory()->fixed_array_map()));
+  }
   HInstruction* length = NULL;
   HInstruction* checked_key = NULL;
   if (map->has_external_array_elements()) {
-    AddInstruction(elements);
     length = AddInstruction(new(zone()) HExternalArrayLength(elements));
     checked_key = AddInstruction(new(zone()) HBoundsCheck(key, length));
     HLoadExternalArrayPointer* external_elements =
@@ -3922,25 +3926,13 @@ HInstruction* HGraphBuilder::BuildMonomorphicElementAccess(HValue* object,
     return BuildExternalArrayElementAccess(external_elements, checked_key,
                                            val, map->elements_kind(), is_store);
   }
-  bool fast_double_elements = map->has_fast_double_elements();
   ASSERT(map->has_fast_elements() || fast_double_elements);
   if (map->instance_type() == JS_ARRAY_TYPE) {
-    length = AddInstruction(new(zone()) HJSArrayLength(object));
-    checked_key = AddInstruction(new(zone()) HBoundsCheck(key, length));
-    AddInstruction(elements);
-    if (is_store && !fast_double_elements) {
-      AddInstruction(new(zone()) HCheckMap(
-          elements, isolate()->factory()->fixed_array_map()));
-    }
+    length = AddInstruction(new(zone()) HJSArrayLength(object, mapcheck));
   } else {
-    AddInstruction(elements);
-    if (is_store && !fast_double_elements) {
-      AddInstruction(new(zone()) HCheckMap(
-          elements, isolate()->factory()->fixed_array_map()));
-    }
     length = AddInstruction(new(zone()) HFixedArrayLength(elements));
-    checked_key = AddInstruction(new(zone()) HBoundsCheck(key, length));
   }
+  checked_key = AddInstruction(new(zone()) HBoundsCheck(key, length));
   if (is_store) {
     if (fast_double_elements) {
       return new(zone()) HStoreKeyedFastDoubleElement(elements,
@@ -3992,7 +3984,8 @@ HValue* HGraphBuilder::HandlePolymorphicElementAccess(HValue* object,
 
   HInstruction* elements_kind_instr =
       AddInstruction(new(zone()) HElementsKind(object));
-  HInstruction* elements = NULL;
+  HCompareConstantEqAndBranch* elements_kind_branch = NULL;
+  HInstruction* elements = AddInstruction(new(zone()) HLoadElements(object));
   HLoadExternalArrayPointer* external_elements = NULL;
   HInstruction* checked_key = NULL;
 
@@ -4008,14 +4001,6 @@ HValue* HGraphBuilder::HandlePolymorphicElementAccess(HValue* object,
                   JSObject::LAST_ELEMENTS_KIND);
     if (elements_kind == JSObject::FIRST_EXTERNAL_ARRAY_ELEMENTS_KIND
         && todo_external_array) {
-      elements = AddInstruction(new(zone()) HLoadElements(object));
-      // We need to forcibly prevent some ElementsKind-dependent instructions
-      // from being hoisted out of any loops they might occur in, because
-      // the current loop-invariant-code-motion algorithm isn't clever enough
-      // to deal with them properly.
-      // There's some performance to be gained by developing a smarter
-      // solution for this.
-      elements->ClearFlag(HValue::kUseGVN);
       HInstruction* length =
           AddInstruction(new(zone()) HExternalArrayLength(elements));
       checked_key = AddInstruction(new(zone()) HBoundsCheck(key, length));
@@ -4025,18 +4010,22 @@ HValue* HGraphBuilder::HandlePolymorphicElementAccess(HValue* object,
     if (type_todo[elements_kind]) {
       HBasicBlock* if_true = graph()->CreateBasicBlock();
       HBasicBlock* if_false = graph()->CreateBasicBlock();
-      HCompareConstantEqAndBranch* compare =
-          new(zone()) HCompareConstantEqAndBranch(elements_kind_instr,
-                                                  elements_kind,
-                                                  Token::EQ_STRICT);
-      compare->SetSuccessorAt(0, if_true);
-      compare->SetSuccessorAt(1, if_false);
-      current_block()->Finish(compare);
+      elements_kind_branch = new(zone()) HCompareConstantEqAndBranch(
+          elements_kind_instr, elements_kind, Token::EQ_STRICT);
+      elements_kind_branch->SetSuccessorAt(0, if_true);
+      elements_kind_branch->SetSuccessorAt(1, if_false);
+      current_block()->Finish(elements_kind_branch);
 
       set_current_block(if_true);
       HInstruction* access;
       if (elements_kind == JSObject::FAST_ELEMENTS ||
           elements_kind == JSObject::FAST_DOUBLE_ELEMENTS) {
+        bool fast_double_elements =
+            elements_kind == JSObject::FAST_DOUBLE_ELEMENTS;
+        if (is_store && !fast_double_elements) {
+          AddInstruction(new(zone()) HCheckMap(
+              elements, isolate()->factory()->fixed_array_map()));
+        }
         HBasicBlock* if_jsarray = graph()->CreateBasicBlock();
         HBasicBlock* if_fastobject = graph()->CreateBasicBlock();
         HHasInstanceTypeAndBranch* typecheck =
@@ -4046,14 +4035,9 @@ HValue* HGraphBuilder::HandlePolymorphicElementAccess(HValue* object,
         current_block()->Finish(typecheck);
 
         set_current_block(if_jsarray);
-        HInstruction* length = new(zone()) HJSArrayLength(object);
+        HInstruction* length = new(zone()) HJSArrayLength(object, typecheck);
         AddInstruction(length);
-        length->ClearFlag(HValue::kUseGVN);
         checked_key = AddInstruction(new(zone()) HBoundsCheck(key, length));
-        elements = AddInstruction(new(zone()) HLoadElements(object));
-        elements->ClearFlag(HValue::kUseGVN);
-        bool fast_double_elements =
-            elements_kind == JSObject::FAST_DOUBLE_ELEMENTS;
         if (is_store) {
           if (fast_double_elements) {
             access = AddInstruction(
@@ -4061,8 +4045,6 @@ HValue* HGraphBuilder::HandlePolymorphicElementAccess(HValue* object,
                                                          checked_key,
                                                          val));
           } else {
-            AddInstruction(new(zone()) HCheckMap(
-                elements, isolate()->factory()->fixed_array_map()));
             access = AddInstruction(
                 new(zone()) HStoreKeyedFastElement(elements, checked_key, val));
           }
@@ -4083,12 +4065,6 @@ HValue* HGraphBuilder::HandlePolymorphicElementAccess(HValue* object,
         if_jsarray->Goto(join);
 
         set_current_block(if_fastobject);
-        elements = AddInstruction(new(zone()) HLoadElements(object));
-        elements->ClearFlag(HValue::kUseGVN);
-        if (is_store && !fast_double_elements) {
-          AddInstruction(new(zone()) HCheckMap(
-              elements, isolate()->factory()->fixed_array_map()));
-        }
         length = AddInstruction(new(zone()) HFixedArrayLength(elements));
         checked_key = AddInstruction(new(zone()) HBoundsCheck(key, length));
         if (is_store) {
@@ -4233,8 +4209,9 @@ void HGraphBuilder::VisitProperty(Property* expr) {
   if (expr->IsArrayLength()) {
     HValue* array = Pop();
     AddInstruction(new(zone()) HCheckNonSmi(array));
-    AddInstruction(HCheckInstanceType::NewIsJSArray(array));
-    instr = new(zone()) HJSArrayLength(array);
+    HInstruction* mapcheck =
+        AddInstruction(HCheckInstanceType::NewIsJSArray(array));
+    instr = new(zone()) HJSArrayLength(array, mapcheck);
 
   } else if (expr->IsStringLength()) {
     HValue* string = Pop();