Skip to content

Commit

Permalink
deps: V8: cherry-pick bf0bd4868dde
Browse files Browse the repository at this point in the history
Original commit message:

    [fastcall] Disable fast calls with stack args on M1

    Bug: v8:13171
    Change-Id: I549d942d8ae24e2de0aa3202d7400b587254fb75
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3963995
    Commit-Queue: Camillo Bruni <cbruni@chromium.org>
    Auto-Submit: Maya Lekova <mslekova@chromium.org>
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Commit-Queue: Maya Lekova <mslekova@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#83886}

Refs: v8/v8@bf0bd48
PR-URL: #45908
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
  • Loading branch information
targos authored and juanarbol committed Mar 5, 2023
1 parent 8adaa13 commit 5453cd9
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 7 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.23',
'v8_embedder_string': '-node.24',

##### V8 defaults for Node.js #####

Expand Down
7 changes: 7 additions & 0 deletions deps/v8/src/compiler/fast-api-calls.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ OverloadsResolutionResult ResolveOverloads(
bool CanOptimizeFastSignature(const CFunctionInfo* c_signature) {
USE(c_signature);

#if defined(V8_OS_MACOS) && defined(V8_TARGET_ARCH_ARM64)
// On MacArm64 hardware we don't support passing of arguments on the stack.
if (c_signature->ArgumentCount() > 8) {
return false;
}
#endif // defined(V8_OS_MACOS) && defined(V8_TARGET_ARCH_ARM64)

#ifndef V8_ENABLE_FP_PARAMS_IN_C_LINKAGE
if (c_signature->ReturnInfo().GetType() == CTypeInfo::Type::kFloat32 ||
c_signature->ReturnInfo().GetType() == CTypeInfo::Type::kFloat64) {
Expand Down
67 changes: 61 additions & 6 deletions deps/v8/src/d8/d8-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,19 @@ class FastCApiObject {
}

#ifdef V8_USE_SIMULATOR_WITH_GENERIC_C_CALLS
static AnyCType AddAll32BitIntFastCallback_8ArgsPatch(
AnyCType receiver, AnyCType should_fallback, AnyCType arg1_i32,
AnyCType arg2_i32, AnyCType arg3_i32, AnyCType arg4_u32,
AnyCType arg5_u32, AnyCType arg6_u32, AnyCType arg7_u32,
AnyCType arg8_u32, AnyCType options) {
AnyCType ret;
ret.int32_value = AddAll32BitIntFastCallback_8Args(
receiver.object_value, should_fallback.bool_value, arg1_i32.int32_value,
arg2_i32.int32_value, arg3_i32.int32_value, arg4_u32.uint32_value,
arg5_u32.uint32_value, arg6_u32.uint32_value, arg7_u32.uint32_value,
arg8_u32.uint32_value, *options.options_value);
return ret;
}
static AnyCType AddAll32BitIntFastCallback_6ArgsPatch(
AnyCType receiver, AnyCType should_fallback, AnyCType arg1_i32,
AnyCType arg2_i32, AnyCType arg3_i32, AnyCType arg4_u32,
Expand All @@ -479,6 +492,26 @@ class FastCApiObject {
}
#endif // V8_USE_SIMULATOR_WITH_GENERIC_C_CALLS

static int AddAll32BitIntFastCallback_8Args(
Local<Object> receiver, bool should_fallback, int32_t arg1_i32,
int32_t arg2_i32, int32_t arg3_i32, uint32_t arg4_u32, uint32_t arg5_u32,
uint32_t arg6_u32, uint32_t arg7_u32, uint32_t arg8_u32,
FastApiCallbackOptions& options) {
FastCApiObject* self = UnwrapObject(receiver);
CHECK_SELF_OR_FALLBACK(0);
self->fast_call_count_++;

if (should_fallback) {
options.fallback = true;
return 0;
}

int64_t result = static_cast<int64_t>(arg1_i32) + arg2_i32 + arg3_i32 +
arg4_u32 + arg5_u32 + arg6_u32 + arg7_u32 + arg8_u32;
if (result > INT_MAX) return INT_MAX;
if (result < INT_MIN) return INT_MIN;
return static_cast<int>(result);
}
static int AddAll32BitIntFastCallback_6Args(
Local<Object> receiver, bool should_fallback, int32_t arg1_i32,
int32_t arg2_i32, int32_t arg3_i32, uint32_t arg4_u32, uint32_t arg5_u32,
Expand Down Expand Up @@ -516,24 +549,29 @@ class FastCApiObject {

HandleScope handle_scope(isolate);

Local<Context> context = isolate->GetCurrentContext();
double sum = 0;
if (args.Length() > 1 && args[1]->IsNumber()) {
sum += args[1]->Int32Value(isolate->GetCurrentContext()).FromJust();
sum += args[1]->Int32Value(context).FromJust();
}
if (args.Length() > 2 && args[2]->IsNumber()) {
sum += args[2]->Int32Value(isolate->GetCurrentContext()).FromJust();
sum += args[2]->Int32Value(context).FromJust();
}
if (args.Length() > 3 && args[3]->IsNumber()) {
sum += args[3]->Int32Value(isolate->GetCurrentContext()).FromJust();
sum += args[3]->Int32Value(context).FromJust();
}
if (args.Length() > 4 && args[4]->IsNumber()) {
sum += args[4]->Uint32Value(isolate->GetCurrentContext()).FromJust();
sum += args[4]->Uint32Value(context).FromJust();
}
if (args.Length() > 5 && args[5]->IsNumber()) {
sum += args[5]->Uint32Value(isolate->GetCurrentContext()).FromJust();
sum += args[5]->Uint32Value(context).FromJust();
}
if (args.Length() > 6 && args[6]->IsNumber()) {
sum += args[6]->Uint32Value(isolate->GetCurrentContext()).FromJust();
sum += args[6]->Uint32Value(context).FromJust();
}
if (args.Length() > 7 && args[7]->IsNumber() && args[8]->IsNumber()) {
sum += args[7]->Uint32Value(context).FromJust();
sum += args[8]->Uint32Value(context).FromJust();
}

args.GetReturnValue().Set(Number::New(isolate, sum));
Expand Down Expand Up @@ -804,6 +842,9 @@ Local<FunctionTemplate> Shell::CreateTestFastCApiTemplate(Isolate* isolate) {
signature, 1, ConstructorBehavior::kThrow,
SideEffectType::kHasSideEffect, {add_all_invalid_overloads, 2}));

CFunction add_all_32bit_int_8args_c_func = CFunction::Make(
FastCApiObject::AddAll32BitIntFastCallback_8Args V8_IF_USE_SIMULATOR(
FastCApiObject::AddAll32BitIntFastCallback_8ArgsPatch));
CFunction add_all_32bit_int_6args_c_func = CFunction::Make(
FastCApiObject::AddAll32BitIntFastCallback_6Args V8_IF_USE_SIMULATOR(
FastCApiObject::AddAll32BitIntFastCallback_6ArgsPatch));
Expand All @@ -820,6 +861,20 @@ Local<FunctionTemplate> Shell::CreateTestFastCApiTemplate(Isolate* isolate) {
signature, 1, ConstructorBehavior::kThrow,
SideEffectType::kHasSideEffect, {c_function_overloads, 2}));

api_obj_ctor->PrototypeTemplate()->Set(
isolate, "overloaded_add_all_8args",
FunctionTemplate::New(
isolate, FastCApiObject::AddAll32BitIntSlowCallback, Local<Value>(),
signature, 1, ConstructorBehavior::kThrow,
SideEffectType::kHasSideEffect, &add_all_32bit_int_8args_c_func));

api_obj_ctor->PrototypeTemplate()->Set(
isolate, "overloaded_add_all_32bit_int_no_sig",
FunctionTemplate::NewWithCFunctionOverloads(
isolate, FastCApiObject::AddAll32BitIntSlowCallback, Local<Value>(),
Local<Signature>(), 1, ConstructorBehavior::kThrow,
SideEffectType::kHasSideEffect, {c_function_overloads, 2}));

CFunction add_all_no_options_c_func = CFunction::Make(
FastCApiObject::AddAllFastCallbackNoOptions V8_IF_USE_SIMULATOR(
FastCApiObject::AddAllFastCallbackNoOptionsPatch));
Expand Down
50 changes: 50 additions & 0 deletions deps/v8/test/mjsunit/compiler/fast-api-calls-8args.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Copyright 2021 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// This file tests fast callbacks with more than 8 arguments. It should
// fail on arm64 + OSX configuration, because of stack alignment issue,
// see crbug.com/v8/13171.

// Flags: --turbo-fast-api-calls --expose-fast-api --allow-natives-syntax --turbofan
// --always-turbofan is disabled because we rely on particular feedback for
// optimizing to the fastest path.
// Flags: --no-always-turbofan
// The test relies on optimizing/deoptimizing at predictable moments, so
// it's not suitable for deoptimization fuzzing.
// Flags: --deopt-every-n-times=0

const add_all_32bit_int_arg1 = -42;
const add_all_32bit_int_arg2 = 45;
const add_all_32bit_int_arg3 = -12345678;
const add_all_32bit_int_arg4 = 0x1fffffff;
const add_all_32bit_int_arg5 = 1e6;
const add_all_32bit_int_arg6 = 1e8;
const add_all_32bit_int_arg7 = 31;
const add_all_32bit_int_arg8 = 63;
const add_all_32bit_int_result_8args = add_all_32bit_int_arg1 +
add_all_32bit_int_arg2 + add_all_32bit_int_arg3 + add_all_32bit_int_arg4 +
add_all_32bit_int_arg5 + add_all_32bit_int_arg6 + add_all_32bit_int_arg7 + add_all_32bit_int_arg8;

const fast_c_api = new d8.test.FastCAPI();

(function () {
function overloaded_add_all(should_fallback = false) {
return fast_c_api.overloaded_add_all_8args(should_fallback,
add_all_32bit_int_arg1, add_all_32bit_int_arg2, add_all_32bit_int_arg3,
add_all_32bit_int_arg4, add_all_32bit_int_arg5, add_all_32bit_int_arg6,
add_all_32bit_int_arg7, add_all_32bit_int_arg8);
}

%PrepareFunctionForOptimization(overloaded_add_all);
let result = overloaded_add_all();
assertEquals(add_all_32bit_int_result_8args, result);

fast_c_api.reset_counts();
%OptimizeFunctionOnNextCall(overloaded_add_all);
result = overloaded_add_all();
assertOptimized(overloaded_add_all);

assertEquals(1, fast_c_api.fast_call_count());
assertEquals(0, fast_c_api.slow_call_count());
})();
8 changes: 8 additions & 0 deletions deps/v8/test/mjsunit/mjsunit.status
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,14 @@
'wasm/compare-exchange64-stress': [SKIP],
}], # 'system == macos'

##############################################################################
['system == macos and arch == arm64', {
# BUG(v8:13171): The following tests a function that shouldn't be optimized
# on M1 hardware, unless a proper fix for the stack corruption is
# implemented (see linked issue).
'compiler/fast-api-calls-8args': [FAIL],
}], # 'system == macos and arch == arm64'

##############################################################################
['system == windows', {
# Too slow with turbo fan.
Expand Down

0 comments on commit 5453cd9

Please sign in to comment.