From b054ff46203cc86a2e180f2bb5b45a7106810e9b Mon Sep 17 00:00:00 2001 From: adamk Date: Wed, 15 Apr 2015 14:28:30 -0700 Subject: [PATCH] Revert "Add basic crankshaft support for slow-mode for-in to avoid disabling optimizations" This reverts commit 8c98cc074ef8278ce1c4dcd4790e8aaf6fbeedc6 because it causes flaky failures in the dromaeo.jslibeventprototype benchmark on Linux/Windows and consistent failures on Android. Also reverts the followup "Remove kForInStatementIsNotFastCase bailout reason" (commit ba24e6769615d0ea7f7b5a31c5947769892f93a7) to avoid breaking the build. BUG=chromium:476592 TBR=verwaest@chromium.org LOG=y Review URL: https://codereview.chromium.org/1066663005 Cr-Commit-Position: refs/heads/master@{#27859} --- src/bailout-reason.h | 1 + src/hydrogen.cc | 74 +++++++++++-------------------------- test/mjsunit/for-in-opt.js | 75 -------------------------------------- 3 files changed, 23 insertions(+), 127 deletions(-) delete mode 100644 test/mjsunit/for-in-opt.js diff --git a/src/bailout-reason.h b/src/bailout-reason.h index 09c889f9dd..e48d5243b9 100644 --- a/src/bailout-reason.h +++ b/src/bailout-reason.h @@ -94,6 +94,7 @@ 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/hydrogen.cc b/src/hydrogen.cc index b7130b434e..393e02c9f9 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -5051,6 +5051,10 @@ 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); @@ -5061,42 +5065,13 @@ void HOptimizedGraphBuilder::VisitForInStatement(ForInStatement* stmt) { CHECK_ALIVE(VisitForValue(stmt->enumerable())); HValue* enumerable = Top(); // Leave enumerable at the top. - HInstruction* map; - HInstruction* array; - HInstruction* enum_length; - bool fast = stmt->for_in_type() == ForInStatement::FAST_FOR_IN; - if (fast) { - map = Add(enumerable); - Add(stmt->PrepareId()); + HInstruction* map = Add(enumerable); + Add(stmt->PrepareId()); - array = Add(enumerable, map, - DescriptorArray::kEnumCacheBridgeCacheIndex); - enum_length = Add(map); + HInstruction* array = Add( + enumerable, map, DescriptorArray::kEnumCacheBridgeCacheIndex); - HInstruction* index_cache = Add( - enumerable, map, DescriptorArray::kEnumCacheBridgeIndicesCacheIndex); - HForInCacheArray::cast(array) - ->set_index_cache(HForInCacheArray::cast(index_cache)); - } else { - Add(stmt->PrepareId()); - { - NoObservableSideEffectsScope no_effects(this); - BuildJSObjectCheck(enumerable, 0); - } - Add(stmt->ToObjectId()); - - map = graph()->GetConstant1(); - Runtime::FunctionId function_id = Runtime::kGetPropertyNamesFast; - Add(enumerable); - array = Add(isolate()->factory()->empty_string(), - Runtime::FunctionForId(function_id), 1); - Push(array); - Add(stmt->EnumId()); - Drop(1); - Handle array_map = isolate()->factory()->fixed_array_map(); - HValue* check = Add(array, array_map); - enum_length = AddLoadFixedArrayLength(array, check); - } + HInstruction* enum_length = Add(map); HInstruction* start_index = Add(0); @@ -5105,12 +5080,13 @@ void HOptimizedGraphBuilder::VisitForInStatement(ForInStatement* stmt) { Push(enum_length); Push(start_index); + HInstruction* index_cache = Add( + 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); @@ -5134,21 +5110,15 @@ void HOptimizedGraphBuilder::VisitForInStatement(ForInStatement* stmt) { HValue* key = Add(environment()->ExpressionStackAt(2), // Enum cache. - index, index, FAST_ELEMENTS); + environment()->ExpressionStackAt(0), // Iteration index. + environment()->ExpressionStackAt(0), FAST_ELEMENTS); - if (fast) { - // Check if the expected map still matches that of the enumerable. - // If not just deoptimize. - Add(enumerable, environment()->ExpressionStackAt(3)); - Bind(each_var, key); - } else { - HValue* function = AddLoadJSBuiltin(Builtins::FILTER_KEY); - Add(enumerable, key); - key = Add(function, 2); - Bind(each_var, key); - Add(stmt->AssignmentId()); - Add(key); - } + // Check if the expected map still matches that of the enumerable. + // If not just deoptimize. + Add(environment()->ExpressionStackAt(4), + environment()->ExpressionStackAt(3)); + + Bind(each_var, key); BreakAndContinueInfo break_info(stmt, scope(), 5); { diff --git a/test/mjsunit/for-in-opt.js b/test/mjsunit/for-in-opt.js deleted file mode 100644 index 58344e4dc1..0000000000 --- a/test/mjsunit/for-in-opt.js +++ /dev/null @@ -1,75 +0,0 @@ -// 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();