From b5ab5c7b759aa6afacaa55ee1e343bf8af859ca0 Mon Sep 17 00:00:00 2001 From: Tobias Tebbi Date: Tue, 28 Apr 2020 14:27:43 +0200 Subject: [PATCH] [torque] allow builtins without context parameter Bug: v8:10404, v8:7793 Change-Id: I7ed5fc790bd97af0dd3671669779e416101731ce Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2162877 Commit-Queue: Tobias Tebbi Reviewed-by: Jakob Gruber Cr-Commit-Position: refs/heads/master@{#67435} --- src/builtins/base.tq | 4 +- src/builtins/iterator.tq | 2 - src/builtins/typed-array-sort.tq | 2 +- src/builtins/typed-array.tq | 11 +++-- src/codegen/interface-descriptors.h | 6 ++- src/torque/csa-generator.cc | 60 +++++++++++++++++----------- src/torque/declaration-visitor.cc | 4 ++ src/torque/implementation-visitor.cc | 33 ++++++++------- src/torque/types.cc | 15 +++++++ src/torque/types.h | 3 ++ test/cctest/torque/test-torque.cc | 6 +-- test/torque/test-torque.tq | 40 +++++++++---------- 12 files changed, 108 insertions(+), 78 deletions(-) diff --git a/src/builtins/base.tq b/src/builtins/base.tq index 430bd4bf44..915dd99968 100644 --- a/src/builtins/base.tq +++ b/src/builtins/base.tq @@ -494,9 +494,9 @@ extern transitioning macro ToThisValue(implicit context: Context)( extern transitioning macro GetProperty(implicit context: Context)( JSAny, JSAny): JSAny; extern transitioning builtin SetProperty(implicit context: Context)( - JSAny, JSAny, JSAny); + JSAny, JSAny, JSAny): JSAny; extern transitioning builtin SetPropertyInLiteral(implicit context: Context)( - JSAny, JSAny, JSAny); + JSAny, JSAny, JSAny): JSAny; extern transitioning builtin DeleteProperty(implicit context: Context)( JSAny, JSAny | PrivateSymbol, LanguageModeSmi): Boolean; extern transitioning builtin HasProperty(implicit context: Context)( diff --git a/src/builtins/iterator.tq b/src/builtins/iterator.tq index 9a7fc7e332..3adae54b07 100644 --- a/src/builtins/iterator.tq +++ b/src/builtins/iterator.tq @@ -46,8 +46,6 @@ namespace iterator { extern macro IteratorBuiltinsAssembler::StringListFromIterable( implicit context: Context)(JSAny): JSArray; - extern builtin IterableToListMayPreserveHoles(implicit context: - Context)(JSAny, JSAny); extern builtin IterableToListWithSymbolLookup(implicit context: Context)(JSAny): JSArray; extern builtin IterableToFixedArrayWithSymbolLookupSlow( diff --git a/src/builtins/typed-array-sort.tq b/src/builtins/typed-array-sort.tq index 0c27aaf3c4..55bfb62680 100644 --- a/src/builtins/typed-array-sort.tq +++ b/src/builtins/typed-array-sort.tq @@ -127,7 +127,7 @@ namespace typed_array { const work2: FixedArray = AllocateZeroedFixedArray(Convert(len)); for (let i: uintptr = 0; i < len; ++i) { - const element: Numeric = accessor.LoadNumeric(context, array, i); + const element: Numeric = accessor.LoadNumeric(array, i); work1.objects[i] = element; work2.objects[i] = element; } diff --git a/src/builtins/typed-array.tq b/src/builtins/typed-array.tq index 1b23d3f572..913c178948 100644 --- a/src/builtins/typed-array.tq +++ b/src/builtins/typed-array.tq @@ -81,7 +81,7 @@ namespace typed_array { Context, JSTypedArray, uintptr, JSAny, constexpr ElementsKind) labels IfDetached; - type LoadNumericFn = builtin(Context, JSTypedArray, uintptr) => Numeric; + type LoadNumericFn = builtin(JSTypedArray, uintptr) => Numeric; type StoreNumericFn = builtin(Context, JSTypedArray, uintptr, Numeric) => Smi; type StoreJSAnyFn = builtin(Context, JSTypedArray, uintptr, JSAny) => Smi; @@ -90,10 +90,9 @@ namespace typed_array { const kStoreFailureArrayDetached: Smi = 1; struct TypedArrayAccessor { - macro LoadNumeric( - context: Context, array: JSTypedArray, index: uintptr): Numeric { + macro LoadNumeric(array: JSTypedArray, index: uintptr): Numeric { const loadfn: LoadNumericFn = this.loadNumericFn; - return loadfn(context, array, index); + return loadfn(array, index); } macro StoreNumeric( @@ -189,7 +188,7 @@ namespace typed_array { macro Load(implicit context: Context)(k: uintptr): JSAny { const lf: LoadNumericFn = this.loadfn; - return lf(context, this.unstable, k); + return lf(this.unstable, k); } stable: JSTypedArray; @@ -245,7 +244,7 @@ namespace typed_array { } builtin LoadTypedElement( - _context: Context, array: JSTypedArray, index: uintptr): Numeric { + array: JSTypedArray, index: uintptr): Numeric { return LoadFixedTypedArrayElementAsTagged( array.data_ptr, index, KindForArrayType()); } diff --git a/src/codegen/interface-descriptors.h b/src/codegen/interface-descriptors.h index b1bd6b505d..75bb922e4d 100644 --- a/src/codegen/interface-descriptors.h +++ b/src/codegen/interface-descriptors.h @@ -520,10 +520,12 @@ class V8_EXPORT_PRIVATE VoidDescriptor : public CallInterfaceDescriptor { }; // This class is subclassed by Torque-generated call interface descriptors. -template +template class TorqueInterfaceDescriptor : public CallInterfaceDescriptor { public: - static constexpr int kDescriptorFlags = CallInterfaceDescriptorData::kNoFlags; + static constexpr int kDescriptorFlags = + has_context_parameter ? CallInterfaceDescriptorData::kNoFlags + : CallInterfaceDescriptorData::kNoContext; static constexpr int kParameterCount = parameter_count; enum ParameterIndices { kContext = kParameterCount }; template diff --git a/src/torque/csa-generator.cc b/src/torque/csa-generator.cc index 8b4d2068d3..45ed7f3af4 100644 --- a/src/torque/csa-generator.cc +++ b/src/torque/csa-generator.cc @@ -512,8 +512,14 @@ void CSAGenerator::EmitInstruction(const CallBuiltinInstruction& instruction, LowerType(instruction.builtin->signature().return_type); if (instruction.is_tailcall) { out() << " CodeStubAssembler(state_).TailCallBuiltin(Builtins::k" - << instruction.builtin->ExternalName() << ", "; - PrintCommaSeparatedList(out(), arguments); + << instruction.builtin->ExternalName(); + if (!instruction.builtin->signature().HasContextParameter()) { + // Add dummy context parameter to satisfy the TailCallBuiltin signature. + out() << ", TNode()"; + } + for (const std::string& argument : arguments) { + out() << ", " << argument; + } out() << ");\n"; } else { std::string result_name; @@ -525,25 +531,24 @@ void CSAGenerator::EmitInstruction(const CallBuiltinInstruction& instruction, std::string catch_name = PreCallableExceptionPreparation(instruction.catch_block); Stack pre_call_stack = *stack; - if (result_types.size() == 1) { - std::string generated_type = result_types[0]->GetGeneratedTNodeTypeName(); - stack->Push(result_name); - out() << " " << result_name << " = "; - if (generated_type != "Object") out() << "TORQUE_CAST("; - out() << "CodeStubAssembler(state_).CallBuiltin(Builtins::k" - << instruction.builtin->ExternalName() << ", "; - PrintCommaSeparatedList(out(), arguments); - if (generated_type != "Object") out() << ")"; - out() << ");\n"; - } else { - DCHECK_EQ(0, result_types.size()); - // TODO(tebbi): Actually, builtins have to return a value, so we should - // not have to handle this case. - out() << " CodeStubAssembler(state_).CallBuiltin(Builtins::k" - << instruction.builtin->ExternalName() << ", "; - PrintCommaSeparatedList(out(), arguments); - out() << ");\n"; + + DCHECK_EQ(1, result_types.size()); + std::string generated_type = result_types[0]->GetGeneratedTNodeTypeName(); + stack->Push(result_name); + out() << " " << result_name << " = "; + if (generated_type != "Object") out() << "TORQUE_CAST("; + out() << "CodeStubAssembler(state_).CallBuiltin(Builtins::k" + << instruction.builtin->ExternalName(); + if (!instruction.builtin->signature().HasContextParameter()) { + // Add dummy context parameter to satisfy the CallBuiltin signature. + out() << ", TNode()"; } + for (const std::string& argument : arguments) { + out() << ", " << argument; + } + if (generated_type != "Object") out() << ")"; + out() << ");\n"; + PostCallableExceptionPreparation( catch_name, result_types.size() == 0 ? TypeOracle::GetVoidType() : result_types[0], @@ -555,8 +560,8 @@ void CSAGenerator::EmitInstruction(const CallBuiltinInstruction& instruction, void CSAGenerator::EmitInstruction( const CallBuiltinPointerInstruction& instruction, Stack* stack) { - std::vector function_and_arguments = - stack->PopMany(1 + instruction.argc); + std::vector arguments = stack->PopMany(instruction.argc); + std::string function = stack->Pop(); std::vector result_types = LowerType(instruction.type->return_type()); if (result_types.size() != 1) { @@ -576,8 +581,15 @@ void CSAGenerator::EmitInstruction( "CallableFor(ca_." "isolate()," "ExampleBuiltinForTorqueFunctionPointerType(" - << instruction.type->function_pointer_type_id() << ")).descriptor(), "; - PrintCommaSeparatedList(out(), function_and_arguments); + << instruction.type->function_pointer_type_id() << ")).descriptor(), " + << function; + if (!instruction.type->HasContextParameter()) { + // Add dummy context parameter to satisfy the CallBuiltinPointer signature. + out() << ", TNode()"; + } + for (const std::string& argument : arguments) { + out() << ", " << argument; + } out() << ")"; if (generated_type != "Object") out() << ")"; out() << ";\n"; diff --git a/src/torque/declaration-visitor.cc b/src/torque/declaration-visitor.cc index 5e3c8bbcb2..6ec1b74df5 100644 --- a/src/torque/declaration-visitor.cc +++ b/src/torque/declaration-visitor.cc @@ -103,6 +103,10 @@ Builtin* DeclarationVisitor::CreateBuiltin(BuiltinDeclaration* decl, *signature.return_type, "."); } + if (signature.return_type == TypeOracle::GetVoidType()) { + Error("Builtins cannot have return type void."); + } + return Declarations::CreateBuiltin(std::move(external_name), std::move(readable_name), kind, std::move(signature), body); diff --git a/src/torque/implementation-visitor.cc b/src/torque/implementation-visitor.cc index dad2d8751f..1f23b4e59b 100644 --- a/src/torque/implementation-visitor.cc +++ b/src/torque/implementation-visitor.cc @@ -530,17 +530,8 @@ void ImplementationVisitor::Visit(Builtin* builtin) { } else { DCHECK(builtin->IsStub()); - // Context - const bool context_is_implicit = signature.implicit_count > 0; - std::string parameter0 = - AddParameter(0, builtin, ¶meters, ¶meter_types, - ¶meter_bindings, context_is_implicit); - source_out() << " TNode " << parameter0 - << " = UncheckedCast(Parameter(" - << "Descriptor::kContext));\n"; - source_out() << " USE(" << parameter0 << ");\n"; - - for (size_t i = 1; i < signature.parameter_names.size(); ++i) { + bool has_context_parameter = signature.HasContextParameter(); + for (size_t i = 0; i < signature.parameter_names.size(); ++i) { const Type* type = signature.types()[i]; const bool mark_as_used = signature.implicit_count > i; std::string var = AddParameter(i, builtin, ¶meters, ¶meter_types, @@ -548,8 +539,14 @@ void ImplementationVisitor::Visit(Builtin* builtin) { source_out() << " " << type->GetGeneratedTypeName() << " " << var << " = " << "UncheckedCast<" << type->GetGeneratedTNodeTypeName() - << ">(Parameter(Descriptor::ParameterIndex<" << (i - 1) - << ">()));\n"; + << ">(Parameter("; + if (i == 0 && has_context_parameter) { + source_out() << "Descriptor::kContext"; + } else { + source_out() << "Descriptor::ParameterIndex<" + << (has_context_parameter ? i - 1 : i) << ">()"; + } + source_out() << "));\n"; source_out() << " USE(" << var << ");\n"; } } @@ -2980,13 +2977,15 @@ void ImplementationVisitor::GenerateBuiltinDefinitionsAndInterfaceDescriptors( builtin_definitions << "TFC(" << builtin->ExternalName() << ", " << builtin->ExternalName(); std::string descriptor_name = builtin->ExternalName() + "Descriptor"; - constexpr size_t kFirstNonContextParameter = 1; + bool has_context_parameter = builtin->signature().HasContextParameter(); + size_t kFirstNonContextParameter = has_context_parameter ? 1 : 0; size_t parameter_count = builtin->parameter_names().size() - kFirstNonContextParameter; - interface_descriptors << "class " << descriptor_name - << " : public TorqueInterfaceDescriptor<" - << parameter_count << "> {\n"; + interface_descriptors + << "class " << descriptor_name + << " : public TorqueInterfaceDescriptor<" << parameter_count << ", " + << (has_context_parameter ? "true" : "false") << "> {\n"; interface_descriptors << " DECLARE_DESCRIPTOR_WITH_BASE(" << descriptor_name << ", TorqueInterfaceDescriptor)\n"; diff --git a/src/torque/types.cc b/src/torque/types.cc index 9800d0a941..bf69868796 100644 --- a/src/torque/types.cc +++ b/src/torque/types.cc @@ -705,6 +705,21 @@ bool Signature::HasSameTypesAs(const Signature& other, return true; } +namespace { +bool FirstTypeIsContext(const std::vector parameter_types) { + return !parameter_types.empty() && + parameter_types[0] == TypeOracle::GetContextType(); +} +} // namespace + +bool Signature::HasContextParameter() const { + return FirstTypeIsContext(types()); +} + +bool BuiltinPointerType::HasContextParameter() const { + return FirstTypeIsContext(parameter_types()); +} + bool IsAssignableFrom(const Type* to, const Type* from) { if (to == from) return true; if (from->IsSubtypeOf(to)) return true; diff --git a/src/torque/types.h b/src/torque/types.h index 02c52edc8c..357a653ffd 100644 --- a/src/torque/types.h +++ b/src/torque/types.h @@ -347,6 +347,8 @@ class V8_EXPORT_PRIVATE BuiltinPointerType final : public Type { return {{"Smi", ""}}; } + bool HasContextParameter() const; + private: friend class TypeOracle; BuiltinPointerType(const Type* parent, TypeVector parameter_types, @@ -800,6 +802,7 @@ struct Signature { return TypeVector(parameter_types.types.begin() + implicit_count, parameter_types.types.end()); } + bool HasContextParameter() const; }; void PrintSignature(std::ostream& os, const Signature& sig, bool with_names); diff --git a/test/cctest/torque/test-torque.cc b/test/cctest/torque/test-torque.cc index 54bff6f43c..c52f97b93f 100644 --- a/test/cctest/torque/test-torque.cc +++ b/test/cctest/torque/test-torque.cc @@ -113,8 +113,7 @@ TEST(TestBuiltinSpecialization) { CodeAssemblerTester asm_tester(isolate, 0); TestTorqueAssembler m(asm_tester.state()); { - TNode temp = m.SmiConstant(0); - m.TestBuiltinSpecialization(m.UncheckedCast(temp)); + m.TestBuiltinSpecialization(); m.Return(m.UndefinedConstant()); } FunctionTester ft(asm_tester.GenerateCode(), 0); @@ -170,8 +169,7 @@ TEST(TestFunctionPointerToGeneric) { CodeAssemblerTester asm_tester(isolate, 0); TestTorqueAssembler m(asm_tester.state()); { - TNode temp = m.SmiConstant(0); - m.TestFunctionPointerToGeneric(m.UncheckedCast(temp)); + m.TestFunctionPointerToGeneric(); m.Return(m.UndefinedConstant()); } FunctionTester ft(asm_tester.GenerateCode(), 0); diff --git a/test/torque/test-torque.tq b/test/torque/test-torque.tq index 7d9f5c01f5..e7c4aa472a 100644 --- a/test/torque/test-torque.tq +++ b/test/torque/test-torque.tq @@ -97,20 +97,20 @@ namespace test { } } - builtin GenericBuiltinTest(_c: Context, _param: T): JSAny { + builtin GenericBuiltinTest(_param: T): JSAny { return Null; } - GenericBuiltinTest(_c: Context, param: JSAny): JSAny { + GenericBuiltinTest(param: JSAny): JSAny { return param; } @export - macro TestBuiltinSpecialization(c: Context) { - check(GenericBuiltinTest(c, 0) == Null); - check(GenericBuiltinTest(c, 1) == Null); - check(GenericBuiltinTest(c, Undefined) == Undefined); - check(GenericBuiltinTest(c, Undefined) == Undefined); + macro TestBuiltinSpecialization() { + check(GenericBuiltinTest(0) == Null); + check(GenericBuiltinTest(1) == Null); + check(GenericBuiltinTest(Undefined) == Undefined); + check(GenericBuiltinTest(Undefined) == Undefined); } macro LabelTestHelper4(flag: constexpr bool): never @@ -185,19 +185,19 @@ namespace test { } } - builtin TestHelperPlus1(_context: Context, x: Smi): Smi { + builtin TestHelperPlus1(x: Smi): Smi { return x + 1; } - builtin TestHelperPlus2(_context: Context, x: Smi): Smi { + builtin TestHelperPlus2(x: Smi): Smi { return x + 2; } @export macro TestFunctionPointers(implicit context: Context)(): Boolean { - let fptr: builtin(Context, Smi) => Smi = TestHelperPlus1; - check(fptr(context, 42) == 43); + let fptr: builtin(Smi) => Smi = TestHelperPlus1; + check(fptr(42) == 43); fptr = TestHelperPlus2; - check(fptr(context, 42) == 44); + check(fptr(42) == 44); return True; } @@ -215,14 +215,14 @@ namespace test { } @export - macro TestFunctionPointerToGeneric(c: Context) { - const fptr1: builtin(Context, Smi) => JSAny = GenericBuiltinTest; - const fptr2: builtin(Context, JSAny) => JSAny = GenericBuiltinTest; + macro TestFunctionPointerToGeneric() { + const fptr1: builtin(Smi) => JSAny = GenericBuiltinTest; + const fptr2: builtin(JSAny) => JSAny = GenericBuiltinTest; - check(fptr1(c, 0) == Null); - check(fptr1(c, 1) == Null); - check(fptr2(c, Undefined) == Undefined); - check(fptr2(c, Undefined) == Undefined); + check(fptr1(0) == Null); + check(fptr1(1) == Null); + check(fptr2(Undefined) == Undefined); + check(fptr2(Undefined) == Undefined); } type ObjectToObject = builtin(Context, JSAny) => JSAny; @@ -236,7 +236,7 @@ namespace test { if (TaggedIsSmi(n)) { const m: Smi = UnsafeCast(n); - check(TestHelperPlus1(context, m) == 11); + check(TestHelperPlus1(m) == 11); return True; } return False;