[debug] do not inline builtin if breaking on entry.

R=jarin@chromium.org

Bug: v8:178
Change-Id: I4b77e96072d5b8b70df21477bba8cdbf573d184c
Reviewed-on: https://chromium-review.googlesource.com/908289Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51174}

src/compiler/js-builtin-reducer.cc

@@ -39,6 +39,14 @@ class JSCallReduction {
     return function->shared()->HasBuiltinFunctionId();
   }
 
+  bool BuiltinCanBeInlined() {
+    DCHECK_EQ(IrOpcode::kJSCall, node_->opcode());
+    HeapObjectMatcher m(NodeProperties::GetValueInput(node_, 0));
+    Handle<JSFunction> function = Handle<JSFunction>::cast(m.Value());
+    // Do not inline if the builtin may have break points.
+    return !function->shared()->HasBreakInfo();
+  }
+
   // Retrieves the BuiltinFunctionId as described above.
   BuiltinFunctionId GetBuiltinFunctionId() {
     DCHECK_EQ(IrOpcode::kJSCall, node_->opcode());
@@ -2207,6 +2215,7 @@ Reduction JSBuiltinReducer::Reduce(Node* node) {
 
   // Dispatch according to the BuiltinFunctionId if present.
   if (!r.HasBuiltinFunctionId()) return NoChange();
+  if (!r.BuiltinCanBeInlined()) return NoChange();
   switch (r.GetBuiltinFunctionId()) {
     case kArrayEntries:
       return ReduceArrayIterator(node, IterationKind::kEntries);

src/compiler/js-typed-lowering.cc

@@ -1666,6 +1666,12 @@ Reduction JSTypedLowering::ReduceJSCall(Node* node) {
     Handle<JSFunction> function =
         Handle<JSFunction>::cast(target_type->AsHeapConstant()->Value());
     Handle<SharedFunctionInfo> shared(function->shared(), isolate());
+
+    if (function->shared()->HasBreakInfo()) {
+      // Do not inline the call if we need to check whether to break at entry.
+      return NoChange();
+    }
+
     const int builtin_index = shared->code()->builtin_index();
     const bool is_builtin = (builtin_index != -1);
 
@@ -1697,6 +1703,7 @@ Reduction JSTypedLowering::ReduceJSCall(Node* node) {
     CallDescriptor::Flags flags = CallDescriptor::kNeedsFrameState;
     Node* new_target = jsgraph()->UndefinedConstant();
     Node* argument_count = jsgraph()->Constant(arity);
+
     if (NeedsArgumentAdaptorFrame(shared, arity)) {
       // Patch {node} to an indirect call via the ArgumentsAdaptorTrampoline.
       Callable callable = CodeFactory::ArgumentAdaptor(isolate());

test/cctest/test-debug.cc

@@ -1141,6 +1141,7 @@ TEST(BreakPointReturn) {
 
 // Test that a break point can be set at a return store location.
 TEST(BreakPointBuiltin) {
+  i::FLAG_allow_natives_syntax = true;
   break_point_hit_count = 0;
   DebugLocalContext env;
   v8::HandleScope scope(env->GetIsolate());
@@ -1149,10 +1150,11 @@ TEST(BreakPointBuiltin) {
   v8::Local<v8::Function> builtin =
       CompileRun("String.prototype.repeat").As<v8::Function>();
 
+  // === Test simple builtin ===
   CompileRun("'a'.repeat(10)");
   CHECK_EQ(0, break_point_hit_count);
 
-  // Run with breakpoint
+  // Run with breakpoint.
   int bp = SetBreakPoint(builtin, 0);
   CompileRun("'b'.repeat(10)");
   CHECK_EQ(1, break_point_hit_count);
@@ -1162,6 +1164,70 @@ TEST(BreakPointBuiltin) {
   CompileRun("'b'.repeat(10)");
   CHECK_EQ(1, break_point_hit_count);
 
+  // === Test inlined builtin ===
+  break_point_hit_count = 0;
+  builtin = CompileRun("Math.sin").As<v8::Function>();
+  CompileRun("function test(x) { return 1 + Math.sin(x) }");
+  CompileRun(
+      "test(0.5); test(0.6);"
+      "%OptimizeFunctionOnNextCall(test); test(0.7);");
+  CHECK_EQ(0, break_point_hit_count);
+
+  // Run with breakpoint.
+  bp = SetBreakPoint(builtin, 0);
+  CompileRun("Math.sin(0.1);");
+  CHECK_EQ(1, break_point_hit_count);
+  CompileRun("test(0.2);");
+  CHECK_EQ(2, break_point_hit_count);
+
+  // Re-optimize.
+  CompileRun("%OptimizeFunctionOnNextCall(test);");
+  CompileRun("test(0.3);");
+  CHECK_EQ(3, break_point_hit_count);
+
+  // Run without breakpoints.
+  ClearBreakPoint(bp);
+  CompileRun("test(0.3);");
+  CHECK_EQ(3, break_point_hit_count);
+
+  SetDebugEventListener(env->GetIsolate(), nullptr);
+  CheckDebuggerUnloaded();
+}
+
+TEST(BreakPointInlining) {
+  i::FLAG_allow_natives_syntax = true;
+  break_point_hit_count = 0;
+  DebugLocalContext env;
+  v8::HandleScope scope(env->GetIsolate());
+
+  SetDebugEventListener(env->GetIsolate(), DebugEventBreakPointHitCount);
+
+  break_point_hit_count = 0;
+  v8::Local<v8::Function> inlinee =
+      CompileRun("function f(x) { return x*2; } f").As<v8::Function>();
+  CompileRun("function test(x) { return 1 + f(x) }");
+  CompileRun(
+      "test(0.5); test(0.6);"
+      "%OptimizeFunctionOnNextCall(test); test(0.7);");
+  CHECK_EQ(0, break_point_hit_count);
+
+  // Run with breakpoint.
+  int bp = SetBreakPoint(inlinee, 0);
+  CompileRun("f(0.1);");
+  CHECK_EQ(1, break_point_hit_count);
+  CompileRun("test(0.2);");
+  CHECK_EQ(2, break_point_hit_count);
+
+  // Re-optimize.
+  CompileRun("%OptimizeFunctionOnNextCall(test);");
+  CompileRun("test(0.3);");
+  CHECK_EQ(3, break_point_hit_count);
+
+  // Run without breakpoints.
+  ClearBreakPoint(bp);
+  CompileRun("test(0.3);");
+  CHECK_EQ(3, break_point_hit_count);
+
   SetDebugEventListener(env->GetIsolate(), nullptr);
   CheckDebuggerUnloaded();
 }