Remove dependency from generic lowering on compilation info for determining strictnes...

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

diff --git a/src/compiler.h b/src/compiler.h
index 608b4a5..82c0b87 100644 (file)
--- 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<JSFunction> 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 (file)
--- 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> 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> 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> 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> 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<Declaration*>* 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> 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 (file)
--- 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<StrictMode>(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<Name> key = OpParameter<PrintableUnique<Name> >(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<StoreNamedParameters>(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 (file)
--- 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> 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> name) {
-    OP1(JSStoreNamed, PrintableUnique<Name>, 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> 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 (file)
--- a/src/compiler/pipeline.cc
+++ b/src/compiler/pipeline.cc
@@ -200,7 +200,7 @@ Handle<Code> 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 (file)
--- 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<Code>(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<Object> 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 (file)
--- 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 (file)
--- 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<v8::Context> context = v8::Context::New(CcTest::isolate());
   v8::Context::Scope scope(context);
   v8::Local<v8::Value> value = CompileRun(script);
@@ -276,7 +276,7 @@ TEST(BuiltinLoadedFromActivation) {
       "})()";
 
   // Disable context specialization.
-  FunctionTester T(script, false);
+  FunctionTester T(script);
   v8::Local<v8::Context> context = v8::Context::New(CcTest::isolate());
   v8::Context::Scope scope(context);
   v8::Local<v8::Value> value = CompileRun(script);