From 71f1f0d48970a05bcb4311bf6aa801c0ad05ca5f Mon Sep 17 00:00:00 2001 From: Zhi An Ng Date: Thu, 25 Mar 2021 17:28:32 +0000 Subject: [PATCH] Revert "[fastcall] Add fast API testing facilities to d8" This reverts commit 9eba2d85f420933c9c97caebf357b257b00dc93f. Reason for revert: TSAN failures https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20TSAN%20-%20isolates/14265/overview Original change's description: > [fastcall] Add fast API testing facilities to d8 > > This CL provides the minimum necessary functionality to expose fast API > for testing in mjsunit, exposing the fast path for fuzzing. It exposes > a d8.test.fast_c_api with an `add_all` method, which exercises primitive > types. On x64, all integer and floating point types are supported. On > other platforms currently only 32-bit integers are included in the test. > > Design doc: > https://docs.google.com/document/d/1KUKPfXkSRZTA2gMwaWbpQKlYfw0C-T6AE3XzC4viHbo/ > > Bug: chromium:1052746 > Change-Id: Icc824199a26dd2abd2b869f5483a39d38e4dce3e > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2749154 > Reviewed-by: Camillo Bruni > Reviewed-by: Georg Neis > Reviewed-by: Sathya Gunasekaran > Commit-Queue: Maya Lekova > Cr-Commit-Position: refs/heads/master@{#73670} Bug: chromium:1052746 Change-Id: Iaf5083540ddfe882a747eaa9d1d2a2a8b4ba0ec0 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2787081 Auto-Submit: Zhi An Ng Commit-Queue: Rubber Stamper Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/heads/master@{#73673} --- BUILD.gn | 1 - include/v8-fast-api-calls.h | 1 - src/d8/d8-test.cc | 222 ------------------------ src/d8/d8.cc | 6 - src/d8/d8.h | 1 - test/mjsunit/compiler/fast-api-calls.js | 146 ---------------- test/mjsunit/mjsunit.status | 3 - 7 files changed, 380 deletions(-) delete mode 100644 src/d8/d8-test.cc delete mode 100644 test/mjsunit/compiler/fast-api-calls.js diff --git a/BUILD.gn b/BUILD.gn index 805bf930f7..f603de3aea 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -5488,7 +5488,6 @@ v8_executable("d8") { "src/d8/d8-js.cc", "src/d8/d8-platforms.cc", "src/d8/d8-platforms.h", - "src/d8/d8-test.cc", "src/d8/d8.cc", "src/d8/d8.h", ] diff --git a/include/v8-fast-api-calls.h b/include/v8-fast-api-calls.h index f8b5acb093..dc8a4a52cb 100644 --- a/include/v8-fast-api-calls.h +++ b/include/v8-fast-api-calls.h @@ -187,7 +187,6 @@ #include #include -#include #include #include "v8config.h" // NOLINT(build/include_directory) diff --git a/src/d8/d8-test.cc b/src/d8/d8-test.cc deleted file mode 100644 index 8c9181a8df..0000000000 --- a/src/d8/d8-test.cc +++ /dev/null @@ -1,222 +0,0 @@ -// 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. - -#include "src/d8/d8.h" - -#include "include/v8-fast-api-calls.h" - -// This file exposes a d8.test.fast_c_api object, which adds testing facility -// for writing mjsunit tests that exercise fast API calls. The fast_c_api object -// contains an `add_all` method with the following signature: -// double add_all(bool /*should_fallback*/, int32_t, uint32_t, -// int64_t, uint64_t, float, double), that is wired as a "fast API" method. -// The fast_c_api object also supports querying the number of fast/slow calls -// and resetting these counters. - -// Make sure to sync the following with src/compiler/globals.h. -#if defined(V8_TARGET_ARCH_X64) -#define V8_ENABLE_FP_PARAMS_IN_C_LINKAGE -#endif - -namespace v8 { -namespace { -class FastCApiObject { - public: - static double AddAllFastCallback(ApiObject receiver, bool should_fallback, - int32_t arg_i32, uint32_t arg_u32, - int64_t arg_i64, uint64_t arg_u64, - float arg_f32, double arg_f64, - FastApiCallbackOptions& options) { - Value* receiver_value = reinterpret_cast(&receiver); - CHECK(receiver_value->IsObject()); - FastCApiObject* self = UnwrapObject(Object::Cast(receiver_value)); - self->fast_call_count_++; - - if (should_fallback) { - options.fallback = 1; - return 0; - } - - return static_cast(arg_i32) + static_cast(arg_u32) + - static_cast(arg_i64) + static_cast(arg_u64) + - static_cast(arg_f32) + arg_f64; - } - static void AddAllSlowCallback(const FunctionCallbackInfo& args) { - Isolate* isolate = args.GetIsolate(); - - FastCApiObject* self = UnwrapObject(*args.This()); - self->slow_call_count_++; - - HandleScope handle_scope(isolate); - - double sum = 0; - if (args.Length() > 1) { - sum += args[1]->Int32Value(isolate->GetCurrentContext()).FromJust(); - } - if (args.Length() > 2) { - sum += args[2]->Uint32Value(isolate->GetCurrentContext()).FromJust(); - } - if (args.Length() > 3) { - sum += args[3]->IntegerValue(isolate->GetCurrentContext()).FromJust(); - } - if (args.Length() > 4) { - sum += args[4]->IntegerValue(isolate->GetCurrentContext()).FromJust(); - } - if (args.Length() > 5) { - sum += args[5]->NumberValue(isolate->GetCurrentContext()).FromJust(); - } else { - sum += std::numeric_limits::quiet_NaN(); - } - if (args.Length() > 6) { - sum += args[6]->NumberValue(isolate->GetCurrentContext()).FromJust(); - } else { - sum += std::numeric_limits::quiet_NaN(); - } - - args.GetReturnValue().Set(Number::New(isolate, sum)); - } - - static int Add32BitIntFastCallback(ApiObject receiver, bool should_fallback, - int32_t arg_i32, uint32_t arg_u32, - FastApiCallbackOptions& options) { - Value* receiver_value = reinterpret_cast(&receiver); - CHECK(receiver_value->IsObject()); - FastCApiObject* self = UnwrapObject(Object::Cast(receiver_value)); - self->fast_call_count_++; - - if (should_fallback) { - options.fallback = 1; - return 0; - } - - return arg_i32 + arg_u32; - } - static void Add32BitIntSlowCallback(const FunctionCallbackInfo& args) { - Isolate* isolate = args.GetIsolate(); - - FastCApiObject* self = UnwrapObject(*args.This()); - self->slow_call_count_++; - - HandleScope handle_scope(isolate); - - double sum = 0; - if (args.Length() > 1) { - sum += args[1]->Int32Value(isolate->GetCurrentContext()).FromJust(); - } - if (args.Length() > 2) { - sum += args[2]->Uint32Value(isolate->GetCurrentContext()).FromJust(); - } - - args.GetReturnValue().Set(Number::New(isolate, sum)); - } - - static void FastCallCount(const FunctionCallbackInfo& args) { - FastCApiObject* self = UnwrapObject(*args.This()); - args.GetReturnValue().Set( - Number::New(args.GetIsolate(), self->fast_call_count())); - } - static void SlowCallCount(const FunctionCallbackInfo& args) { - FastCApiObject* self = UnwrapObject(*args.This()); - args.GetReturnValue().Set( - Number::New(args.GetIsolate(), self->slow_call_count())); - } - static void ResetCounts(const FunctionCallbackInfo& args) { - FastCApiObject* self = UnwrapObject(*args.This()); - self->reset_counts(); - args.GetReturnValue().Set(Undefined(args.GetIsolate())); - } - static void SupportsFPParams(const FunctionCallbackInfo& info) { - FastCApiObject* self = UnwrapObject(*info.This()); - info.GetReturnValue().Set(self->supports_fp_params_); - } - - int fast_call_count() const { return fast_call_count_; } - int slow_call_count() const { return slow_call_count_; } - void reset_counts() { - fast_call_count_ = 0; - slow_call_count_ = 0; - } - - static const int kV8WrapperObjectIndex = 1; - - private: - static FastCApiObject* UnwrapObject(Object* object) { - i::Address addr = *reinterpret_cast(object); - auto instance_type = i::Internals::GetInstanceType(addr); - if (instance_type != i::Internals::kJSObjectType && - instance_type != i::Internals::kJSApiObjectType && - instance_type != i::Internals::kJSSpecialApiObjectType) { - return nullptr; - } - FastCApiObject* wrapped = reinterpret_cast( - object->GetAlignedPointerFromInternalField(kV8WrapperObjectIndex)); - CHECK_NOT_NULL(wrapped); - return wrapped; - } - int fast_call_count_ = 0, slow_call_count_ = 0; -#ifdef V8_ENABLE_FP_PARAMS_IN_C_LINKAGE - bool supports_fp_params_ = true; -#else // V8_ENABLE_FP_PARAMS_IN_C_LINKAGE - bool supports_fp_params_ = false; -#endif // V8_ENABLE_FP_PARAMS_IN_C_LINKAGE -}; - -// The object is statically initialized for simplicity, typically the embedder -// will take care of managing their C++ objects lifetime. -FastCApiObject kFastCApiObject; -} // namespace - -void CreateObject(const FunctionCallbackInfo& info) { - Local api_object = info.Holder(); - api_object->SetAlignedPointerInInternalField( - FastCApiObject::kV8WrapperObjectIndex, - reinterpret_cast(&kFastCApiObject)); - api_object->SetAccessorProperty( - String::NewFromUtf8Literal(info.GetIsolate(), "supports_fp_params"), - FunctionTemplate::New(info.GetIsolate(), FastCApiObject::SupportsFPParams) - ->GetFunction(api_object->GetCreationContext().ToLocalChecked()) - .ToLocalChecked()); -} - -Local Shell::CreateTestFastCApiTemplate(Isolate* isolate) { - Local api_obj_ctor = - FunctionTemplate::New(isolate, CreateObject); - Local signature = Signature::New(isolate, api_obj_ctor); - { - CFunction add_all_c_func = - CFunction::Make(FastCApiObject::AddAllFastCallback); - api_obj_ctor->PrototypeTemplate()->Set( - isolate, "add_all", - FunctionTemplate::New(isolate, FastCApiObject::AddAllSlowCallback, - Local(), signature, 1, - ConstructorBehavior::kThrow, - SideEffectType::kHasSideEffect, &add_all_c_func)); - CFunction add_32bit_int_c_func = - CFunction::Make(FastCApiObject::Add32BitIntFastCallback); - api_obj_ctor->PrototypeTemplate()->Set( - isolate, "add_32bit_int", - FunctionTemplate::New( - isolate, FastCApiObject::Add32BitIntSlowCallback, Local(), - signature, 1, ConstructorBehavior::kThrow, - SideEffectType::kHasSideEffect, &add_32bit_int_c_func)); - api_obj_ctor->PrototypeTemplate()->Set( - isolate, "fast_call_count", - FunctionTemplate::New(isolate, FastCApiObject::FastCallCount, - Local(), signature)); - api_obj_ctor->PrototypeTemplate()->Set( - isolate, "slow_call_count", - FunctionTemplate::New(isolate, FastCApiObject::SlowCallCount, - Local(), signature)); - api_obj_ctor->PrototypeTemplate()->Set( - isolate, "reset_counts", - FunctionTemplate::New(isolate, FastCApiObject::ResetCounts, - Local(), signature)); - } - api_obj_ctor->InstanceTemplate()->SetInternalFieldCount( - FastCApiObject::kV8WrapperObjectIndex + 1); - - return api_obj_ctor; -} - -} // namespace v8 diff --git a/src/d8/d8.cc b/src/d8/d8.cc index 8346a6c1b9..1775dde327 100644 --- a/src/d8/d8.cc +++ b/src/d8/d8.cc @@ -2662,7 +2662,6 @@ Local Shell::CreateTestRunnerTemplate(Isolate* isolate) { // installed on the global object can be hidden with the --omit-quit flag // (e.g. on asan bots). test_template->Set(isolate, "quit", FunctionTemplate::New(isolate, Quit)); - return test_template; } @@ -2718,11 +2717,6 @@ Local Shell::CreateD8Template(Isolate* isolate) { test_template->Set( isolate, "verifySourcePositions", FunctionTemplate::New(isolate, TestVerifySourcePositions)); - if (i::FLAG_turbo_fast_api_calls) { - test_template->Set(isolate, "fast_c_api", - Shell::CreateTestFastCApiTemplate(isolate)); - } - d8_template->Set(isolate, "test", test_template); } return d8_template; diff --git a/src/d8/d8.h b/src/d8/d8.h index d5f13817d3..e5bdc3e25c 100644 --- a/src/d8/d8.h +++ b/src/d8/d8.h @@ -632,7 +632,6 @@ class Shell : public i::AllStatic { static Local CreatePerformanceTemplate(Isolate* isolate); static Local CreateRealmTemplate(Isolate* isolate); static Local CreateD8Template(Isolate* isolate); - static Local CreateTestFastCApiTemplate(Isolate* isolate); static MaybeLocal CreateRealm( const v8::FunctionCallbackInfo& args, int index, diff --git a/test/mjsunit/compiler/fast-api-calls.js b/test/mjsunit/compiler/fast-api-calls.js deleted file mode 100644 index 002f71115d..0000000000 --- a/test/mjsunit/compiler/fast-api-calls.js +++ /dev/null @@ -1,146 +0,0 @@ -// 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 excercises basic fast API calls and enables fuzzing of this -// functionality. - -// Flags: --turbo-fast-api-calls --allow-natives-syntax --opt -// --always-opt is disabled because we rely on particular feedback for -// optimizing to the fastest path. -// Flags: --no-always-opt -const fast_c_api = new d8.test.fast_c_api(); - -// ----------- add_all ----------- -// `add_all` has the following signature: -// double add_all(bool /*should_fallback*/, int32_t, uint32_t, -// int64_t, uint64_t, float, double) - -const max_safe_float = 2**24 - 1; -const add_all_result = -42 + 45 + Number.MIN_SAFE_INTEGER + Number.MAX_SAFE_INTEGER + - max_safe_float * 0.5 + Math.PI; - -function add_all(should_fallback = false) { - return fast_c_api.add_all(should_fallback, - -42, 45, Number.MIN_SAFE_INTEGER, Number.MAX_SAFE_INTEGER, - max_safe_float * 0.5, Math.PI); -} - -%PrepareFunctionForOptimization(add_all); -assertEquals(add_all_result, add_all()); -%OptimizeFunctionOnNextCall(add_all); - -if (fast_c_api.supports_fp_params) { - // Test that regular call hits the fast path. - fast_c_api.reset_counts(); - assertEquals(add_all_result, add_all()); - assertEquals(1, fast_c_api.fast_call_count()); - assertEquals(0, fast_c_api.slow_call_count()); - - // Test fallback to slow path. - fast_c_api.reset_counts(); - assertEquals(add_all_result, add_all(true)); - assertEquals(1, fast_c_api.fast_call_count()); - assertEquals(1, fast_c_api.slow_call_count()); - - // Test that no fallback hits the fast path again. - fast_c_api.reset_counts(); - assertEquals(add_all_result, add_all()); - assertEquals(1, fast_c_api.fast_call_count()); - assertEquals(0, fast_c_api.slow_call_count()); -} else { - // Test that calling with unsupported types hits the slow path. - fast_c_api.reset_counts(); - assertEquals(add_all_result, add_all()); - assertEquals(0, fast_c_api.fast_call_count()); - assertEquals(1, fast_c_api.slow_call_count()); -} - -// ----------- Test add_all signature mismatche ----------- -function add_all_mismatch() { - return fast_c_api.add_all(false /*should_fallback*/, - 45, -42, Number.MAX_SAFE_INTEGER, max_safe_float * 0.5, - Number.MIN_SAFE_INTEGER, Math.PI); -} - -%PrepareFunctionForOptimization(add_all_mismatch); -const add_all_mismatch_result = add_all_mismatch(); -%OptimizeFunctionOnNextCall(add_all_mismatch); - -fast_c_api.reset_counts(); -assertEquals(add_all_mismatch_result, add_all_mismatch()); -assertEquals(1, fast_c_api.slow_call_count()); -assertEquals(0, fast_c_api.fast_call_count()); -// If the function was ever optimized to the fast path, it should -// have been deoptimized due to the argument types mismatch. If it -// wasn't optimized due to lack of support for FP params, it will -// stay optimized. -if (fast_c_api.supports_fp_params) { - assertUnoptimized(add_all_mismatch); -} - -// ----------- add_32bit_int ----------- -// `add_32bit_int` has the following signature: -// int add_32bit_int(bool /*should_fallback*/, int32_t, uint32_t) - -const add_32bit_int_result = -42 + 45; - -function add_32bit_int(should_fallback = false) { - return fast_c_api.add_32bit_int(should_fallback, -42, 45); -} - -%PrepareFunctionForOptimization(add_32bit_int); -assertEquals(add_32bit_int_result, add_32bit_int()); -%OptimizeFunctionOnNextCall(add_32bit_int); - -// Test that regular call hits the fast path. -fast_c_api.reset_counts(); -assertEquals(add_32bit_int_result, add_32bit_int()); -assertEquals(1, fast_c_api.fast_call_count()); -assertEquals(0, fast_c_api.slow_call_count()); - -// Test fallback to slow path. -fast_c_api.reset_counts(); -assertEquals(add_32bit_int_result, add_32bit_int(true)); -assertEquals(1, fast_c_api.fast_call_count()); -assertEquals(1, fast_c_api.slow_call_count()); - -// Test that no fallback hits the fast path again. -fast_c_api.reset_counts(); -assertEquals(add_32bit_int_result, add_32bit_int()); -assertEquals(1, fast_c_api.fast_call_count()); -assertEquals(0, fast_c_api.slow_call_count()); - -// ----------- Test various signature mismatches ----------- -function add_32bit_int_mismatch(arg0, arg1, arg2, arg3) { - return fast_c_api.add_32bit_int(arg0, arg1, arg2, arg3); -} - -%PrepareFunctionForOptimization(add_32bit_int_mismatch); -assertEquals(add_32bit_int_result, add_32bit_int_mismatch(false, -42, 45)); -%OptimizeFunctionOnNextCall(add_32bit_int_mismatch); - -// Test that passing extra argument stays on the fast path. -fast_c_api.reset_counts(); -assertEquals(add_32bit_int_result, add_32bit_int_mismatch(false, -42, 45, -42)); -assertEquals(1, fast_c_api.fast_call_count()); - -// Test that passing wrong argument types stays on the fast path. -fast_c_api.reset_counts(); -assertEquals(Math.round(-42 + 3.14), add_32bit_int_mismatch(false, -42, 3.14)); -assertEquals(1, fast_c_api.fast_call_count()); - -// Test that passing too few argument falls down the slow path, -// because it's an argument type mismatch (undefined vs. int). -fast_c_api.reset_counts(); -assertEquals(-42, add_32bit_int_mismatch(false, -42)); -assertEquals(1, fast_c_api.slow_call_count()); -assertEquals(0, fast_c_api.fast_call_count()); -assertUnoptimized(add_32bit_int_mismatch); - -// Test that the function can be optimized again. -%PrepareFunctionForOptimization(add_32bit_int_mismatch); -%OptimizeFunctionOnNextCall(add_32bit_int_mismatch); -fast_c_api.reset_counts(); -assertEquals(add_32bit_int_result, add_32bit_int_mismatch(false, -42, 45)); -assertEquals(1, fast_c_api.fast_call_count()); diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index bbf6bc0fd2..b1425cb73d 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -350,9 +350,6 @@ 'regexp-tier-up-multiple': [SKIP], 'regress/regress-996234': [SKIP], - # This test relies on TurboFan being enabled. - 'compiler/fast-api-calls': [SKIP], - # These tests check that we can trace the compiler. 'tools/compiler-trace-flags': [SKIP],