Reland "Add basic crankshaft support for slow-mode for-in to avoid disabling optimiza...

BUG=chromium:476592
LOG=n

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

Cr-Commit-Position: refs/heads/master@{#27898}

diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index 2aa549a..25baa70 100644 (file)
--- a/src/arm/lithium-codegen-arm.cc
+++ b/src/arm/lithium-codegen-arm.cc
@@ -5762,15 +5762,6 @@ void LCodeGen::DoOsrEntry(LOsrEntry* instr) {
 
 
 void LCodeGen::DoForInPrepareMap(LForInPrepareMap* instr) {
-  __ LoadRoot(ip, Heap::kUndefinedValueRootIndex);
-  __ cmp(r0, ip);
-  DeoptimizeIf(eq, instr, Deoptimizer::kUndefined);
-
-  Register null_value = r5;
-  __ LoadRoot(null_value, Heap::kNullValueRootIndex);
-  __ cmp(r0, null_value);
-  DeoptimizeIf(eq, instr, Deoptimizer::kNull);
-
   __ SmiTst(r0);
   DeoptimizeIf(eq, instr, Deoptimizer::kSmi);
 
@@ -5779,6 +5770,8 @@ void LCodeGen::DoForInPrepareMap(LForInPrepareMap* instr) {
   DeoptimizeIf(le, instr, Deoptimizer::kWrongInstanceType);
 
   Label use_cache, call_runtime;
+  Register null_value = r5;
+  __ LoadRoot(null_value, Heap::kNullValueRootIndex);
   __ CheckEnumCache(null_value, &call_runtime);
 
   __ ldr(r0, FieldMemOperand(r0, HeapObject::kMapOffset));

diff --git a/src/arm64/lithium-codegen-arm64.cc b/src/arm64/lithium-codegen-arm64.cc
index 44937d4..7c9ee11 100644 (file)
--- a/src/arm64/lithium-codegen-arm64.cc
+++ b/src/arm64/lithium-codegen-arm64.cc
@@ -2897,13 +2897,6 @@ void LCodeGen::DoForInPrepareMap(LForInPrepareMap* instr) {
   DCHECK(instr->IsMarkedAsCall());
   DCHECK(object.Is(x0));
 
-  DeoptimizeIfRoot(object, Heap::kUndefinedValueRootIndex, instr,
-                   Deoptimizer::kUndefined);
-
-  __ LoadRoot(null_value, Heap::kNullValueRootIndex);
-  __ Cmp(object, null_value);
-  DeoptimizeIf(eq, instr, Deoptimizer::kNull);
-
   DeoptimizeIfSmi(object, instr, Deoptimizer::kSmi);
 
   STATIC_ASSERT(FIRST_JS_PROXY_TYPE == FIRST_SPEC_OBJECT_TYPE);
@@ -2911,6 +2904,7 @@ void LCodeGen::DoForInPrepareMap(LForInPrepareMap* instr) {
   DeoptimizeIf(le, instr, Deoptimizer::kNotAJavaScriptObject);
 
   Label use_cache, call_runtime;
+  __ LoadRoot(null_value, Heap::kNullValueRootIndex);
   __ CheckEnumCache(object, null_value, x1, x2, x3, x4, &call_runtime);
 
   __ Ldr(object, FieldMemOperand(object, HeapObject::kMapOffset));

diff --git a/src/bailout-reason.h b/src/bailout-reason.h
index e48d524..09c889f 100644 (file)
--- a/src/bailout-reason.h
+++ b/src/bailout-reason.h
@@ -94,7 +94,6 @@ namespace internal {
   V(kExternalStringExpectedButNotFound,                                        \
     "External string expected, but not found")                                 \
   V(kFailedBailedOutLastTime, "Failed/bailed out last time")                   \
-  V(kForInStatementIsNotFastCase, "ForInStatement is not fast case")           \
   V(kForInStatementOptimizationIsDisabled,                                     \
     "ForInStatement optimization is disabled")                                 \
   V(kForInStatementWithNonLocalEachVariable,                                   \

diff --git a/src/deoptimizer.h b/src/deoptimizer.h
index c025487..25e96f2 100644 (file)
--- a/src/deoptimizer.h
+++ b/src/deoptimizer.h
@@ -160,7 +160,8 @@ class OptimizedFunctionVisitor BASE_EMBEDDED {
   V(kUnknownMap, "Unknown map")                                                \
   V(kValueMismatch, "value mismatch")                                          \
   V(kWrongInstanceType, "wrong instance type")                                 \
-  V(kWrongMap, "wrong map")
+  V(kWrongMap, "wrong map")                                                    \
+  V(kUndefinedOrNullInForIn, "null or undefined in for-in")
 
 
 class Deoptimizer : public Malloced {

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 8ee79e7..e966d59 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -5051,10 +5051,6 @@ void HOptimizedGraphBuilder::VisitForInStatement(ForInStatement* stmt) {
     return Bailout(kForInStatementOptimizationIsDisabled);
   }
 
-  if (stmt->for_in_type() != ForInStatement::FAST_FOR_IN) {
-    return Bailout(kForInStatementIsNotFastCase);
-  }
-
   if (!stmt->each()->IsVariableProxy() ||
       !stmt->each()->AsVariableProxy()->var()->IsStackLocal()) {
     return Bailout(kForInStatementWithNonLocalEachVariable);
@@ -5065,13 +5061,57 @@ void HOptimizedGraphBuilder::VisitForInStatement(ForInStatement* stmt) {
   CHECK_ALIVE(VisitForValue(stmt->enumerable()));
   HValue* enumerable = Top();  // Leave enumerable at the top.
 
-  HInstruction* map = Add<HForInPrepareMap>(enumerable);
-  Add<HSimulate>(stmt->PrepareId());
-
-  HInstruction* array = Add<HForInCacheArray>(
-      enumerable, map, DescriptorArray::kEnumCacheBridgeCacheIndex);
+  IfBuilder if_undefined_or_null(this);
+  if_undefined_or_null.If<HCompareObjectEqAndBranch>(
+      enumerable, graph()->GetConstantUndefined());
+  if_undefined_or_null.Or();
+  if_undefined_or_null.If<HCompareObjectEqAndBranch>(
+      enumerable, graph()->GetConstantNull());
+  if_undefined_or_null.ThenDeopt(Deoptimizer::kUndefinedOrNullInForIn);
+  if_undefined_or_null.End();
+  BuildForInBody(stmt, each_var, enumerable);
+}
+
+
+void HOptimizedGraphBuilder::BuildForInBody(ForInStatement* stmt,
+                                            Variable* each_var,
+                                            HValue* enumerable) {
+  HInstruction* map;
+  HInstruction* array;
+  HInstruction* enum_length;
+  bool fast = stmt->for_in_type() == ForInStatement::FAST_FOR_IN;
+  if (fast) {
+    map = Add<HForInPrepareMap>(enumerable);
+    Add<HSimulate>(stmt->PrepareId());
+
+    array = Add<HForInCacheArray>(enumerable, map,
+                                  DescriptorArray::kEnumCacheBridgeCacheIndex);
+    enum_length = Add<HMapEnumLength>(map);
+
+    HInstruction* index_cache = Add<HForInCacheArray>(
+        enumerable, map, DescriptorArray::kEnumCacheBridgeIndicesCacheIndex);
+    HForInCacheArray::cast(array)
+        ->set_index_cache(HForInCacheArray::cast(index_cache));
+  } else {
+    Add<HSimulate>(stmt->PrepareId());
+    {
+      NoObservableSideEffectsScope no_effects(this);
+      BuildJSObjectCheck(enumerable, 0);
+    }
+    Add<HSimulate>(stmt->ToObjectId());
 
-  HInstruction* enum_length = Add<HMapEnumLength>(map);
+    map = graph()->GetConstant1();
+    Runtime::FunctionId function_id = Runtime::kGetPropertyNamesFast;
+    Add<HPushArguments>(enumerable);
+    array = Add<HCallRuntime>(isolate()->factory()->empty_string(),
+                              Runtime::FunctionForId(function_id), 1);
+    Push(array);
+    Add<HSimulate>(stmt->EnumId());
+    Drop(1);
+    Handle<Map> array_map = isolate()->factory()->fixed_array_map();
+    HValue* check = Add<HCheckMaps>(array, array_map);
+    enum_length = AddLoadFixedArrayLength(array, check);
+  }
 
   HInstruction* start_index = Add<HConstant>(0);
 
@@ -5080,13 +5120,12 @@ void HOptimizedGraphBuilder::VisitForInStatement(ForInStatement* stmt) {
   Push(enum_length);
   Push(start_index);
 
-  HInstruction* index_cache = Add<HForInCacheArray>(
-      enumerable, map, DescriptorArray::kEnumCacheBridgeIndicesCacheIndex);
-  HForInCacheArray::cast(array)
-      ->set_index_cache(HForInCacheArray::cast(index_cache));
-
   HBasicBlock* loop_entry = BuildLoopEntry(stmt);
 
+  // Reload the values to ensure we have up-to-date values inside of the loop.
+  // This is relevant especially for OSR where the values don't come from the
+  // computation above, but from the OSR entry block.
+  enumerable = environment()->ExpressionStackAt(4);
   HValue* index = environment()->ExpressionStackAt(0);
   HValue* limit = environment()->ExpressionStackAt(1);
 
@@ -5110,15 +5149,21 @@ void HOptimizedGraphBuilder::VisitForInStatement(ForInStatement* stmt) {
 
   HValue* key =
       Add<HLoadKeyed>(environment()->ExpressionStackAt(2),  // Enum cache.
-                      environment()->ExpressionStackAt(0),  // Iteration index.
-                      environment()->ExpressionStackAt(0), FAST_ELEMENTS);
+                      index, index, FAST_ELEMENTS);
 
-  // Check if the expected map still matches that of the enumerable.
-  // If not just deoptimize.
-  Add<HCheckMapValue>(environment()->ExpressionStackAt(4),
-                      environment()->ExpressionStackAt(3));
-
-  Bind(each_var, key);
+  if (fast) {
+    // Check if the expected map still matches that of the enumerable.
+    // If not just deoptimize.
+    Add<HCheckMapValue>(enumerable, environment()->ExpressionStackAt(3));
+    Bind(each_var, key);
+  } else {
+    HValue* function = AddLoadJSBuiltin(Builtins::FILTER_KEY);
+    Add<HPushArguments>(enumerable, key);
+    key = Add<HInvokeFunction>(function, 2);
+    Bind(each_var, key);
+    Add<HSimulate>(stmt->AssignmentId());
+    Add<HCheckHeapObject>(key);
+  }
 
   BreakAndContinueInfo break_info(stmt, scope(), 5);
   {

diff --git a/src/hydrogen.h b/src/hydrogen.h
index b162328..77b741c 100644 (file)
--- a/src/hydrogen.h
+++ b/src/hydrogen.h
@@ -2258,6 +2258,9 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
   void VisitLoopBody(IterationStatement* stmt,
                      HBasicBlock* loop_entry);
 
+  void BuildForInBody(ForInStatement* stmt, Variable* each_var,
+                      HValue* enumerable);
+
   // Create a back edge in the flow graph.  body_exit is the predecessor
   // block and loop_entry is the successor block.  loop_successor is the
   // block where control flow exits the loop normally (e.g., via failure of

diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc
index a46cd4b..5ecb80a 100644 (file)
--- a/src/ia32/lithium-codegen-ia32.cc
+++ b/src/ia32/lithium-codegen-ia32.cc
@@ -5599,12 +5599,6 @@ void LCodeGen::DoOsrEntry(LOsrEntry* instr) {
 
 void LCodeGen::DoForInPrepareMap(LForInPrepareMap* instr) {
   DCHECK(ToRegister(instr->context()).is(esi));
-  __ cmp(eax, isolate()->factory()->undefined_value());
-  DeoptimizeIf(equal, instr, Deoptimizer::kUndefined);
-
-  __ cmp(eax, isolate()->factory()->null_value());
-  DeoptimizeIf(equal, instr, Deoptimizer::kNull);
-
   __ test(eax, Immediate(kSmiTagMask));
   DeoptimizeIf(zero, instr, Deoptimizer::kSmi);
 

diff --git a/src/mips/lithium-codegen-mips.cc b/src/mips/lithium-codegen-mips.cc
index 2b2e767..e101bb4 100644 (file)
--- a/src/mips/lithium-codegen-mips.cc
+++ b/src/mips/lithium-codegen-mips.cc
@@ -5819,13 +5819,6 @@ void LCodeGen::DoOsrEntry(LOsrEntry* instr) {
 void LCodeGen::DoForInPrepareMap(LForInPrepareMap* instr) {
   Register result = ToRegister(instr->result());
   Register object = ToRegister(instr->object());
-  __ LoadRoot(at, Heap::kUndefinedValueRootIndex);
-  DeoptimizeIf(eq, instr, Deoptimizer::kUndefined, object, Operand(at));
-
-  Register null_value = t1;
-  __ LoadRoot(null_value, Heap::kNullValueRootIndex);
-  DeoptimizeIf(eq, instr, Deoptimizer::kNull, object, Operand(null_value));
-
   __ And(at, object, kSmiTagMask);
   DeoptimizeIf(eq, instr, Deoptimizer::kSmi, at, Operand(zero_reg));
 
@@ -5836,6 +5829,8 @@ void LCodeGen::DoForInPrepareMap(LForInPrepareMap* instr) {
 
   Label use_cache, call_runtime;
   DCHECK(object.is(a0));
+  Register null_value = t1;
+  __ LoadRoot(null_value, Heap::kNullValueRootIndex);
   __ CheckEnumCache(null_value, &call_runtime);
 
   __ lw(result, FieldMemOperand(object, HeapObject::kMapOffset));

diff --git a/src/mips64/lithium-codegen-mips64.cc b/src/mips64/lithium-codegen-mips64.cc
index 2e6ede5..7df1676 100644 (file)
--- a/src/mips64/lithium-codegen-mips64.cc
+++ b/src/mips64/lithium-codegen-mips64.cc
@@ -5880,12 +5880,6 @@ void LCodeGen::DoOsrEntry(LOsrEntry* instr) {
 void LCodeGen::DoForInPrepareMap(LForInPrepareMap* instr) {
   Register result = ToRegister(instr->result());
   Register object = ToRegister(instr->object());
-  __ LoadRoot(at, Heap::kUndefinedValueRootIndex);
-  DeoptimizeIf(eq, instr, Deoptimizer::kUndefined, object, Operand(at));
-
-  Register null_value = a5;
-  __ LoadRoot(null_value, Heap::kNullValueRootIndex);
-  DeoptimizeIf(eq, instr, Deoptimizer::kNull, object, Operand(null_value));
 
   __ And(at, object, kSmiTagMask);
   DeoptimizeIf(eq, instr, Deoptimizer::kSmi, at, Operand(zero_reg));
@@ -5897,6 +5891,8 @@ void LCodeGen::DoForInPrepareMap(LForInPrepareMap* instr) {
 
   Label use_cache, call_runtime;
   DCHECK(object.is(a0));
+  Register null_value = a5;
+  __ LoadRoot(null_value, Heap::kNullValueRootIndex);
   __ CheckEnumCache(null_value, &call_runtime);
 
   __ ld(result, FieldMemOperand(object, HeapObject::kMapOffset));

diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc
index a5a38cc..57b5ba9 100644 (file)
--- a/src/x64/lithium-codegen-x64.cc
+++ b/src/x64/lithium-codegen-x64.cc
@@ -5772,12 +5772,7 @@ void LCodeGen::DoOsrEntry(LOsrEntry* instr) {
 
 void LCodeGen::DoForInPrepareMap(LForInPrepareMap* instr) {
   DCHECK(ToRegister(instr->context()).is(rsi));
-  __ CompareRoot(rax, Heap::kUndefinedValueRootIndex);
-  DeoptimizeIf(equal, instr, Deoptimizer::kUndefined);
 
-  Register null_value = rdi;
-  __ LoadRoot(null_value, Heap::kNullValueRootIndex);
-  __ cmpp(rax, null_value);
   DeoptimizeIf(equal, instr, Deoptimizer::kNull);
 
   Condition cc = masm()->CheckSmi(rax);
@@ -5788,6 +5783,8 @@ void LCodeGen::DoForInPrepareMap(LForInPrepareMap* instr) {
   DeoptimizeIf(below_equal, instr, Deoptimizer::kWrongInstanceType);
 
   Label use_cache, call_runtime;
+  Register null_value = rdi;
+  __ LoadRoot(null_value, Heap::kNullValueRootIndex);
   __ CheckEnumCache(null_value, &call_runtime);
 
   __ movp(rax, FieldOperand(rax, HeapObject::kMapOffset));

diff --git a/test/mjsunit/for-in-opt.js b/test/mjsunit/for-in-opt.js
new file mode 100644 (file)
index 0000000..67ef2d8
--- /dev/null
+++ b/test/mjsunit/for-in-opt.js
@@ -0,0 +1,86 @@
+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-proxies --allow-natives-syntax
+
+"use strict";
+
+// Test non-JSObject receiver.
+function f(o) {
+  var result = [];
+  for (var i in o) {
+    result.push(i);
+  }
+  return result;
+}
+
+assertEquals(["0"], f("a"));
+assertEquals(["0"], f("a"));
+%OptimizeFunctionOnNextCall(f);
+assertEquals(["0","1","2"], f("bla"));
+
+// Test the lazy deopt points.
+var keys = ["a", "b", "c", "d"];
+var has_keys = [];
+var deopt_has = false;
+var deopt_enum = false;
+
+var handler = {
+  enumerate: function(target) {
+    if (deopt_enum) {
+      %DeoptimizeFunction(f2);
+      deopt_enum = false;
+    }
+    return keys;
+  },
+
+  getPropertyDescriptor: function(k) {
+    if (deopt_has) {
+      %DeoptimizeFunction(f2);
+      deopt_has = false;
+    }
+    has_keys.push(k);
+    return {value: 10, configurable: true, writable: false, enumerable: true};
+  }
+};
+
+
+var proxy = Proxy.create(handler);
+var o = {__proto__: proxy};
+
+function f2(o) {
+  var result = [];
+  for (var i in o) {
+    result.push(i);
+  }
+  return result;
+}
+
+function check_f2() {
+  assertEquals(keys, f2(o));
+  assertEquals(keys, has_keys);
+  has_keys.length = 0;
+}
+
+check_f2();
+check_f2();
+// Test lazy deopt after GetPropertyNamesFast
+%OptimizeFunctionOnNextCall(f2);
+deopt_enum = true;
+check_f2();
+// Test lazy deopt after FILTER_KEY
+%OptimizeFunctionOnNextCall(f2);
+deopt_has = true;
+check_f2();
+
+function f3(o) {
+  for (var i in o) {
+  }
+}
+
+f3({__proto__:{x:1}});
+f3({__proto__:{x:1}});
+%OptimizeFunctionOnNextCall(f3);
+f3(undefined);
+f3(null);