[turbofan] Fix various issues with --turbo-inlining enabled.

This is in preparation to enabling --turbo-inlining by default, fixing
various issues when general purpose inlining is running against our
entire test suite.

R=bmeurer@chromium.org
BUG=v8:4493
LOG=n

Review URL: https://codereview.chromium.org/1407533004

Cr-Commit-Position: refs/heads/master@{#31294}

src/compiler/js-inlining-heuristic.cc

@@ -56,8 +56,11 @@ Reduction JSInliningHeuristic::Reduce(Node* node) {
 
   // Gather feedback on how often this call site has been hit before.
   CallFunctionParameters p = CallFunctionParametersOf(node->op());
-  CallICNexus nexus(p.feedback().vector(), p.feedback().slot());
-  int calls = nexus.ExtractCallCount();
+  int calls = -1;  // Same default as CallICNexus::ExtractCallCount.
+  if (p.feedback().IsValid()) {
+    CallICNexus nexus(p.feedback().vector(), p.feedback().slot());
+    calls = nexus.ExtractCallCount();
+  }
 
   // ---------------------------------------------------------------------------
   // Everything above this line is part of the inlining heuristic.

src/compiler/js-inlining.cc

@@ -310,6 +310,14 @@ Reduction JSInliner::ReduceJSCallFunction(Node* node,
     }
   }
 
+  // TODO(turbofan): Inlining into a try-block is not yet supported.
+  if (NodeProperties::IsExceptionalCall(node)) {
+    TRACE("Not inlining %s into %s because of surrounding try-block\n",
+          function->shared()->DebugName()->ToCString().get(),
+          info_->shared_info()->DebugName()->ToCString().get());
+    return NoChange();
+  }
+
   Zone zone;
   ParseInfo parse_info(&zone, function);
   CompilationInfo info(&parse_info);

src/compiler/js-operator.h

@@ -24,7 +24,7 @@ class VectorSlotPair {
   VectorSlotPair(Handle<TypeFeedbackVector> vector, FeedbackVectorSlot slot)
       : vector_(vector), slot_(slot) {}
 
-  bool IsValid() const { return !vector_.is_null(); }
+  bool IsValid() const { return !vector_.is_null() && !slot_.IsInvalid(); }
 
   Handle<TypeFeedbackVector> vector() const { return vector_; }
   FeedbackVectorSlot slot() const { return slot_; }

src/compiler/js-type-feedback-lowering.cc

@@ -43,7 +43,7 @@ Reduction JSTypeFeedbackLowering::ReduceJSLoadNamed(Node* node) {
   // We need to make optimistic assumptions to continue.
   if (!(flags() & kDeoptimizationEnabled)) return NoChange();
   LoadNamedParameters const& p = LoadNamedParametersOf(node->op());
-  if (p.feedback().vector().is_null()) return NoChange();
+  if (!p.feedback().IsValid()) return NoChange();  // No feedback.
   if (p.name().is_identical_to(factory()->length_string())) {
     LoadICNexus nexus(p.feedback().vector(), p.feedback().slot());
     MapHandleList maps;

src/compiler/node-properties.h

@@ -72,6 +72,8 @@ class NodeProperties final {
     return IrOpcode::IsPhiOpcode(node->opcode());
   }
 
+  // Determines whether exceptions thrown by the given node are handled locally
+  // within the graph (i.e. an IfException projection is present).
   static bool IsExceptionalCall(Node* node);
 
   // ---------------------------------------------------------------------------

test/cctest/test-api.cc

@@ -12249,6 +12249,7 @@ TEST(SetFunctionEntryHook) {
   // This test breaks because InstallGetter (function from snapshot that
   // only gets called from experimental natives) is compiled with entry hooks.
   i::FLAG_allow_natives_syntax = true;
+  i::FLAG_turbo_inlining = false;
   i::FLAG_use_inlining = false;
 
   SetFunctionEntryHookTest test;

test/cctest/test-debug.cc

@@ -6065,6 +6065,7 @@ static void DebugBreakMessageHandler(const v8::Debug::Message& message) {
 
 // Test that a debug break can be scheduled while in a message handler.
 TEST(DebugBreakInMessageHandler) {
+  i::FLAG_turbo_inlining = false;  // Make sure g is not inlined into f.
   DebugLocalContext env;
   v8::HandleScope scope(env->GetIsolate());
 
@@ -6078,7 +6079,7 @@ TEST(DebugBreakInMessageHandler) {
   v8::Local<v8::Function> g = v8::Local<v8::Function>::Cast(
       env->Global()->Get(v8::String::NewFromUtf8(env->GetIsolate(), "g")));
 
-  // Call f then g. The debugger statement in f will casue a break which will
+  // Call f then g. The debugger statement in f will cause a break which will
   // cause another break.
   f->Call(env->Global(), 0, NULL);
   CHECK_EQ(2, message_handler_break_hit_count);

test/cctest/test-deoptimization.cc

@@ -48,50 +48,60 @@ using ::v8::internal::Object;
 // Size of temp buffer for formatting small strings.
 #define SMALL_STRING_BUFFER_SIZE 80
 
-// Utility class to set --allow-natives-syntax --always-opt and --nouse-inlining
-// when constructed and return to their default state when destroyed.
+// Utility class to set the following runtime flags when constructed and return
+// to their default state when destroyed:
+//   --allow-natives-syntax --always-opt --noturbo-inlining --nouse-inlining
 class AlwaysOptimizeAllowNativesSyntaxNoInlining {
  public:
   AlwaysOptimizeAllowNativesSyntaxNoInlining()
       : always_opt_(i::FLAG_always_opt),
         allow_natives_syntax_(i::FLAG_allow_natives_syntax),
+        turbo_inlining_(i::FLAG_turbo_inlining),
         use_inlining_(i::FLAG_use_inlining) {
     i::FLAG_always_opt = true;
     i::FLAG_allow_natives_syntax = true;
+    i::FLAG_turbo_inlining = false;
     i::FLAG_use_inlining = false;
   }
 
   ~AlwaysOptimizeAllowNativesSyntaxNoInlining() {
-    i::FLAG_allow_natives_syntax = allow_natives_syntax_;
     i::FLAG_always_opt = always_opt_;
+    i::FLAG_allow_natives_syntax = allow_natives_syntax_;
+    i::FLAG_turbo_inlining = turbo_inlining_;
     i::FLAG_use_inlining = use_inlining_;
   }
 
  private:
   bool always_opt_;
   bool allow_natives_syntax_;
+  bool turbo_inlining_;
   bool use_inlining_;
 };
 
 
-// Utility class to set --allow-natives-syntax and --nouse-inlining when
-// constructed and return to their default state when destroyed.
+// Utility class to set the following runtime flags when constructed and return
+// to their default state when destroyed:
+//   --allow-natives-syntax --noturbo-inlining --nouse-inlining
 class AllowNativesSyntaxNoInlining {
  public:
   AllowNativesSyntaxNoInlining()
       : allow_natives_syntax_(i::FLAG_allow_natives_syntax),
+        turbo_inlining_(i::FLAG_turbo_inlining),
         use_inlining_(i::FLAG_use_inlining) {
     i::FLAG_allow_natives_syntax = true;
+    i::FLAG_turbo_inlining = false;
     i::FLAG_use_inlining = false;
   }
 
   ~AllowNativesSyntaxNoInlining() {
     i::FLAG_allow_natives_syntax = allow_natives_syntax_;
+    i::FLAG_turbo_inlining = turbo_inlining_;
     i::FLAG_use_inlining = use_inlining_;
   }
 
  private:
   bool allow_natives_syntax_;
+  bool turbo_inlining_;
   bool use_inlining_;
 };
 

test/cctest/test-heap-profiler.cc

@@ -2498,22 +2498,23 @@ TEST(TrackHeapAllocations) {
 
 
 static const char* inline_heap_allocation_source =
-"function f_0(x) {\n"
-"  return f_1(x+1);\n"
-"}\n"
-"%NeverOptimizeFunction(f_0);\n"
-"function f_1(x) {\n"
-"  return new f_2(x+1);\n"
-"}\n"
-"function f_2(x) {\n"
-"  this.foo = x;\n"
-"}\n"
-"var instances = [];\n"
-"function start() {\n"
-"  instances.push(f_0(0));\n"
-"}\n"
-"\n"
-"for (var i = 0; i < 100; i++) start();\n";
+    "function f_0(x) {\n"
+    "  return f_1(x+1);\n"
+    "}\n"
+    "%NeverOptimizeFunction(f_0);\n"
+    "function f_1(x) {\n"
+    "  return new f_2(x+1);\n"
+    "}\n"
+    "%NeverOptimizeFunction(f_1);\n"
+    "function f_2(x) {\n"
+    "  this.foo = x;\n"
+    "}\n"
+    "var instances = [];\n"
+    "function start() {\n"
+    "  instances.push(f_0(0));\n"
+    "}\n"
+    "\n"
+    "for (var i = 0; i < 100; i++) start();\n";
 
 
 TEST(TrackBumpPointerAllocations) {

test/cctest/test-log-stack-tracer.cc

@@ -141,6 +141,7 @@ static void CreateTraceCallerFunction(v8::Local<v8::Context> context,
 // walking.
 TEST(CFromJSStackTrace) {
   // BUG(1303) Inlining of JSFuncDoTrace() in JSTrace below breaks this test.
+  i::FLAG_turbo_inlining = false;
   i::FLAG_use_inlining = false;
 
   TickSample sample;
@@ -189,6 +190,7 @@ TEST(CFromJSStackTrace) {
 TEST(PureJSStackTrace) {
   // This test does not pass with inlining enabled since inlined functions
   // don't appear in the stack trace.
+  i::FLAG_turbo_inlining = false;
   i::FLAG_use_inlining = false;
 
   TickSample sample;

test/cctest/test-profile-generator.cc

@@ -496,6 +496,7 @@ static const ProfileNode* PickChild(const ProfileNode* parent,
 TEST(RecordStackTraceAtStartProfiling) {
   // This test does not pass with inlining enabled since inlined functions
   // don't appear in the stack trace.
+  i::FLAG_turbo_inlining = false;
   i::FLAG_use_inlining = false;
 
   v8::HandleScope scope(CcTest::isolate());
@@ -571,6 +572,7 @@ static const v8::CpuProfileNode* PickChild(const v8::CpuProfileNode* parent,
 TEST(ProfileNodeScriptId) {
   // This test does not pass with inlining enabled since inlined functions
   // don't appear in the stack trace.
+  i::FLAG_turbo_inlining = false;
   i::FLAG_use_inlining = false;
 
   v8::HandleScope scope(CcTest::isolate());

test/cctest/test-sampler-api.cc

@@ -226,16 +226,17 @@ TEST(StackDepthDoesNotExceedMaxValue) {
 //                              ^      ^       ^
 // sample.stack indices         2      1       0
 TEST(StackFramesConsistent) {
-  // Note: The arguments.callee stuff is there so that the
-  //       functions are not optimized away.
+  i::FLAG_allow_natives_syntax = true;
   const char* test_script =
       "function test_sampler_api_inner() {"
       "  CollectSample();"
-      "  return arguments.callee.toString();"
+      "  return 0;"
       "}"
       "function test_sampler_api_outer() {"
-      "  return test_sampler_api_inner() + arguments.callee.toString();"
+      "  return test_sampler_api_inner();"
       "}"
+      "%NeverOptimizeFunction(test_sampler_api_inner);"
+      "%NeverOptimizeFunction(test_sampler_api_outer);"
       "test_sampler_api_outer();";
 
   SamplingTestHelper helper(test_script);

test/mjsunit/mjsunit.status

@@ -110,6 +110,7 @@
   'debug-step-3': [PASS, NO_VARIANTS],  # flaky in no-snap mode.
   'debug-stepframe-clearing': [PASS, NO_VARIANTS],  # only in no-snap debug.
   'debug-stepin-call-function-stub': [PASS, NO_VARIANTS],  # only in no-snap debug.
+  'debug-stepin-positions': [PASS, NO_VARIANTS],  # only due to inlining.
   'regress/regress-3717': [PASS, NO_VARIANTS],  # only in no-snap mode.
   'regress/regress-2451': [PASS, NO_VARIANTS],  # with custom snapshot and gc-stress.
   'debug-multiple-breakpoints': [PASS, NO_VARIANTS],  # with custom snapshot and gc-stress.

test/mjsunit/regress/regress-crbug-465298.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --noturbo-osr --expose-debug-as debug
+// Flags: --noturbo-osr --noturbo-inlining --expose-debug-as debug
 
 var stdlib = this;
 var buffer = new ArrayBuffer(64 * 1024);