From ce8146929788041d27f8367e7c1d2f22e6ee79c1 Mon Sep 17 00:00:00 2001 From: Ng Zhi An Date: Mon, 23 Aug 2021 16:47:35 -0700 Subject: [PATCH] Add class template SharedTurboAssemblerBase as a base class Previously SharedTurboAssembler was a base class for ia32 and x64 TurboAssembler. This made it easy to share code, only if the implementation was the same. In some cases, like ExternalReferenceAsOperand, the implementation defers slightly between the two architectures. We add a new class template SharedTurboAssemblerBase, which derives from SharedTurboAssembler. Using the CRTP pattern, we can call derived classes functions using the template parameter. For any function that is exactly the same, we can declare them in the header and define them in the cc file, instead of inlining them all into the header. Bug: v8:11589 Change-Id: I9319bd0c26c76995cef43ae5ec3f69392b3f825b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3097109 Commit-Queue: Zhi An Ng Reviewed-by: Toon Verwaest Cr-Commit-Position: refs/heads/main@{#76469} --- src/codegen/ia32/macro-assembler-ia32.h | 5 ++-- .../macro-assembler-shared-ia32-x64.h | 27 +++++++++++++++++++ src/codegen/x64/macro-assembler-x64.h | 5 ++-- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/codegen/ia32/macro-assembler-ia32.h b/src/codegen/ia32/macro-assembler-ia32.h index 1d5243f518..cd6788defd 100644 --- a/src/codegen/ia32/macro-assembler-ia32.h +++ b/src/codegen/ia32/macro-assembler-ia32.h @@ -68,9 +68,10 @@ class StackArgumentsAccessor { DISALLOW_IMPLICIT_CONSTRUCTORS(StackArgumentsAccessor); }; -class V8_EXPORT_PRIVATE TurboAssembler : public SharedTurboAssembler { +class V8_EXPORT_PRIVATE TurboAssembler + : public SharedTurboAssemblerBase { public: - using SharedTurboAssembler::SharedTurboAssembler; + using SharedTurboAssemblerBase::SharedTurboAssemblerBase; void CheckPageFlag(Register object, Register scratch, int mask, Condition cc, Label* condition_met, diff --git a/src/codegen/shared-ia32-x64/macro-assembler-shared-ia32-x64.h b/src/codegen/shared-ia32-x64/macro-assembler-shared-ia32-x64.h index 06199b8549..946874ee35 100644 --- a/src/codegen/shared-ia32-x64/macro-assembler-shared-ia32-x64.h +++ b/src/codegen/shared-ia32-x64/macro-assembler-shared-ia32-x64.h @@ -29,6 +29,12 @@ constexpr int kStackSavedSavedFPSize = 2 * kDoubleSize; constexpr int kStackSavedSavedFPSize = kDoubleSize; #endif // V8_ENABLE_WEBASSEMBLY +// Base class for SharedTurboAssemblerBase. This class contains macro-assembler +// functions that can be shared across ia32 and x64 without any template +// machinery, i.e. does not require the CRTP pattern that +// SharedTurboAssemblerBase exposes. This allows us to keep the bulk of +// definition inside a separate source file, rather than putting everything +// inside this header. class V8_EXPORT_PRIVATE SharedTurboAssembler : public TurboAssemblerBase { public: using TurboAssemblerBase::TurboAssemblerBase; @@ -376,6 +382,27 @@ class V8_EXPORT_PRIVATE SharedTurboAssembler : public TurboAssemblerBase { template void I16x8SplatPreAvx2(XMMRegister dst, Op src); }; + +// Common base class template shared by ia32 and x64 TurboAssembler. This uses +// the Curiously Recurring Template Pattern (CRTP), where Impl is the actual +// class (subclass of SharedTurboAssemblerBase instantiated with the actual +// class). This allows static polymorphism, where member functions can be move +// into SharedTurboAssembler, and we can also call into member functions +// defined in ia32 or x64 specific TurboAssembler from within this template +// class, via Impl. +// +// Note: all member functions must be defined in this header file so that the +// compiler can generate code for the function definitions. See +// https://isocpp.org/wiki/faq/templates#templates-defn-vs-decl for rationale. +// If a function does not need polymorphism, move it into SharedTurboAssembler, +// and define it outside of this header. +template +class V8_EXPORT_PRIVATE SharedTurboAssemblerBase : public SharedTurboAssembler { + using SharedTurboAssembler::SharedTurboAssembler; + // TODO(zhin): intentionally empty for now, will move polymorphic functions + // here in future changes. +}; + } // namespace internal } // namespace v8 #endif // V8_CODEGEN_SHARED_IA32_X64_MACRO_ASSEMBLER_SHARED_IA32_X64_H_ diff --git a/src/codegen/x64/macro-assembler-x64.h b/src/codegen/x64/macro-assembler-x64.h index 0f747b0453..0da222d487 100644 --- a/src/codegen/x64/macro-assembler-x64.h +++ b/src/codegen/x64/macro-assembler-x64.h @@ -57,9 +57,10 @@ class StackArgumentsAccessor { DISALLOW_IMPLICIT_CONSTRUCTORS(StackArgumentsAccessor); }; -class V8_EXPORT_PRIVATE TurboAssembler : public SharedTurboAssembler { +class V8_EXPORT_PRIVATE TurboAssembler + : public SharedTurboAssemblerBase { public: - using SharedTurboAssembler::SharedTurboAssembler; + using SharedTurboAssemblerBase::SharedTurboAssemblerBase; AVX_OP(Subsd, subsd) AVX_OP(Divss, divss) AVX_OP(Divsd, divsd)