[codegen] Defer debug hook in InvokeFunctionCode

If we do the check in InvokeFunctionCode instead of doing it in
CheckDebugHook (now aptly named 'DebugHook'), we can defer the
debug hook to the end of InvokeFunctionCode and is to make the
common case stay in straight-line code.

Bug: v8:9771
Change-Id: Icff84e8ced9d120ae2fe293a391aaadd53441644
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1829219
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Reviewed-by: Dan Elphick <delphick@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64183}

src/codegen/arm/macro-assembler-arm.cc

@@ -1602,56 +1602,43 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
   }
 }
 
-void MacroAssembler::CheckDebugHook(Register fun, Register new_target,
-                                    const ParameterCount& expected,
-                                    const ParameterCount& actual) {
-  Label skip_hook;
-
-  ExternalReference debug_hook_active =
-      ExternalReference::debug_hook_on_function_call_address(isolate());
-  Move(r4, debug_hook_active);
-  ldrsb(r4, MemOperand(r4));
-  cmp(r4, Operand(0));
-  b(eq, &skip_hook);
-
-  {
-    // Load receiver to pass it later to DebugOnFunctionCall hook.
-    if (actual.is_reg()) {
-      ldr(r4, MemOperand(sp, actual.reg(), LSL, kPointerSizeLog2));
-    } else {
-      ldr(r4, MemOperand(sp, actual.immediate() << kPointerSizeLog2));
-    }
-    FrameScope frame(this,
-                     has_frame() ? StackFrame::NONE : StackFrame::INTERNAL);
-    if (expected.is_reg()) {
-      SmiTag(expected.reg());
-      Push(expected.reg());
-    }
-    if (actual.is_reg()) {
-      SmiTag(actual.reg());
-      Push(actual.reg());
-    }
-    if (new_target.is_valid()) {
-      Push(new_target);
-    }
-    Push(fun);
-    Push(fun);
-    Push(r4);
-    CallRuntime(Runtime::kDebugOnFunctionCall);
-    Pop(fun);
-    if (new_target.is_valid()) {
-      Pop(new_target);
-    }
-    if (actual.is_reg()) {
-      Pop(actual.reg());
-      SmiUntag(actual.reg());
-    }
-    if (expected.is_reg()) {
-      Pop(expected.reg());
-      SmiUntag(expected.reg());
-    }
+void MacroAssembler::CallDebugOnFunctionCall(Register fun, Register new_target,
+                                             const ParameterCount& expected,
+                                             const ParameterCount& actual) {
+  // Load receiver to pass it later to DebugOnFunctionCall hook.
+  if (actual.is_reg()) {
+    ldr(r4, MemOperand(sp, actual.reg(), LSL, kPointerSizeLog2));
+  } else {
+    ldr(r4, MemOperand(sp, actual.immediate() << kPointerSizeLog2));
+  }
+  FrameScope frame(this, has_frame() ? StackFrame::NONE : StackFrame::INTERNAL);
+  if (expected.is_reg()) {
+    SmiTag(expected.reg());
+    Push(expected.reg());
+  }
+  if (actual.is_reg()) {
+    SmiTag(actual.reg());
+    Push(actual.reg());
+  }
+  if (new_target.is_valid()) {
+    Push(new_target);
+  }
+  Push(fun);
+  Push(fun);
+  Push(r4);
+  CallRuntime(Runtime::kDebugOnFunctionCall);
+  Pop(fun);
+  if (new_target.is_valid()) {
+    Pop(new_target);
+  }
+  if (actual.is_reg()) {
+    Pop(actual.reg());
+    SmiUntag(actual.reg());
+  }
+  if (expected.is_reg()) {
+    Pop(expected.reg());
+    SmiUntag(expected.reg());
   }
-  bind(&skip_hook);
 }
 
 void MacroAssembler::InvokeFunctionCode(Register function, Register new_target,
@@ -1664,7 +1651,16 @@ void MacroAssembler::InvokeFunctionCode(Register function, Register new_target,
   DCHECK_IMPLIES(new_target.is_valid(), new_target == r3);
 
   // On function call, call into the debugger if necessary.
-  CheckDebugHook(function, new_target, expected, actual);
+  Label debug_hook, continue_after_hook;
+  {
+    ExternalReference debug_hook_active =
+        ExternalReference::debug_hook_on_function_call_address(isolate());
+    Move(r4, debug_hook_active);
+    ldrsb(r4, MemOperand(r4));
+    cmp(r4, Operand(0));
+    b(ne, &debug_hook);
+  }
+  bind(&continue_after_hook);
 
   // Clear the new.target register if not given.
   if (!new_target.is_valid()) {
@@ -1686,11 +1682,17 @@ void MacroAssembler::InvokeFunctionCode(Register function, Register new_target,
       DCHECK(flag == JUMP_FUNCTION);
       JumpCodeObject(code);
     }
-
-    // Continue here if InvokePrologue does handle the invocation due to
-    // mismatched parameter counts.
-    bind(&done);
   }
+  b(&done);
+
+  // Deferred debug hook.
+  bind(&debug_hook);
+  CallDebugOnFunctionCall(function, new_target, expected, actual);
+  b(&continue_after_hook);
+
+  // Continue here if InvokePrologue does handle the invocation due to
+  // mismatched parameter counts.
+  bind(&done);
 }
 
 void MacroAssembler::InvokeFunction(Register fun, Register new_target,

src/codegen/arm/macro-assembler-arm.h

@@ -633,10 +633,10 @@ class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler {
                           const ParameterCount& expected,
                           const ParameterCount& actual, InvokeFlag flag);
 
-  // On function call, call into the debugger if necessary.
-  void CheckDebugHook(Register fun, Register new_target,
-                      const ParameterCount& expected,
-                      const ParameterCount& actual);
+  // On function call, call into the debugger.
+  void CallDebugOnFunctionCall(Register fun, Register new_target,
+                               const ParameterCount& expected,
+                               const ParameterCount& actual);
 
   // Invoke the JavaScript function in the given register. Changes the
   // current context to the context in the function before invoking.

src/codegen/arm64/macro-assembler-arm64.cc

@@ -2211,44 +2211,34 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
   Bind(&regular_invoke);
 }
 
-void MacroAssembler::CheckDebugHook(Register fun, Register new_target,
-                                    const ParameterCount& expected,
-                                    const ParameterCount& actual) {
-  Label skip_hook;
-
-  Mov(x4, ExternalReference::debug_hook_on_function_call_address(isolate()));
-  Ldrsb(x4, MemOperand(x4));
-  Cbz(x4, &skip_hook);
-
-  {
-    // Load receiver to pass it later to DebugOnFunctionCall hook.
-    if (actual.is_reg()) {
-      Ldr(x4, MemOperand(sp, actual.reg(), LSL, kSystemPointerSizeLog2));
-    } else {
-      Ldr(x4, MemOperand(sp, actual.immediate() << kSystemPointerSizeLog2));
-    }
-    FrameScope frame(this,
-                     has_frame() ? StackFrame::NONE : StackFrame::INTERNAL);
+void MacroAssembler::CallDebugOnFunctionCall(Register fun, Register new_target,
+                                             const ParameterCount& expected,
+                                             const ParameterCount& actual) {
+  // Load receiver to pass it later to DebugOnFunctionCall hook.
+  if (actual.is_reg()) {
+    Ldr(x4, MemOperand(sp, actual.reg(), LSL, kSystemPointerSizeLog2));
+  } else {
+    Ldr(x4, MemOperand(sp, actual.immediate() << kSystemPointerSizeLog2));
+  }
+  FrameScope frame(this, has_frame() ? StackFrame::NONE : StackFrame::INTERNAL);
 
-    Register expected_reg = padreg;
-    Register actual_reg = padreg;
-    if (expected.is_reg()) expected_reg = expected.reg();
-    if (actual.is_reg()) actual_reg = actual.reg();
-    if (!new_target.is_valid()) new_target = padreg;
+  Register expected_reg = padreg;
+  Register actual_reg = padreg;
+  if (expected.is_reg()) expected_reg = expected.reg();
+  if (actual.is_reg()) actual_reg = actual.reg();
+  if (!new_target.is_valid()) new_target = padreg;
 
-    // Save values on stack.
-    SmiTag(expected_reg);
-    SmiTag(actual_reg);
-    Push(expected_reg, actual_reg, new_target, fun);
-    Push(fun, x4);
-    CallRuntime(Runtime::kDebugOnFunctionCall);
+  // Save values on stack.
+  SmiTag(expected_reg);
+  SmiTag(actual_reg);
+  Push(expected_reg, actual_reg, new_target, fun);
+  Push(fun, x4);
+  CallRuntime(Runtime::kDebugOnFunctionCall);
 
-    // Restore values from stack.
-    Pop(fun, new_target, actual_reg, expected_reg);
-    SmiUntag(actual_reg);
-    SmiUntag(expected_reg);
-  }
-  Bind(&skip_hook);
+  // Restore values from stack.
+  Pop(fun, new_target, actual_reg, expected_reg);
+  SmiUntag(actual_reg);
+  SmiUntag(expected_reg);
 }
 
 void MacroAssembler::InvokeFunctionCode(Register function, Register new_target,
@@ -2261,7 +2251,13 @@ void MacroAssembler::InvokeFunctionCode(Register function, Register new_target,
   DCHECK_IMPLIES(new_target.is_valid(), new_target.is(x3));
 
   // On function call, call into the debugger if necessary.
-  CheckDebugHook(function, new_target, expected, actual);
+  Label debug_hook, continue_after_hook;
+  {
+    Mov(x4, ExternalReference::debug_hook_on_function_call_address(isolate()));
+    Ldrsb(x4, MemOperand(x4));
+    Cbnz(x4, &debug_hook);
+  }
+  bind(&continue_after_hook);
 
   // Clear the new.target register if not given.
   if (!new_target.is_valid()) {
@@ -2289,6 +2285,12 @@ void MacroAssembler::InvokeFunctionCode(Register function, Register new_target,
       JumpCodeObject(code);
     }
   }
+  B(&done);
+
+  // Deferred debug hook.
+  bind(&debug_hook);
+  CallDebugOnFunctionCall(function, new_target, expected, actual);
+  B(&continue_after_hook);
 
   // Continue here if InvokePrologue does handle the invocation due to
   // mismatched parameter counts.

src/codegen/arm64/macro-assembler-arm64.h

@@ -1770,10 +1770,10 @@ class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler {
                       const ParameterCount& actual, Label* done,
                       InvokeFlag flag, bool* definitely_mismatches);
 
-  // On function call, call into the debugger if necessary.
-  void CheckDebugHook(Register fun, Register new_target,
-                      const ParameterCount& expected,
-                      const ParameterCount& actual);
+  // On function call, call into the debugger.
+  void CallDebugOnFunctionCall(Register fun, Register new_target,
+                               const ParameterCount& expected,
+                               const ParameterCount& actual);
   void InvokeFunctionCode(Register function, Register new_target,
                           const ParameterCount& expected,
                           const ParameterCount& actual, InvokeFlag flag);

src/codegen/ia32/macro-assembler-ia32.cc

@@ -1168,57 +1168,44 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
   }
 }
 
-void MacroAssembler::CheckDebugHook(Register fun, Register new_target,
-                                    const ParameterCount& expected,
-                                    const ParameterCount& actual) {
-  Label skip_hook;
-
-  ExternalReference debug_hook_active =
-      ExternalReference::debug_hook_on_function_call_address(isolate());
-  push(eax);
-  cmpb(ExternalReferenceAsOperand(debug_hook_active, eax), Immediate(0));
-  pop(eax);
-  j(equal, &skip_hook);
-
-  {
-    FrameScope frame(this,
-                     has_frame() ? StackFrame::NONE : StackFrame::INTERNAL);
-    if (expected.is_reg()) {
-      SmiTag(expected.reg());
-      Push(expected.reg());
-    }
-    if (actual.is_reg()) {
-      SmiTag(actual.reg());
-      Push(actual.reg());
-      SmiUntag(actual.reg());
-    }
-    if (new_target.is_valid()) {
-      Push(new_target);
-    }
-    Push(fun);
-    Push(fun);
-    Operand receiver_op =
-        actual.is_reg()
-            ? Operand(ebp, actual.reg(), times_system_pointer_size,
-                      kSystemPointerSize * 2)
-            : Operand(ebp, actual.immediate() * times_system_pointer_size +
-                               kSystemPointerSize * 2);
-    Push(receiver_op);
-    CallRuntime(Runtime::kDebugOnFunctionCall);
-    Pop(fun);
-    if (new_target.is_valid()) {
-      Pop(new_target);
-    }
-    if (actual.is_reg()) {
-      Pop(actual.reg());
-      SmiUntag(actual.reg());
-    }
-    if (expected.is_reg()) {
-      Pop(expected.reg());
-      SmiUntag(expected.reg());
-    }
+void MacroAssembler::CallDebugOnFunctionCall(Register fun, Register new_target,
+                                             const ParameterCount& expected,
+                                             const ParameterCount& actual) {
+  FrameScope frame(this, has_frame() ? StackFrame::NONE : StackFrame::INTERNAL);
+  if (expected.is_reg()) {
+    SmiTag(expected.reg());
+    Push(expected.reg());
+  }
+  if (actual.is_reg()) {
+    SmiTag(actual.reg());
+    Push(actual.reg());
+    SmiUntag(actual.reg());
+  }
+  if (new_target.is_valid()) {
+    Push(new_target);
+  }
+  Push(fun);
+  Push(fun);
+  Operand receiver_op =
+      actual.is_reg()
+          ? Operand(ebp, actual.reg(), times_system_pointer_size,
+                    kSystemPointerSize * 2)
+          : Operand(ebp, actual.immediate() * times_system_pointer_size +
+                             kSystemPointerSize * 2);
+  Push(receiver_op);
+  CallRuntime(Runtime::kDebugOnFunctionCall);
+  Pop(fun);
+  if (new_target.is_valid()) {
+    Pop(new_target);
+  }
+  if (actual.is_reg()) {
+    Pop(actual.reg());
+    SmiUntag(actual.reg());
+  }
+  if (expected.is_reg()) {
+    Pop(expected.reg());
+    SmiUntag(expected.reg());
   }
-  bind(&skip_hook);
 }
 
 void MacroAssembler::InvokeFunctionCode(Register function, Register new_target,
@@ -1233,7 +1220,16 @@ void MacroAssembler::InvokeFunctionCode(Register function, Register new_target,
   DCHECK_IMPLIES(actual.is_reg(), actual.reg() == eax);
 
   // On function call, call into the debugger if necessary.
-  CheckDebugHook(function, new_target, expected, actual);
+  Label debug_hook, continue_after_hook;
+  {
+    ExternalReference debug_hook_active =
+        ExternalReference::debug_hook_on_function_call_address(isolate());
+    push(eax);
+    cmpb(ExternalReferenceAsOperand(debug_hook_active, eax), Immediate(0));
+    pop(eax);
+    j(not_equal, &debug_hook, Label::kNear);
+  }
+  bind(&continue_after_hook);
 
   // Clear the new.target register if not given.
   if (!new_target.is_valid()) {
@@ -1256,8 +1252,15 @@ void MacroAssembler::InvokeFunctionCode(Register function, Register new_target,
       DCHECK(flag == JUMP_FUNCTION);
       JumpCodeObject(ecx);
     }
-    bind(&done);
   }
+  jmp(&done, Label::kNear);
+
+  // Deferred debug hook.
+  bind(&debug_hook);
+  CallDebugOnFunctionCall(function, new_target, expected, actual);
+  jmp(&continue_after_hook, Label::kNear);
+
+  bind(&done);
 }
 
 void MacroAssembler::InvokeFunction(Register fun, Register new_target,

src/codegen/ia32/macro-assembler-ia32.h

@@ -552,11 +552,11 @@ class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler {
                           const ParameterCount& expected,
                           const ParameterCount& actual, InvokeFlag flag);
 
-  // On function call, call into the debugger if necessary.
+  // On function call, call into the debugger.
   // This may clobber ecx.
-  void CheckDebugHook(Register fun, Register new_target,
-                      const ParameterCount& expected,
-                      const ParameterCount& actual);
+  void CallDebugOnFunctionCall(Register fun, Register new_target,
+                               const ParameterCount& expected,
+                               const ParameterCount& actual);
 
   // Invoke the JavaScript function in the given register. Changes the
   // current context to the context in the function before invoking.

src/codegen/x64/macro-assembler-x64.cc

@@ -2281,7 +2281,16 @@ void MacroAssembler::InvokeFunctionCode(Register function, Register new_target,
   DCHECK_IMPLIES(new_target.is_valid(), new_target == rdx);
 
   // On function call, call into the debugger if necessary.
-  CheckDebugHook(function, new_target, expected, actual);
+  Label debug_hook, continue_after_hook;
+  {
+    ExternalReference debug_hook_active =
+        ExternalReference::debug_hook_on_function_call_address(isolate());
+    Operand debug_hook_active_operand =
+        ExternalReferenceAsOperand(debug_hook_active);
+    cmpb(debug_hook_active_operand, Immediate(0));
+    j(not_equal, &debug_hook, Label::kNear);
+  }
+  bind(&continue_after_hook);
 
   // Clear the new.target register if not given.
   if (!new_target.is_valid()) {
@@ -2305,8 +2314,15 @@ void MacroAssembler::InvokeFunctionCode(Register function, Register new_target,
       DCHECK(flag == JUMP_FUNCTION);
       JumpCodeObject(rcx);
     }
-    bind(&done);
   }
+  jmp(&done, Label::kNear);
+
+  // Deferred debug hook.
+  bind(&debug_hook);
+  CallDebugOnFunctionCall(function, new_target, expected, actual);
+  jmp(&continue_after_hook, Label::kNear);
+
+  bind(&done);
 }
 
 void MacroAssembler::InvokePrologue(const ParameterCount& expected,
@@ -2371,50 +2387,38 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
   }
 }
 
-void MacroAssembler::CheckDebugHook(Register fun, Register new_target,
-                                    const ParameterCount& expected,
-                                    const ParameterCount& actual) {
-  Label skip_hook;
-  ExternalReference debug_hook_active =
-      ExternalReference::debug_hook_on_function_call_address(isolate());
-  Operand debug_hook_active_operand =
-      ExternalReferenceAsOperand(debug_hook_active);
-  cmpb(debug_hook_active_operand, Immediate(0));
-  j(equal, &skip_hook);
-
-  {
-    FrameScope frame(this,
-                     has_frame() ? StackFrame::NONE : StackFrame::INTERNAL);
-    if (expected.is_reg()) {
-      SmiTag(expected.reg());
-      Push(expected.reg());
-    }
-    if (actual.is_reg()) {
-      SmiTag(actual.reg());
-      Push(actual.reg());
-      SmiUntag(actual.reg());
-    }
-    if (new_target.is_valid()) {
-      Push(new_target);
-    }
-    Push(fun);
-    Push(fun);
-    Push(StackArgumentsAccessor(rbp, actual).GetReceiverOperand());
-    CallRuntime(Runtime::kDebugOnFunctionCall);
-    Pop(fun);
-    if (new_target.is_valid()) {
-      Pop(new_target);
-    }
-    if (actual.is_reg()) {
-      Pop(actual.reg());
-      SmiUntag(actual.reg());
-    }
-    if (expected.is_reg()) {
-      Pop(expected.reg());
-      SmiUntag(expected.reg());
-    }
+void MacroAssembler::CallDebugOnFunctionCall(Register fun, Register new_target,
+                                             const ParameterCount& expected,
+                                             const ParameterCount& actual) {
+  FrameScope frame(this, has_frame() ? StackFrame::NONE : StackFrame::INTERNAL);
+  if (expected.is_reg()) {
+    SmiTag(expected.reg());
+    Push(expected.reg());
+  }
+  if (actual.is_reg()) {
+    SmiTag(actual.reg());
+    Push(actual.reg());
+    SmiUntag(actual.reg());
+  }
+  if (new_target.is_valid()) {
+    Push(new_target);
+  }
+  Push(fun);
+  Push(fun);
+  Push(StackArgumentsAccessor(rbp, actual).GetReceiverOperand());
+  CallRuntime(Runtime::kDebugOnFunctionCall);
+  Pop(fun);
+  if (new_target.is_valid()) {
+    Pop(new_target);
+  }
+  if (actual.is_reg()) {
+    Pop(actual.reg());
+    SmiUntag(actual.reg());
+  }
+  if (expected.is_reg()) {
+    Pop(expected.reg());
+    SmiUntag(expected.reg());
   }
-  bind(&skip_hook);
 }
 
 void TurboAssembler::StubPrologue(StackFrame::Type type) {

src/codegen/x64/macro-assembler-x64.h

@@ -649,10 +649,10 @@ class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler {
                           const ParameterCount& expected,
                           const ParameterCount& actual, InvokeFlag flag);
 
-  // On function call, call into the debugger if necessary.
-  void CheckDebugHook(Register fun, Register new_target,
-                      const ParameterCount& expected,
-                      const ParameterCount& actual);
+  // On function call, call into the debugger.
+  void CallDebugOnFunctionCall(Register fun, Register new_target,
+                               const ParameterCount& expected,
+                               const ParameterCount& actual);
 
   // Invoke the JavaScript function in the given register. Changes the
   // current context to the context in the function before invoking.