From 6b5f42082825f6d2f861fc1493b544129458a842 Mon Sep 17 00:00:00 2001 From: Seth Brenith Date: Tue, 8 Dec 2020 07:11:37 -0800 Subject: [PATCH] [torque] Make runtime macros inlinable Currently, all runtime C++ code generated for Torque macros all goes into a single .cc file and corresponding header. This is simple, but limits how we can use that generated code. For example, field accessors are generally expected to be inlinable at compilation time (not relying on LTO). This change updates the Torque compiler to output runtime C++ code into the same *-tq-inl.inc files that contain implementations of member functions for generated classes. All Torque macros transitively called from the top-level macros are included in the same file, to avoid any need for these generated files to #include each other. These macros are emitted within per-file namespaces to avoid multiple-definition build failures. Bug: v8:7793 Change-Id: Ic9ac3748c5020a05304773a66d7249efdc56b080 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2565067 Commit-Queue: Seth Brenith Reviewed-by: Tobias Tebbi Cr-Commit-Position: refs/heads/master@{#71664} --- BUILD.gn | 3 - src/diagnostics/objects-debug.cc | 1 - src/objects/heap-object.h | 1 + src/objects/ordered-hash-table-inl.h | 2 + src/torque/global-context.h | 30 +++++-- src/torque/implementation-visitor.cc | 116 +++++++++++---------------- src/torque/implementation-visitor.h | 44 +++------- src/torque/runtime-macro-shims.h | 10 ++- src/torque/torque-compiler.cc | 2 - src/torque/types.cc | 3 +- 10 files changed, 94 insertions(+), 118 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index 57e9ef10ce..b78ca36b5e 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1400,8 +1400,6 @@ template("run_torque") { "$target_gen_dir/torque-generated/exported-macros-assembler.h", "$target_gen_dir/torque-generated/csa-types.h", "$target_gen_dir/torque-generated/instance-types.h", - "$target_gen_dir/torque-generated/runtime-macros.cc", - "$target_gen_dir/torque-generated/runtime-macros.h", "$target_gen_dir/torque-generated/class-forward-declarations.h", ] @@ -1522,7 +1520,6 @@ v8_source_set("torque_generated_definitions") { "$target_gen_dir/torque-generated/class-verifiers.h", "$target_gen_dir/torque-generated/factory.cc", "$target_gen_dir/torque-generated/objects-printer.cc", - "$target_gen_dir/torque-generated/runtime-macros.cc", ] foreach(file, torque_files) { filetq = string_replace(file, ".tq", "-tq") diff --git a/src/diagnostics/objects-debug.cc b/src/diagnostics/objects-debug.cc index eb9e4c4ac1..dbf2809347 100644 --- a/src/diagnostics/objects-debug.cc +++ b/src/diagnostics/objects-debug.cc @@ -76,7 +76,6 @@ #include "src/utils/ostreams.h" #include "src/wasm/wasm-objects-inl.h" #include "torque-generated/class-verifiers.h" -#include "torque-generated/runtime-macros.h" namespace v8 { namespace internal { diff --git a/src/objects/heap-object.h b/src/objects/heap-object.h index de93ac99ac..b412d7c733 100644 --- a/src/objects/heap-object.h +++ b/src/objects/heap-object.h @@ -10,6 +10,7 @@ #include "src/objects/objects.h" #include "src/objects/tagged-field.h" #include "src/roots/roots.h" +#include "src/torque/runtime-macro-shims.h" // Has to be the last include (doesn't have include guards): #include "src/objects/object-macros.h" diff --git a/src/objects/ordered-hash-table-inl.h b/src/objects/ordered-hash-table-inl.h index 8a1bad9f62..3aef224e57 100644 --- a/src/objects/ordered-hash-table-inl.h +++ b/src/objects/ordered-hash-table-inl.h @@ -20,6 +20,8 @@ namespace v8 { namespace internal { +#include "torque-generated/src/objects/ordered-hash-table-tq-inl.inc" + CAST_ACCESSOR(OrderedNameDictionary) CAST_ACCESSOR(SmallOrderedNameDictionary) CAST_ACCESSOR(OrderedHashMap) diff --git a/src/torque/global-context.h b/src/torque/global-context.h index 7ccbc851c6..cd6ddef8b2 100644 --- a/src/torque/global-context.h +++ b/src/torque/global-context.h @@ -60,14 +60,28 @@ class GlobalContext : public ContextualClass { } struct PerFileStreams { + PerFileStreams() : file(SourceId::Invalid()) {} + SourceId file; std::stringstream csa_headerfile; std::stringstream csa_ccfile; std::stringstream class_definition_headerfile; + + // The beginning of the generated -inl.inc file, which includes declarations + // for functions corresponding to Torque macros. + std::stringstream class_definition_inline_headerfile_macro_declarations; + // The second part of the generated -inl.inc file, which includes + // definitions for functions declared in the first part. + std::stringstream class_definition_inline_headerfile_macro_definitions; + // The portion of the generated -inl.inc file containing member function + // definitions for the generated class. std::stringstream class_definition_inline_headerfile; + std::stringstream class_definition_ccfile; }; static PerFileStreams& GeneratedPerFile(SourceId file) { - return Get().generated_per_file_[file]; + PerFileStreams& result = Get().generated_per_file_[file]; + result.file = file; + return result; } static void SetInstanceTypesInitialized() { @@ -77,13 +91,15 @@ class GlobalContext : public ContextualClass { static bool IsInstanceTypesInitialized() { return Get().instance_types_initialized_; } - static void EnsureInCCOutputList(TorqueMacro* macro) { + static void EnsureInCCOutputList(TorqueMacro* macro, SourceId source) { GlobalContext& c = Get(); - if (c.macros_for_cc_output_set_.insert(macro).second) { - c.macros_for_cc_output_.push_back(macro); + auto item = std::make_pair(macro, source); + if (c.macros_for_cc_output_set_.insert(item).second) { + c.macros_for_cc_output_.push_back(item); } } - static const std::vector& AllMacrosForCCOutput() { + static const std::vector>& + AllMacrosForCCOutput() { return Get().macros_for_cc_output_; } @@ -96,8 +112,8 @@ class GlobalContext : public ContextualClass { std::set cpp_includes_; std::map generated_per_file_; std::map fresh_ids_; - std::vector macros_for_cc_output_; - std::unordered_set macros_for_cc_output_set_; + std::vector> macros_for_cc_output_; + std::set> macros_for_cc_output_set_; bool instance_types_initialized_ = false; friend class LanguageServerData; diff --git a/src/torque/implementation-visitor.cc b/src/torque/implementation-visitor.cc index 03dd60a074..9e2e4faef3 100644 --- a/src/torque/implementation-visitor.cc +++ b/src/torque/implementation-visitor.cc @@ -66,9 +66,10 @@ void ImplementationVisitor::BeginGeneratedFiles() { } for (SourceId file : SourceFileMap::AllSources()) { + auto& streams = GlobalContext::GeneratedPerFile(file); // Output beginning of CSA .cc file. { - std::ostream& out = GlobalContext::GeneratedPerFile(file).csa_ccfile; + std::ostream& out = streams.csa_ccfile; for (const std::string& include_path : GlobalContext::CppIncludes()) { out << "#include " << StringLiteralQuote(include_path) << "\n"; @@ -87,12 +88,12 @@ void ImplementationVisitor::BeginGeneratedFiles() { } // Output beginning of CSA .h file. { - std::ostream& out = GlobalContext::GeneratedPerFile(file).csa_headerfile; - std::string headerDefine = + std::ostream& out = streams.csa_headerfile; + std::string header_define = "V8_GEN_TORQUE_GENERATED_" + - UnderlinifyPath(SourceFileMap::PathFromV8Root(file)) + "_H_"; - out << "#ifndef " << headerDefine << "\n"; - out << "#define " << headerDefine << "\n\n"; + UnderlinifyPath(SourceFileMap::PathFromV8Root(file)) + "_CSA_H_"; + out << "#ifndef " << header_define << "\n"; + out << "#define " << header_define << "\n\n"; out << "#include \"src/builtins/torque-csa-header-includes.h\"\n"; out << "\n"; @@ -102,7 +103,6 @@ void ImplementationVisitor::BeginGeneratedFiles() { } // Output beginning of class definition .cc file. { - auto& streams = GlobalContext::GeneratedPerFile(file); std::ostream& out = streams.class_definition_ccfile; if (contains_class_definitions.count(file) != 0) { out << "#include \"" @@ -120,28 +120,28 @@ void ImplementationVisitor::BeginGeneratedFiles() { void ImplementationVisitor::EndGeneratedFiles() { for (SourceId file : SourceFileMap::AllSources()) { + auto& streams = GlobalContext::GeneratedPerFile(file); { - std::ostream& out = GlobalContext::GeneratedPerFile(file).csa_ccfile; + std::ostream& out = streams.csa_ccfile; out << "} // namespace internal\n" << "} // namespace v8\n" << "\n"; } { - std::ostream& out = GlobalContext::GeneratedPerFile(file).csa_headerfile; + std::ostream& out = streams.csa_headerfile; - std::string headerDefine = + std::string header_define = "V8_GEN_TORQUE_GENERATED_" + - UnderlinifyPath(SourceFileMap::PathFromV8Root(file)) + "_H_"; + UnderlinifyPath(SourceFileMap::PathFromV8Root(file)) + "_CSA_H_"; out << "} // namespace internal\n" << "} // namespace v8\n" << "\n"; - out << "#endif // " << headerDefine << "\n"; + out << "#endif // " << header_define << "\n"; } { - std::ostream& out = - GlobalContext::GeneratedPerFile(file).class_definition_ccfile; + std::ostream& out = streams.class_definition_ccfile; out << "} // namespace v8\n"; out << "} // namespace internal\n"; @@ -149,46 +149,6 @@ void ImplementationVisitor::EndGeneratedFiles() { } } -void ImplementationVisitor::BeginRuntimeMacrosFile() { - std::ostream& source = runtime_macros_cc_; - std::ostream& header = runtime_macros_h_; - - source << "#include \"torque-generated/runtime-macros.h\"\n\n"; - source << "#include \"src/torque/runtime-macro-shims.h\"\n"; - for (const std::string& include_path : GlobalContext::CppIncludes()) { - source << "#include " << StringLiteralQuote(include_path) << "\n"; - } - source << "\n"; - - source << "namespace v8 {\n" - << "namespace internal {\n" - << "\n"; - - const char* kHeaderDefine = "V8_GEN_TORQUE_GENERATED_RUNTIME_MACROS_H_"; - header << "#ifndef " << kHeaderDefine << "\n"; - header << "#define " << kHeaderDefine << "\n\n"; - header << "#include \"src/builtins/torque-csa-header-includes.h\"\n"; - header << "\n"; - - header << "namespace v8 {\n" - << "namespace internal {\n" - << "\n"; -} - -void ImplementationVisitor::EndRuntimeMacrosFile() { - std::ostream& source = runtime_macros_cc_; - std::ostream& header = runtime_macros_h_; - - source << "} // namespace internal\n" - << "} // namespace v8\n" - << "\n"; - - header << "\n} // namespace internal\n" - << "} // namespace v8\n" - << "\n"; - header << "#endif // V8_GEN_TORQUE_GENERATED_RUNTIME_MACROS_H_\n"; -} - void ImplementationVisitor::Visit(NamespaceConstant* decl) { Signature signature{{}, base::nullopt, {{}, false}, 0, decl->type(), {}, false}; @@ -356,6 +316,14 @@ void ImplementationVisitor::VisitMacroCommon(Macro* macro) { GenerateMacroFunctionDeclaration(csa_headerfile(), macro); csa_headerfile() << ";\n"; + // Avoid multiple-definition errors since it is possible for multiple + // generated -inl.inc files to all contain function definitions for the same + // Torque macro. + if (output_type_ == OutputType::kCC) { + csa_ccfile() << "#ifndef V8_INTERNAL_DEFINED_" << macro->CCName() << "\n"; + csa_ccfile() << "#define V8_INTERNAL_DEFINED_" << macro->CCName() << "\n"; + } + GenerateMacroFunctionDeclaration(csa_ccfile(), macro); csa_ccfile() << " {\n"; @@ -469,7 +437,12 @@ void ImplementationVisitor::VisitMacroCommon(Macro* macro) { } csa_ccfile() << ";\n"; } - csa_ccfile() << "}\n\n"; + csa_ccfile() << "}\n"; + if (output_type_ == OutputType::kCC) { + csa_ccfile() << "#endif // V8_INTERNAL_DEFINED_" << macro->CCName() + << "\n"; + } + csa_ccfile() << "\n"; } void ImplementationVisitor::Visit(TorqueMacro* macro) { @@ -1697,13 +1670,13 @@ void ImplementationVisitor::GenerateImplementation(const std::string& dir) { WriteFile(base_filename + "-tq-csa.h", streams.csa_headerfile.str()); WriteFile(base_filename + "-tq.inc", streams.class_definition_headerfile.str()); - WriteFile(base_filename + "-tq-inl.inc", - streams.class_definition_inline_headerfile.str()); + WriteFile( + base_filename + "-tq-inl.inc", + streams.class_definition_inline_headerfile_macro_declarations.str() + + streams.class_definition_inline_headerfile_macro_definitions.str() + + streams.class_definition_inline_headerfile.str()); WriteFile(base_filename + "-tq.cc", streams.class_definition_ccfile.str()); } - - WriteFile(dir + "/runtime-macros.h", runtime_macros_h_.str()); - WriteFile(dir + "/runtime-macros.cc", runtime_macros_cc_.str()); } void ImplementationVisitor::GenerateMacroFunctionDeclaration(std::ostream& o, @@ -1719,6 +1692,9 @@ std::vector ImplementationVisitor::GenerateFunctionDeclaration( const Signature& signature, const NameVector& parameter_names, bool pass_code_assembler_state) { std::vector generated_parameter_names; + if (output_type_ == OutputType::kCC) { + o << "inline "; + } if (signature.return_type->IsVoidOrNever()) { o << "void"; } else { @@ -2677,10 +2653,12 @@ VisitResult ImplementationVisitor::GenerateCall( // If we're currently generating a C++ macro and it's calling another macro, // then we need to make sure that we also generate C++ code for the called - // macro. + // macro within the same -inl.inc file. if (output_type_ == OutputType::kCC && !inline_macro) { if (auto* torque_macro = TorqueMacro::DynamicCast(macro)) { - GlobalContext::EnsureInCCOutputList(torque_macro); + auto* streams = CurrentFileStreams::Get(); + SourceId file = streams ? streams->file : SourceId::Invalid(); + GlobalContext::EnsureInCCOutputList(torque_macro, file); } } @@ -3176,11 +3154,11 @@ void ImplementationVisitor::VisitAllDeclarables() { // Do the same for macros which generate C++ code. output_type_ = OutputType::kCC; - const std::vector& cc_macros = + const std::vector>& cc_macros = GlobalContext::AllMacrosForCCOutput(); for (size_t i = 0; i < cc_macros.size(); ++i) { try { - Visit(static_cast(cc_macros[i])); + Visit(static_cast(cc_macros[i].first), cc_macros[i].second); } catch (TorqueAbortCompilation&) { // Recover from compile errors here. The error is recorded already. } @@ -3188,11 +3166,13 @@ void ImplementationVisitor::VisitAllDeclarables() { output_type_ = OutputType::kCSA; } -void ImplementationVisitor::Visit(Declarable* declarable) { +void ImplementationVisitor::Visit(Declarable* declarable, + base::Optional file) { CurrentScope::Scope current_scope(declarable->ParentScope()); CurrentSourcePosition::Scope current_source_position(declarable->Position()); CurrentFileStreams::Scope current_file_streams( - &GlobalContext::GeneratedPerFile(declarable->Position().source)); + &GlobalContext::GeneratedPerFile(file ? *file + : declarable->Position().source)); if (Callable* callable = Callable::DynamicCast(declarable)) { if (!callable->ShouldGenerateExternalCode(output_type_)) CurrentFileStreams::Get() = nullptr; @@ -4686,13 +4666,15 @@ void ImplementationVisitor::GenerateClassVerifiers( cc_contents << "#include \"torque-generated/" << file_name << ".h\"\n"; cc_contents << "#include " "\"src/objects/all-objects-inl.h\"\n"; - cc_contents << "#include \"torque-generated/runtime-macros.h\"\n"; IncludeObjectMacrosScope object_macros(cc_contents); NamespaceScope h_namespaces(h_contents, {"v8", "internal"}); NamespaceScope cc_namespaces(cc_contents, {"v8", "internal"}); + cc_contents + << "#include \"torque-generated/test/torque/test-torque-tq-inl.inc\"\n"; + // Generate forward declarations to avoid including any headers. h_contents << "class Isolate;\n"; for (const ClassType* type : TypeOracle::GetClasses()) { diff --git a/src/torque/implementation-visitor.h b/src/torque/implementation-visitor.h index 4654ecd42c..037697e698 100644 --- a/src/torque/implementation-visitor.h +++ b/src/torque/implementation-visitor.h @@ -511,7 +511,7 @@ class ImplementationVisitor { VisitResult Visit(FieldAccessExpression* expr); void VisitAllDeclarables(); - void Visit(Declarable* delarable); + void Visit(Declarable* delarable, base::Optional file = {}); void Visit(TypeAlias* decl); VisitResult InlineMacro(Macro* macro, base::Optional this_reference, @@ -561,10 +561,6 @@ class ImplementationVisitor { void BeginGeneratedFiles(); void EndGeneratedFiles(); - // TODO(tebbi): Switch to per-file generation for runtime macros and merge - // these functions into {Begin,End}GeneratedFiles(). - void BeginRuntimeMacrosFile(); - void EndRuntimeMacrosFile(); void GenerateImplementation(const std::string& dir); @@ -772,33 +768,19 @@ class ImplementationVisitor { std::ostream& csa_ccfile() { if (auto* streams = CurrentFileStreams::Get()) { - return output_type_ == OutputType::kCSA ? streams->csa_ccfile - : runtime_macros_cc_; + return output_type_ == OutputType::kCSA + ? streams->csa_ccfile + : streams + ->class_definition_inline_headerfile_macro_definitions; } return null_stream_; } std::ostream& csa_headerfile() { if (auto* streams = CurrentFileStreams::Get()) { - return output_type_ == OutputType::kCSA ? streams->csa_headerfile - : runtime_macros_h_; - } - return null_stream_; - } - std::ostream& class_definition_headerfile() { - if (auto* streams = CurrentFileStreams::Get()) { - return streams->class_definition_headerfile; - } - return null_stream_; - } - std::ostream& class_definition_inline_headerfile() { - if (auto* streams = CurrentFileStreams::Get()) { - return streams->class_definition_inline_headerfile; - } - return null_stream_; - } - std::ostream& class_definition_ccfile() { - if (auto* streams = CurrentFileStreams::Get()) { - return streams->class_definition_ccfile; + return output_type_ == OutputType::kCSA + ? streams->csa_headerfile + : streams + ->class_definition_inline_headerfile_macro_declarations; } return null_stream_; } @@ -850,14 +832,6 @@ class ImplementationVisitor { std::unordered_map bitfield_expressions_; - // The contents of the runtime macros output files. These contain all Torque - // macros that have been generated using the C++ backend. They're not yet - // split per source file like CSA macros, but eventually we should change them - // to generate -inl.inc files so that callers can easily inline their - // contents. - std::stringstream runtime_macros_cc_; - std::stringstream runtime_macros_h_; - OutputType output_type_ = OutputType::kCSA; }; diff --git a/src/torque/runtime-macro-shims.h b/src/torque/runtime-macro-shims.h index 89e566bc62..a7c017f93d 100644 --- a/src/torque/runtime-macro-shims.h +++ b/src/torque/runtime-macro-shims.h @@ -8,10 +8,13 @@ #ifndef V8_TORQUE_RUNTIME_MACRO_SHIMS_H_ #define V8_TORQUE_RUNTIME_MACRO_SHIMS_H_ -#include "src/objects/smi.h" +#include namespace v8 { namespace internal { + +class Isolate; + namespace TorqueRuntimeMacroShims { namespace CodeStubAssembler { @@ -26,7 +29,10 @@ inline intptr_t IntPtrMul(Isolate* isolate, intptr_t a, intptr_t b) { inline intptr_t Signed(Isolate* isolate, uintptr_t u) { return static_cast(u); } -inline int32_t SmiUntag(Isolate* isolate, Smi s) { return s.value(); } +template +inline int32_t SmiUntag(Isolate* isolate, Smi s) { + return s.value(); +} } // namespace CodeStubAssembler } // namespace TorqueRuntimeMacroShims diff --git a/src/torque/torque-compiler.cc b/src/torque/torque-compiler.cc index 9e00412ca1..88b3b94566 100644 --- a/src/torque/torque-compiler.cc +++ b/src/torque/torque-compiler.cc @@ -76,7 +76,6 @@ void CompileCurrentAst(TorqueCompilerOptions options) { implementation_visitor.GenerateInstanceTypes(output_directory); implementation_visitor.BeginGeneratedFiles(); - implementation_visitor.BeginRuntimeMacrosFile(); implementation_visitor.VisitAllDeclarables(); @@ -96,7 +95,6 @@ void CompileCurrentAst(TorqueCompilerOptions options) { implementation_visitor.GenerateCSATypes(output_directory); implementation_visitor.EndGeneratedFiles(); - implementation_visitor.EndRuntimeMacrosFile(); implementation_visitor.GenerateImplementation(output_directory); if (GlobalContext::collect_language_server_data()) { diff --git a/src/torque/types.cc b/src/torque/types.cc index 50f3489a8c..848fb21e08 100644 --- a/src/torque/types.cc +++ b/src/torque/types.cc @@ -897,7 +897,8 @@ void ClassType::GenerateSliceAccessor(size_t field_index) { Macro* macro = Declarations::DeclareMacro(macro_name, true, base::nullopt, signature, block, base::nullopt); - GlobalContext::EnsureInCCOutputList(TorqueMacro::cast(macro)); + GlobalContext::EnsureInCCOutputList(TorqueMacro::cast(macro), + macro->Position().source); } bool ClassType::HasStaticSize() const {