Revision: 23479
Author:   sigu...@chromium.org
Date:     Thu Aug 28 08:39:24 2014 UTC
Log: 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=mstarzin...@chromium.org, tit...@chromium.org

Review URL: https://codereview.chromium.org/473263004
https://code.google.com/p/v8/source/detail?r=23479

Modified:
 /branches/bleeding_edge/src/compiler/ast-graph-builder.cc
 /branches/bleeding_edge/src/compiler/js-generic-lowering.cc
 /branches/bleeding_edge/src/compiler/js-operator.h
 /branches/bleeding_edge/src/compiler/pipeline.cc
 /branches/bleeding_edge/src/compiler.h
 /branches/bleeding_edge/test/cctest/compiler/function-tester.h
 /branches/bleeding_edge/test/cctest/compiler/test-run-inlining.cc
 /branches/bleeding_edge/test/cctest/compiler/test-run-jscalls.cc

=======================================
--- /branches/bleeding_edge/src/compiler/ast-graph-builder.cc Mon Aug 25 16:32:35 2014 UTC +++ /branches/bleeding_edge/src/compiler/ast-graph-builder.cc Thu Aug 28 08:39:24 2014 UTC
@@ -898,8 +898,8 @@
             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 @@
     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 @@
       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 @@
       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 @@
       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 @@
       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 @@
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 @@
       // 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;
=======================================
--- /branches/bleeding_edge/src/compiler/js-generic-lowering.cc Tue Aug 26 09:50:09 2014 UTC +++ /branches/bleeding_edge/src/compiler/js-generic-lowering.cc Thu Aug 28 08:39:24 2014 UTC
@@ -422,9 +422,7 @@


 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::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;
 }
=======================================
--- /branches/bleeding_edge/src/compiler/js-operator.h Mon Aug 11 12:26:17 2014 UTC +++ /branches/bleeding_edge/src/compiler/js-operator.h Thu Aug 28 08:39:24 2014 UTC
@@ -51,6 +51,13 @@
   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,11 +131,16 @@
         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,
=======================================
--- /branches/bleeding_edge/src/compiler/pipeline.cc Wed Aug 27 09:52:32 2014 UTC +++ /branches/bleeding_edge/src/compiler/pipeline.cc Thu Aug 28 08:39:24 2014 UTC
@@ -200,7 +200,7 @@
     VerifyAndPrintGraph(&graph, "Context specialized");
   }

-  if (FLAG_turbo_inlining) {
+  if (info()->is_inlining_enabled()) {
     SourcePositionTable::Scope pos(&source_positions,
                                    SourcePosition::Unknown());
     JSInliner inliner(info(), &jsgraph);
=======================================
--- /branches/bleeding_edge/src/compiler.h      Tue Aug 26 15:17:57 2014 UTC
+++ /branches/bleeding_edge/src/compiler.h      Thu Aug 28 08:39:24 2014 UTC
@@ -80,7 +80,8 @@
     kCompilingForDebugging = 1 << 13,
     kParseRestriction = 1 << 14,
     kSerializing = 1 << 15,
-    kContextSpecializing = 1 << 16
+    kContextSpecializing = 1 << 16,
+    kInliningEnabled = 1 << 17
   };

   CompilationInfo(Handle<JSFunction> closure, Zone* zone);
@@ -185,6 +186,10 @@
   void MarkAsContextSpecializing() { SetFlag(kContextSpecializing); }

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() &&
=======================================
--- /branches/bleeding_edge/test/cctest/compiler/function-tester.h Tue Aug 26 15:17:57 2014 UTC +++ /branches/bleeding_edge/test/cctest/compiler/function-tester.h Thu Aug 28 08:39:24 2014 UTC
@@ -25,13 +25,16 @@
 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 @@
     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 @@
Handle<Object> false_value() { return isolate->factory()->false_value(); }

  private:
-  bool context_specialization_;
+  uint32_t flags_;
 };
 }
 }
=======================================
--- /branches/bleeding_edge/test/cctest/compiler/test-run-inlining.cc Mon Aug 25 13:38:10 2014 UTC +++ /branches/bleeding_edge/test/cctest/compiler/test-run-inlining.cc Thu Aug 28 08:39:24 2014 UTC
@@ -36,13 +36,13 @@


 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(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(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 @@
 // 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(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(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(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(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(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(InlineTwiceDependentDiamondReal) {
-  FLAG_context_specialization = true;
-  FLAG_turbo_inlining = true;
   FunctionTester T(
       "(function () {"
       "var x = 41;"
@@ -185,7 +183,9 @@
       "                  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));
=======================================
--- /branches/bleeding_edge/test/cctest/compiler/test-run-jscalls.cc Thu Aug 21 12:40:10 2014 UTC +++ /branches/bleeding_edge/test/cctest/compiler/test-run-jscalls.cc Thu Aug 28 08:39:24 2014 UTC
@@ -255,7 +255,7 @@
       "})()";

   // 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 @@
       "})()";

   // 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);

--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to