From 1850803d56d6da94adf1f3ac779e5e9ea2388333 Mon Sep 17 00:00:00 2001 From: titzer Date: Tue, 21 Apr 2015 08:13:13 -0700 Subject: [PATCH] [turbofan] Fix reduction of LoadProperty/StoreProperty to LoadNamed/StoreNamed. R=mstarzinger@chromium.org BUG= Review URL: https://codereview.chromium.org/1095313002 Cr-Commit-Position: refs/heads/master@{#27972} --- src/compiler.cc | 1 + src/compiler/js-generic-lowering.cc | 12 +++++++++--- src/compiler/js-operator.cc | 10 ++++++---- src/compiler/js-operator.h | 30 +++++++++++++++++++++++------- src/compiler/js-type-feedback.cc | 12 ++++++++++-- test/mjsunit/compiler/named-load.js | 31 +++++++++++++++++++++++++++++++ test/mjsunit/compiler/named-store.js | 31 +++++++++++++++++++++++++++++++ 7 files changed, 111 insertions(+), 16 deletions(-) create mode 100644 test/mjsunit/compiler/named-load.js create mode 100644 test/mjsunit/compiler/named-store.js diff --git a/src/compiler.cc b/src/compiler.cc index 524eca8..58f47bc 100644 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -392,6 +392,7 @@ OptimizedCompileJob::Status OptimizedCompileJob::CreateGraph() { info()->MarkAsContextSpecializing(); } else if (FLAG_turbo_type_feedback) { info()->MarkAsTypeFeedbackEnabled(); + info()->EnsureFeedbackVector(); } Timer t(this, &time_taken_to_create_graph_); diff --git a/src/compiler/js-generic-lowering.cc b/src/compiler/js-generic-lowering.cc index a034512..ef419c2 100644 --- a/src/compiler/js-generic-lowering.cc +++ b/src/compiler/js-generic-lowering.cc @@ -320,8 +320,11 @@ void JSGenericLowering::LowerJSLoadProperty(Node* node) { void JSGenericLowering::LowerJSLoadNamed(Node* node) { const LoadNamedParameters& p = LoadNamedParametersOf(node->op()); - Callable callable = CodeFactory::LoadICInOptimizedCode( - isolate(), p.contextual_mode(), UNINITIALIZED); + Callable callable = + p.load_ic() == NAMED + ? CodeFactory::LoadICInOptimizedCode(isolate(), p.contextual_mode(), + UNINITIALIZED) + : CodeFactory::KeyedLoadICInOptimizedCode(isolate(), UNINITIALIZED); node->InsertInput(zone(), 1, jsgraph()->HeapConstant(p.name())); if (FLAG_vector_ics) { node->InsertInput(zone(), 2, jsgraph()->SmiConstant(p.feedback().index())); @@ -342,7 +345,10 @@ void JSGenericLowering::LowerJSStoreProperty(Node* node) { void JSGenericLowering::LowerJSStoreNamed(Node* node) { const StoreNamedParameters& p = StoreNamedParametersOf(node->op()); - Callable callable = CodeFactory::StoreIC(isolate(), p.language_mode()); + Callable callable = p.store_ic() == NAMED + ? CodeFactory::StoreIC(isolate(), p.language_mode()) + : CodeFactory::KeyedStoreICInOptimizedCode( + isolate(), p.language_mode(), UNINITIALIZED); node->InsertInput(zone(), 1, jsgraph()->HeapConstant(p.name())); ReplaceWithStubCall(node, callable, CallDescriptor::kPatchableCallSite); } diff --git a/src/compiler/js-operator.cc b/src/compiler/js-operator.cc index 1a38930..aeb317b 100644 --- a/src/compiler/js-operator.cc +++ b/src/compiler/js-operator.cc @@ -324,8 +324,9 @@ const Operator* JSOperatorBuilder::CallConstruct(int arguments) { const Operator* JSOperatorBuilder::LoadNamed(const Unique& name, const VectorSlotPair& feedback, - ContextualMode contextual_mode) { - LoadNamedParameters parameters(name, feedback, contextual_mode); + ContextualMode contextual_mode, + PropertyICMode load_ic) { + LoadNamedParameters parameters(name, feedback, contextual_mode, load_ic); return new (zone()) Operator1( // -- IrOpcode::kJSLoadNamed, Operator::kNoProperties, // opcode "JSLoadNamed", // name @@ -357,8 +358,9 @@ const Operator* JSOperatorBuilder::StoreProperty(LanguageMode language_mode) { const Operator* JSOperatorBuilder::StoreNamed(LanguageMode language_mode, - const Unique& name) { - StoreNamedParameters parameters(language_mode, name); + const Unique& name, + PropertyICMode store_ic) { + StoreNamedParameters parameters(language_mode, name, store_ic); return new (zone()) Operator1( // -- IrOpcode::kJSStoreNamed, Operator::kNoProperties, // opcode "JSStoreNamed", // name diff --git a/src/compiler/js-operator.h b/src/compiler/js-operator.h index c410338..401f1f0 100644 --- a/src/compiler/js-operator.h +++ b/src/compiler/js-operator.h @@ -115,23 +115,34 @@ class VectorSlotPair { bool operator==(VectorSlotPair const& lhs, VectorSlotPair const& rhs); +// For (Load|Store)Named operators, the mode of the IC that needs +// to be called. This is needed because (Load|Store)Property nodes can be +// reduced to named versions, but still need to call the correct original +// IC mode because of the layout of feedback vectors. +enum PropertyICMode { NAMED, KEYED }; + // Defines the property being loaded from an object by a named load. This is // used as a parameter by JSLoadNamed operators. class LoadNamedParameters final { public: LoadNamedParameters(const Unique& name, const VectorSlotPair& feedback, - ContextualMode contextual_mode) - : name_(name), contextual_mode_(contextual_mode), feedback_(feedback) {} + ContextualMode contextual_mode, PropertyICMode load_ic) + : name_(name), + feedback_(feedback), + contextual_mode_(contextual_mode), + load_ic_(load_ic) {} const Unique& name() const { return name_; } ContextualMode contextual_mode() const { return contextual_mode_; } + PropertyICMode load_ic() const { return load_ic_; } const VectorSlotPair& feedback() const { return feedback_; } private: const Unique name_; - const ContextualMode contextual_mode_; const VectorSlotPair feedback_; + const ContextualMode contextual_mode_; + const PropertyICMode load_ic_; }; bool operator==(LoadNamedParameters const&, LoadNamedParameters const&); @@ -171,15 +182,18 @@ const LoadPropertyParameters& LoadPropertyParametersOf(const Operator* op); // used as a parameter by JSStoreNamed operators. class StoreNamedParameters final { public: - StoreNamedParameters(LanguageMode language_mode, const Unique& name) - : language_mode_(language_mode), name_(name) {} + StoreNamedParameters(LanguageMode language_mode, const Unique& name, + PropertyICMode store_ic) + : language_mode_(language_mode), name_(name), store_ic_(store_ic) {} LanguageMode language_mode() const { return language_mode_; } const Unique& name() const { return name_; } + PropertyICMode store_ic() const { return store_ic_; } private: const LanguageMode language_mode_; const Unique name_; + const PropertyICMode store_ic_; }; bool operator==(StoreNamedParameters const&, StoreNamedParameters const&); @@ -237,11 +251,13 @@ class JSOperatorBuilder final : public ZoneObject { const Operator* LoadProperty(const VectorSlotPair& feedback); const Operator* LoadNamed(const Unique& name, const VectorSlotPair& feedback, - ContextualMode contextual_mode = NOT_CONTEXTUAL); + ContextualMode contextual_mode = NOT_CONTEXTUAL, + PropertyICMode load_ic = NAMED); const Operator* StoreProperty(LanguageMode language_mode); const Operator* StoreNamed(LanguageMode language_mode, - const Unique& name); + const Unique& name, + PropertyICMode store_ic = NAMED); const Operator* DeleteProperty(LanguageMode language_mode); diff --git a/src/compiler/js-type-feedback.cc b/src/compiler/js-type-feedback.cc index d93dc5c..a9e0e22 100644 --- a/src/compiler/js-type-feedback.cc +++ b/src/compiler/js-type-feedback.cc @@ -14,6 +14,7 @@ #include "src/compiler/common-operator.h" #include "src/compiler/node-aux-data.h" #include "src/compiler/node-matchers.h" +#include "src/compiler/operator-properties.h" #include "src/compiler/simplified-operator.h" namespace v8 { @@ -41,7 +42,8 @@ Reduction JSTypeFeedbackSpecializer::Reduce(Node* node) { Unique name = match.Value(); const VectorSlotPair& feedback = LoadPropertyParametersOf(node->op()).feedback(); - node->set_op(jsgraph()->javascript()->LoadNamed(name, feedback)); + node->set_op(jsgraph()->javascript()->LoadNamed(name, feedback, + NOT_CONTEXTUAL, KEYED)); node->RemoveInput(1); return ReduceJSLoadNamed(node); } @@ -57,7 +59,13 @@ Reduction JSTypeFeedbackSpecializer::Reduce(Node* node) { // StoreProperty(o, "constant", v) => StoreNamed["constant"](o, v). Unique name = match.Value(); LanguageMode language_mode = OpParameter(node); - node->set_op(jsgraph()->javascript()->StoreNamed(language_mode, name)); + if (FLAG_turbo_deoptimization) { + // StoreProperty has 2 frame state inputs, but StoreNamed only 1. + DCHECK_EQ(2, OperatorProperties::GetFrameStateInputCount(node->op())); + node->RemoveInput(NodeProperties::FirstFrameStateIndex(node) + 1); + } + node->set_op( + jsgraph()->javascript()->StoreNamed(language_mode, name, KEYED)); node->RemoveInput(1); return ReduceJSStoreNamed(node); } diff --git a/test/mjsunit/compiler/named-load.js b/test/mjsunit/compiler/named-load.js new file mode 100644 index 0000000..3527b83 --- /dev/null +++ b/test/mjsunit/compiler/named-load.js @@ -0,0 +1,31 @@ +// 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. + +function Foo(a, b) { + this.a = a; + this.b = b; + var bname = "b"; + this.x = this["a"] + this[bname]; +} + +var f1 = new Foo(3, 4); +assertEquals(7, f1.x); + +// SMIs +for (var i = 0; i < 6; i++) { + var f = new Foo(i, i + 2); + assertEquals(i + i + 2, f.x); +} + +// derbles +for (var i = 0.25; i < 6.25; i++) { + var f = new Foo(i, i + 2); + assertEquals(i + i + 2, f.x); +} + +// stirngs +for (var i = 0; i < 6; i++) { + var f = new Foo(i + "", (i + 2) + ""); + assertEquals((i + "") + ((i + 2) + ""), f.x); +} diff --git a/test/mjsunit/compiler/named-store.js b/test/mjsunit/compiler/named-store.js new file mode 100644 index 0000000..8d1306a --- /dev/null +++ b/test/mjsunit/compiler/named-store.js @@ -0,0 +1,31 @@ +// 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. + +function Foo(a, b) { + var bname = "b"; + this["a"] = a; + this[bname] = b; + this.x = this.a + this.b; +} + +var f1 = new Foo(3, 4); +assertEquals(7, f1.x); + +// SMIs +for (var i = 0; i < 6; i++) { + var f = new Foo(i, i + 2); + assertEquals(i + i + 2, f.x); +} + +// derbles +for (var i = 0.25; i < 6.25; i++) { + var f = new Foo(i, i + 2); + assertEquals(i + i + 2, f.x); +} + +// stirngs +for (var i = 0; i < 6; i++) { + var f = new Foo(i + "", (i + 2) + ""); + assertEquals((i + "") + ((i + 2) + ""), f.x); +} -- 2.7.4