From e7817a9e544e38f0b11d59608fcbd9e0706f3c2c Mon Sep 17 00:00:00 2001 From: "sigurds@chromium.org" Date: Thu, 28 Aug 2014 08:39:24 +0000 Subject: [PATCH] Remove dependency from generic lowering on compilation info for determining strictness and builtins. This makes the graphs compositional for inlining (i.e. we can now inline a strict function into a non-strict function, or vice versa). 1) Store strict mode as parameter in StoreNamed/StoreProperty. R=mstarzinger@chromium.org, titzer@chromium.org Review URL: https://codereview.chromium.org/473263004 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@23479 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/compiler.h | 7 +++- src/compiler/ast-graph-builder.cc | 29 +++++++++------ src/compiler/js-generic-lowering.cc | 13 +++---- src/compiler/js-operator.h | 18 ++++++++-- src/compiler/pipeline.cc | 2 +- test/cctest/compiler/function-tester.h | 18 +++++++--- test/cctest/compiler/test-run-inlining.cc | 60 +++++++++++++++---------------- test/cctest/compiler/test-run-jscalls.cc | 4 +-- 8 files changed, 89 insertions(+), 62 deletions(-) diff --git a/src/compiler.h b/src/compiler.h index 608b4a5..82c0b87 100644 --- a/src/compiler.h +++ b/src/compiler.h @@ -80,7 +80,8 @@ class CompilationInfo { kCompilingForDebugging = 1 << 13, kParseRestriction = 1 << 14, kSerializing = 1 << 15, - kContextSpecializing = 1 << 16 + kContextSpecializing = 1 << 16, + kInliningEnabled = 1 << 17 }; CompilationInfo(Handle closure, Zone* zone); @@ -186,6 +187,10 @@ class CompilationInfo { bool is_context_specializing() const { return GetFlag(kContextSpecializing); } + void MarkAsInliningEnabled() { SetFlag(kInliningEnabled); } + + bool is_inlining_enabled() const { return GetFlag(kInliningEnabled); } + bool IsCodePreAgingActive() const { return FLAG_optimize_for_size && FLAG_age_code && !will_serialize() && !is_debug(); diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc index 0e642c9..9ec91e5 100644 --- a/src/compiler/ast-graph-builder.cc +++ b/src/compiler/ast-graph-builder.cc @@ -898,8 +898,8 @@ void AstGraphBuilder::VisitObjectLiteral(ObjectLiteral* expr) { VisitForValue(property->value()); Node* value = environment()->Pop(); PrintableUnique name = MakeUnique(key->AsPropertyName()); - Node* store = - NewNode(javascript()->StoreNamed(name), literal, value); + Node* store = NewNode(javascript()->StoreNamed(strict_mode(), name), + literal, value); PrepareFrameState(store, key->id()); } else { VisitForEffect(property->value()); @@ -992,7 +992,8 @@ void AstGraphBuilder::VisitArrayLiteral(ArrayLiteral* expr) { VisitForValue(subexpr); Node* value = environment()->Pop(); Node* index = jsgraph()->Constant(i); - Node* store = NewNode(javascript()->StoreProperty(), literal, index, value); + Node* store = NewNode(javascript()->StoreProperty(strict_mode()), literal, + index, value); PrepareFrameState(store, expr->GetIdForElement(i)); } @@ -1023,7 +1024,8 @@ void AstGraphBuilder::VisitForInAssignment(Expression* expr, Node* value) { value = environment()->Pop(); PrintableUnique name = MakeUnique(property->key()->AsLiteral()->AsPropertyName()); - Node* store = NewNode(javascript()->StoreNamed(name), object, value); + Node* store = + NewNode(javascript()->StoreNamed(strict_mode(), name), object, value); // TODO(jarin) Fill in the correct bailout id. PrepareFrameState(store, BailoutId::None()); break; @@ -1035,7 +1037,8 @@ void AstGraphBuilder::VisitForInAssignment(Expression* expr, Node* value) { Node* key = environment()->Pop(); Node* object = environment()->Pop(); value = environment()->Pop(); - Node* store = NewNode(javascript()->StoreProperty(), object, key, value); + Node* store = NewNode(javascript()->StoreProperty(strict_mode()), object, + key, value); // TODO(jarin) Fill in the correct bailout id. PrepareFrameState(store, BailoutId::None()); break; @@ -1116,14 +1119,16 @@ void AstGraphBuilder::VisitAssignment(Assignment* expr) { Node* object = environment()->Pop(); PrintableUnique name = MakeUnique(property->key()->AsLiteral()->AsPropertyName()); - Node* store = NewNode(javascript()->StoreNamed(name), object, value); + Node* store = + NewNode(javascript()->StoreNamed(strict_mode(), name), object, value); PrepareFrameState(store, expr->AssignmentId()); break; } case KEYED_PROPERTY: { Node* key = environment()->Pop(); Node* object = environment()->Pop(); - Node* store = NewNode(javascript()->StoreProperty(), object, key, value); + Node* store = NewNode(javascript()->StoreProperty(strict_mode()), object, + key, value); PrepareFrameState(store, expr->AssignmentId()); break; } @@ -1421,14 +1426,16 @@ void AstGraphBuilder::VisitCountOperation(CountOperation* expr) { Node* object = environment()->Pop(); PrintableUnique name = MakeUnique(property->key()->AsLiteral()->AsPropertyName()); - Node* store = NewNode(javascript()->StoreNamed(name), object, value); + Node* store = + NewNode(javascript()->StoreNamed(strict_mode(), name), object, value); PrepareFrameState(store, expr->AssignmentId()); break; } case KEYED_PROPERTY: { Node* key = environment()->Pop(); Node* object = environment()->Pop(); - Node* store = NewNode(javascript()->StoreProperty(), object, key, value); + Node* store = NewNode(javascript()->StoreProperty(strict_mode()), object, + key, value); PrepareFrameState(store, expr->AssignmentId()); break; } @@ -1531,7 +1538,7 @@ void AstGraphBuilder::VisitDeclarations(ZoneList* declarations) { for (int i = 0; i < globals()->length(); ++i) data->set(i, *globals()->at(i)); int encoded_flags = DeclareGlobalsEvalFlag::encode(info()->is_eval()) | DeclareGlobalsNativeFlag::encode(info()->is_native()) | - DeclareGlobalsStrictMode::encode(info()->strict_mode()); + DeclareGlobalsStrictMode::encode(strict_mode()); Node* flags = jsgraph()->Constant(encoded_flags); Node* pairs = jsgraph()->Constant(data); Operator* op = javascript()->Runtime(Runtime::kDeclareGlobals, 3); @@ -1838,7 +1845,7 @@ Node* AstGraphBuilder::BuildVariableAssignment(Variable* variable, Node* value, // Global var, const, or let variable. Node* global = BuildLoadGlobalObject(); PrintableUnique name = MakeUnique(variable->name()); - Operator* op = javascript()->StoreNamed(name); + Operator* op = javascript()->StoreNamed(strict_mode(), name); Node* store = NewNode(op, global, value); PrepareFrameState(store, bailout_id); return store; diff --git a/src/compiler/js-generic-lowering.cc b/src/compiler/js-generic-lowering.cc index f9ac3ee..f4be6e7 100644 --- a/src/compiler/js-generic-lowering.cc +++ b/src/compiler/js-generic-lowering.cc @@ -422,9 +422,7 @@ Node* JSGenericLowering::LowerJSLoadNamed(Node* node) { Node* JSGenericLowering::LowerJSStoreProperty(Node* node) { - // TODO(mstarzinger): The strict_mode needs to be carried along in the - // operator so that graphs are fully compositional for inlining. - StrictMode strict_mode = info()->strict_mode(); + StrictMode strict_mode = OpParameter(node); KeyedStoreICStubShim stub(isolate(), strict_mode); ReplaceWithICStubCall(node, &stub); return node; @@ -432,12 +430,9 @@ Node* JSGenericLowering::LowerJSStoreProperty(Node* node) { Node* JSGenericLowering::LowerJSStoreNamed(Node* node) { - PrintableUnique key = OpParameter >(node); - // TODO(mstarzinger): The strict_mode needs to be carried along in the - // operator so that graphs are fully compositional for inlining. - StrictMode strict_mode = info()->strict_mode(); - StoreICStubShim stub(isolate(), strict_mode); - PatchInsertInput(node, 1, jsgraph()->HeapConstant(key)); + StoreNamedParameters params = OpParameter(node); + StoreICStubShim stub(isolate(), params.strict_mode); + PatchInsertInput(node, 1, jsgraph()->HeapConstant(params.name)); ReplaceWithICStubCall(node, &stub); return node; } diff --git a/src/compiler/js-operator.h b/src/compiler/js-operator.h index fd9547d..9f1e2ce 100644 --- a/src/compiler/js-operator.h +++ b/src/compiler/js-operator.h @@ -51,6 +51,13 @@ struct CallParameters { CallFunctionFlags flags; }; +// Defines the property being stored to an object by a named store. This is +// used as a parameter by JSStoreNamed operators. +struct StoreNamedParameters { + StrictMode strict_mode; + PrintableUnique name; +}; + // Interface for building JavaScript-level operators, e.g. directly from the // AST. Most operators have no parameters, thus can be globally shared for all // graphs. @@ -124,12 +131,17 @@ class JSOperatorBuilder { 1, 1); } - Operator* StoreProperty() { NOPROPS(JSStoreProperty, 3, 0); } - Operator* StoreNamed(PrintableUnique name) { - OP1(JSStoreNamed, PrintableUnique, name, Operator::kNoProperties, 2, + Operator* StoreProperty(StrictMode strict_mode) { + OP1(JSStoreProperty, StrictMode, strict_mode, Operator::kNoProperties, 3, 0); } + Operator* StoreNamed(StrictMode strict_mode, PrintableUnique name) { + StoreNamedParameters parameters = {strict_mode, name}; + OP1(JSStoreNamed, StoreNamedParameters, parameters, Operator::kNoProperties, + 2, 0); + } + Operator* DeleteProperty(StrictMode strict_mode) { OP1(JSDeleteProperty, StrictMode, strict_mode, Operator::kNoProperties, 2, 1); diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc index 7937daf..250999c 100644 --- a/src/compiler/pipeline.cc +++ b/src/compiler/pipeline.cc @@ -200,7 +200,7 @@ Handle Pipeline::GenerateCode() { VerifyAndPrintGraph(&graph, "Context specialized"); } - if (FLAG_turbo_inlining) { + if (info()->is_inlining_enabled()) { SourcePositionTable::Scope pos(&source_positions, SourcePosition::Unknown()); JSInliner inliner(info(), &jsgraph); diff --git a/test/cctest/compiler/function-tester.h b/test/cctest/compiler/function-tester.h index 4a416a4..f2b763c 100644 --- a/test/cctest/compiler/function-tester.h +++ b/test/cctest/compiler/function-tester.h @@ -25,13 +25,16 @@ namespace internal { namespace compiler { class FunctionTester : public InitializedHandleScope { + const uint32_t supported_flags = + CompilationInfo::kContextSpecializing | CompilationInfo::kInliningEnabled; + public: - explicit FunctionTester(const char* source, bool context_specialization = - FLAG_context_specialization) + explicit FunctionTester(const char* source, uint32_t flags = 0) : isolate(main_isolate()), function((FLAG_allow_natives_syntax = true, NewFunction(source))), - context_specialization_(context_specialization) { + flags_(flags) { Compile(function); + CHECK_EQ(0, flags_ & ~supported_flags); } Isolate* isolate; @@ -45,7 +48,12 @@ class FunctionTester : public InitializedHandleScope { StrictMode strict_mode = info.function()->strict_mode(); info.SetStrictMode(strict_mode); info.SetOptimizing(BailoutId::None(), Handle(function->code())); - if (context_specialization_) info.MarkAsContextSpecializing(); + if (flags_ & CompilationInfo::kContextSpecializing) { + info.MarkAsContextSpecializing(); + } + if (flags_ & CompilationInfo::kInliningEnabled) { + info.MarkAsInliningEnabled(); + } CHECK(Rewriter::Rewrite(&info)); CHECK(Scope::Analyze(&info)); CHECK_NE(NULL, info.scope()); @@ -193,7 +201,7 @@ class FunctionTester : public InitializedHandleScope { Handle false_value() { return isolate->factory()->false_value(); } private: - bool context_specialization_; + uint32_t flags_; }; } } diff --git a/test/cctest/compiler/test-run-inlining.cc b/test/cctest/compiler/test-run-inlining.cc index 5e5b604..c4e3aea 100644 --- a/test/cctest/compiler/test-run-inlining.cc +++ b/test/cctest/compiler/test-run-inlining.cc @@ -36,13 +36,13 @@ static void InstallAssertStackDepthHelper(v8::Isolate* isolate) { TEST(SimpleInlining) { - FLAG_context_specialization = true; - FLAG_turbo_inlining = true; FunctionTester T( "(function(){" "function foo(s) { AssertStackDepth(1); return s; };" "function bar(s, t) { return foo(s); };" - "return bar;})();"); + "return bar;})();", + CompilationInfo::kInliningEnabled | + CompilationInfo::kContextSpecializing); InstallAssertStackDepthHelper(CcTest::isolate()); T.CheckCall(T.Val(1), T.Val(1), T.Val(2)); @@ -50,14 +50,14 @@ TEST(SimpleInlining) { TEST(SimpleInliningContext) { - FLAG_context_specialization = true; - FLAG_turbo_inlining = true; FunctionTester T( "(function () {" "function foo(s) { AssertStackDepth(1); var x = 12; return s + x; };" "function bar(s, t) { return foo(s); };" "return bar;" - "})();"); + "})();", + CompilationInfo::kInliningEnabled | + CompilationInfo::kContextSpecializing); InstallAssertStackDepthHelper(CcTest::isolate()); T.CheckCall(T.Val(13), T.Val(1), T.Val(2)); @@ -65,15 +65,15 @@ TEST(SimpleInliningContext) { TEST(CaptureContext) { - FLAG_context_specialization = true; - FLAG_turbo_inlining = true; FunctionTester T( "var f = (function () {" "var x = 42;" "function bar(s) { return x + s; };" "return (function (s) { return bar(s); });" "})();" - "(function (s) { return f(s)})"); + "(function (s) { return f(s)})", + CompilationInfo::kInliningEnabled | + CompilationInfo::kContextSpecializing); InstallAssertStackDepthHelper(CcTest::isolate()); T.CheckCall(T.Val(42 + 12), T.Val(12), T.undefined()); @@ -83,14 +83,14 @@ TEST(CaptureContext) { // TODO(sigurds) For now we do not inline any native functions. If we do at // some point, change this test. TEST(DontInlineEval) { - FLAG_context_specialization = true; - FLAG_turbo_inlining = true; FunctionTester T( "var x = 42;" "(function () {" "function bar(s, t) { return eval(\"AssertStackDepth(2); x\") };" "return bar;" - "})();"); + "})();", + CompilationInfo::kInliningEnabled | + CompilationInfo::kContextSpecializing); InstallAssertStackDepthHelper(CcTest::isolate()); T.CheckCall(T.Val(42), T.Val("x"), T.undefined()); @@ -98,14 +98,14 @@ TEST(DontInlineEval) { TEST(InlineOmitArguments) { - FLAG_context_specialization = true; - FLAG_turbo_inlining = true; FunctionTester T( "(function () {" "var x = 42;" "function bar(s, t, u, v) { AssertStackDepth(1); return x + s; };" "return (function (s,t) { return bar(s); });" - "})();"); + "})();", + CompilationInfo::kInliningEnabled | + CompilationInfo::kContextSpecializing); InstallAssertStackDepthHelper(CcTest::isolate()); T.CheckCall(T.Val(42 + 12), T.Val(12), T.undefined()); @@ -113,15 +113,15 @@ TEST(InlineOmitArguments) { TEST(InlineSurplusArguments) { - FLAG_context_specialization = true; - FLAG_turbo_inlining = true; FunctionTester T( "(function () {" "var x = 42;" "function foo(s) { AssertStackDepth(1); return x + s; };" "function bar(s,t) { return foo(s,t,13); };" "return bar;" - "})();"); + "})();", + CompilationInfo::kInliningEnabled | + CompilationInfo::kContextSpecializing); InstallAssertStackDepthHelper(CcTest::isolate()); T.CheckCall(T.Val(42 + 12), T.Val(12), T.undefined()); @@ -129,14 +129,14 @@ TEST(InlineSurplusArguments) { TEST(InlineTwice) { - FLAG_context_specialization = true; - FLAG_turbo_inlining = true; FunctionTester T( "(function () {" "var x = 42;" "function bar(s) { AssertStackDepth(1); return x + s; };" "return (function (s,t) { return bar(s) + bar(t); });" - "})();"); + "})();", + CompilationInfo::kInliningEnabled | + CompilationInfo::kContextSpecializing); InstallAssertStackDepthHelper(CcTest::isolate()); T.CheckCall(T.Val(2 * 42 + 12 + 4), T.Val(12), T.Val(4)); @@ -144,15 +144,15 @@ TEST(InlineTwice) { TEST(InlineTwiceDependent) { - FLAG_context_specialization = true; - FLAG_turbo_inlining = true; FunctionTester T( "(function () {" "var x = 42;" "function foo(s) { AssertStackDepth(1); return x + s; };" "function bar(s,t) { return foo(foo(s)); };" "return bar;" - "})();"); + "})();", + CompilationInfo::kInliningEnabled | + CompilationInfo::kContextSpecializing); InstallAssertStackDepthHelper(CcTest::isolate()); T.CheckCall(T.Val(42 + 42 + 12), T.Val(12), T.Val(4)); @@ -160,15 +160,15 @@ TEST(InlineTwiceDependent) { TEST(InlineTwiceDependentDiamond) { - FLAG_context_specialization = true; - FLAG_turbo_inlining = true; FunctionTester T( "(function () {" "function foo(s) { if (true) {" " return 12 } else { return 13; } };" "function bar(s,t) { return foo(foo(1)); };" "return bar;" - "})();"); + "})();", + CompilationInfo::kInliningEnabled | + CompilationInfo::kContextSpecializing); InstallAssertStackDepthHelper(CcTest::isolate()); T.CheckCall(T.Val(12), T.undefined(), T.undefined()); @@ -176,8 +176,6 @@ TEST(InlineTwiceDependentDiamond) { TEST(InlineTwiceDependentDiamondReal) { - FLAG_context_specialization = true; - FLAG_turbo_inlining = true; FunctionTester T( "(function () {" "var x = 41;" @@ -185,7 +183,9 @@ TEST(InlineTwiceDependentDiamondReal) { " return x - s } else { return x + s; } };" "function bar(s,t) { return foo(foo(s)); };" "return bar;" - "})();"); + "})();", + CompilationInfo::kInliningEnabled | + CompilationInfo::kContextSpecializing); InstallAssertStackDepthHelper(CcTest::isolate()); T.CheckCall(T.Val(-11), T.Val(11), T.Val(4)); diff --git a/test/cctest/compiler/test-run-jscalls.cc b/test/cctest/compiler/test-run-jscalls.cc index 38b9624..dec7194 100644 --- a/test/cctest/compiler/test-run-jscalls.cc +++ b/test/cctest/compiler/test-run-jscalls.cc @@ -255,7 +255,7 @@ TEST(ContextLoadedFromActivation) { "})()"; // Disable context specialization. - FunctionTester T(script, false); + FunctionTester T(script); v8::Local context = v8::Context::New(CcTest::isolate()); v8::Context::Scope scope(context); v8::Local value = CompileRun(script); @@ -276,7 +276,7 @@ TEST(BuiltinLoadedFromActivation) { "})()"; // Disable context specialization. - FunctionTester T(script, false); + FunctionTester T(script); v8::Local context = v8::Context::New(CcTest::isolate()); v8::Context::Scope scope(context); v8::Local value = CompileRun(script); -- 2.7.4