Author: Richard Smith Date: 2020-06-30T18:22:09-07:00 New Revision: 4eff2beefb2b655fc02d35de235fc86d72d05755
URL: https://github.com/llvm/llvm-project/commit/4eff2beefb2b655fc02d35de235fc86d72d05755 DIFF: https://github.com/llvm/llvm-project/commit/4eff2beefb2b655fc02d35de235fc86d72d05755.diff LOG: [c++20] consteval functions don't get vtable slots. For the Itanium C++ ABI, this implements the rule added in https://github.com/itanium-cxx-abi/cxx-abi/pull/83 For the MS C++ ABI, this implements the direction that seemed most plausible based on personal correspondence with MSVC developers, but is subject to change as they decide their ABI rule. Added: clang/test/CodeGenCXX/vtable-consteval.cpp Modified: clang/include/clang/AST/VTableBuilder.h clang/lib/AST/RecordLayoutBuilder.cpp clang/lib/AST/VTableBuilder.cpp clang/lib/CodeGen/CGExprConstant.cpp Removed: ################################################################################ diff --git a/clang/include/clang/AST/VTableBuilder.h b/clang/include/clang/AST/VTableBuilder.h index e6557e438919..241dd13f903e 100644 --- a/clang/include/clang/AST/VTableBuilder.h +++ b/clang/include/clang/AST/VTableBuilder.h @@ -354,6 +354,9 @@ class VTableContextBase { } bool IsMicrosoftABI; + + /// Determine whether this function should be assigned a vtable slot. + static bool hasVtableSlot(const CXXMethodDecl *MD); }; class ItaniumVTableContext : public VTableContextBase { diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 3d5f785e943e..b44957a35279 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -15,6 +15,7 @@ #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/Expr.h" +#include "clang/AST/VTableBuilder.h" #include "clang/Basic/TargetInfo.h" #include "llvm/ADT/SmallSet.h" #include "llvm/Support/Format.h" @@ -2568,9 +2569,11 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) { // information about the bases, such as required alignment and the presence of // zero sized members. const ASTRecordLayout *PreviousBaseLayout = nullptr; + bool HasPolymorphicBaseClass = false; // Iterate through the bases and lay out the non-virtual ones. for (const CXXBaseSpecifier &Base : RD->bases()) { const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl(); + HasPolymorphicBaseClass |= BaseDecl->isPolymorphic(); const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(BaseDecl); // Mark and skip virtual bases. if (Base.isVirtual()) { @@ -2594,11 +2597,23 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) { layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout); } // Figure out if we need a fresh VFPtr for this class. - if (!PrimaryBase && RD->isDynamicClass()) - for (CXXRecordDecl::method_iterator i = RD->method_begin(), - e = RD->method_end(); - !HasOwnVFPtr && i != e; ++i) - HasOwnVFPtr = i->isVirtual() && i->size_overridden_methods() == 0; + if (RD->isPolymorphic()) { + if (!HasPolymorphicBaseClass) + // This class introduces polymorphism, so we need a vftable to store the + // RTTI information. + HasOwnVFPtr = true; + else if (!PrimaryBase) { + // We have a polymorphic base class but can't extend its vftable. Add a + // new vfptr if we would use any vftable slots. + for (CXXMethodDecl *M : RD->methods()) { + if (MicrosoftVTableContext::hasVtableSlot(M) && + M->size_overridden_methods() == 0) { + HasOwnVFPtr = true; + break; + } + } + } + } // If we don't have a primary base then we have a leading object that could // itself lead with a zero-sized object, something we track. bool CheckLeadingLayout = !PrimaryBase; @@ -2993,7 +3008,8 @@ void MicrosoftRecordLayoutBuilder::computeVtorDispSet( llvm::SmallPtrSet<const CXXRecordDecl *, 2> BasesWithOverriddenMethods; // Seed the working set with our non-destructor, non-pure virtual methods. for (const CXXMethodDecl *MD : RD->methods()) - if (MD->isVirtual() && !isa<CXXDestructorDecl>(MD) && !MD->isPure()) + if (MicrosoftVTableContext::hasVtableSlot(MD) && + !isa<CXXDestructorDecl>(MD) && !MD->isPure()) Work.insert(MD); while (!Work.empty()) { const CXXMethodDecl *MD = *Work.begin(); diff --git a/clang/lib/AST/VTableBuilder.cpp b/clang/lib/AST/VTableBuilder.cpp index 013ebe23ec91..f5865ce96b64 100644 --- a/clang/lib/AST/VTableBuilder.cpp +++ b/clang/lib/AST/VTableBuilder.cpp @@ -408,7 +408,7 @@ void FinalOverriders::dump(raw_ostream &Out, BaseSubobject Base, // Now dump the overriders for this base subobject. for (const auto *MD : RD->methods()) { - if (!MD->isVirtual()) + if (!VTableContextBase::hasVtableSlot(MD)) continue; MD = MD->getCanonicalDecl(); @@ -486,8 +486,8 @@ static bool HasSameVirtualSignature(const CXXMethodDecl *LHS, bool VCallOffsetMap::MethodsCanShareVCallOffset(const CXXMethodDecl *LHS, const CXXMethodDecl *RHS) { - assert(LHS->isVirtual() && "LHS must be virtual!"); - assert(RHS->isVirtual() && "LHS must be virtual!"); + assert(VTableContextBase::hasVtableSlot(LHS) && "LHS must be virtual!"); + assert(VTableContextBase::hasVtableSlot(RHS) && "LHS must be virtual!"); // A destructor can share a vcall offset with another destructor. if (isa<CXXDestructorDecl>(LHS)) @@ -697,7 +697,7 @@ void VCallAndVBaseOffsetBuilder::AddVCallOffsets(BaseSubobject Base, // Add the vcall offsets. for (const auto *MD : RD->methods()) { - if (!MD->isVirtual()) + if (!VTableContextBase::hasVtableSlot(MD)) continue; MD = MD->getCanonicalDecl(); @@ -1085,7 +1085,7 @@ typedef llvm::SmallPtrSet<const CXXMethodDecl *, 8> OverriddenMethodsSetTy; template <class VisitorTy> static void visitAllOverriddenMethods(const CXXMethodDecl *MD, VisitorTy &Visitor) { - assert(MD->isVirtual() && "Method is not virtual!"); + assert(VTableContextBase::hasVtableSlot(MD) && "Method is not virtual!"); for (const CXXMethodDecl *OverriddenMD : MD->overridden_methods()) { if (!Visitor(OverriddenMD)) @@ -1489,7 +1489,7 @@ void ItaniumVTableBuilder::AddMethods( // Now go through all virtual member functions and add them. for (const auto *MD : RD->methods()) { - if (!MD->isVirtual()) + if (!ItaniumVTableContext::hasVtableSlot(MD)) continue; MD = MD->getCanonicalDecl(); @@ -2169,7 +2169,7 @@ void ItaniumVTableBuilder::dumpLayout(raw_ostream &Out) { for (const auto *MD : MostDerivedClass->methods()) { // We only want virtual member functions. - if (!MD->isVirtual()) + if (!ItaniumVTableContext::hasVtableSlot(MD)) continue; MD = MD->getCanonicalDecl(); @@ -2257,6 +2257,10 @@ VTableLayout::VTableLayout(ArrayRef<size_t> VTableIndices, VTableLayout::~VTableLayout() { } +bool VTableContextBase::hasVtableSlot(const CXXMethodDecl *MD) { + return MD->isVirtual() && !MD->isConsteval(); +} + ItaniumVTableContext::ItaniumVTableContext( ASTContext &Context, VTableComponentLayout ComponentLayout) : VTableContextBase(/*MS=*/false), ComponentLayout(ComponentLayout) {} @@ -2537,8 +2541,9 @@ class VFTableBuilder { BasesSetVectorTy VisitedBases; AddMethods(BaseSubobject(MostDerivedClass, CharUnits::Zero()), 0, nullptr, VisitedBases); - assert((HasRTTIComponent ? Components.size() - 1 : Components.size()) && - "vftable can't be empty"); + // Note that it is possible for the vftable to contain only an RTTI + // pointer, if all virtual functions are constewval. + assert(!Components.empty() && "vftable can't be empty"); assert(MethodVFTableLocations.empty()); for (const auto &I : MethodInfoMap) { @@ -2917,7 +2922,7 @@ static void GroupNewVirtualOverloads( if (Inserted) Groups.push_back(MethodGroup()); if (const auto *MD = dyn_cast<CXXMethodDecl>(ND)) - if (MD->isVirtual()) + if (MicrosoftVTableContext::hasVtableSlot(MD)) Groups[J->second].push_back(MD->getCanonicalDecl()); } @@ -3513,7 +3518,7 @@ static const FullPathTy *selectBestPath(ASTContext &Context, getOffsetOfFullPath(Context, TopLevelRD, SpecificPath); FinalOverriders Overriders(TopLevelRD, CharUnits::Zero(), TopLevelRD); for (const CXXMethodDecl *MD : Info.IntroducingObject->methods()) { - if (!MD->isVirtual()) + if (!MicrosoftVTableContext::hasVtableSlot(MD)) continue; FinalOverriders::OverriderInfo OI = Overriders.getOverrider(MD->getCanonicalDecl(), BaseOffset); @@ -3652,7 +3657,7 @@ void MicrosoftVTableContext::dumpMethodLocations( for (const auto &I : NewMethods) { const CXXMethodDecl *MD = cast<const CXXMethodDecl>(I.first.getDecl()); - assert(MD->isVirtual()); + assert(hasVtableSlot(MD)); std::string MethodName = PredefinedExpr::ComputeName( PredefinedExpr::PrettyFunctionNoVirtual, MD); @@ -3772,7 +3777,7 @@ MicrosoftVTableContext::getVFTableLayout(const CXXRecordDecl *RD, MethodVFTableLocation MicrosoftVTableContext::getMethodVFTableLocation(GlobalDecl GD) { - assert(cast<CXXMethodDecl>(GD.getDecl())->isVirtual() && + assert(hasVtableSlot(cast<CXXMethodDecl>(GD.getDecl())) && "Only use this method for virtual methods or dtors"); if (isa<CXXDestructorDecl>(GD.getDecl())) assert(GD.getDtorType() == Dtor_Deleting); diff --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp index a01571633f6c..c6b2930faece 100644 --- a/clang/lib/CodeGen/CGExprConstant.cpp +++ b/clang/lib/CodeGen/CGExprConstant.cpp @@ -777,7 +777,7 @@ bool ConstStructBuilder::Build(const APValue &Val, const RecordDecl *RD, if (const CXXRecordDecl *CD = dyn_cast<CXXRecordDecl>(RD)) { // Add a vtable pointer, if we need one and it hasn't already been added. - if (CD->isDynamicClass() && !IsPrimaryBase) { + if (Layout.hasOwnVFPtr()) { llvm::Constant *VTableAddressPoint = CGM.getCXXABI().getVTableAddressPointForConstExpr( BaseSubobject(CD, Offset), VTableClass); diff --git a/clang/test/CodeGenCXX/vtable-consteval.cpp b/clang/test/CodeGenCXX/vtable-consteval.cpp new file mode 100644 index 000000000000..ed4a25290d35 --- /dev/null +++ b/clang/test/CodeGenCXX/vtable-consteval.cpp @@ -0,0 +1,64 @@ +// RUN: %clang_cc1 -std=c++20 %s -emit-llvm -o - -triple x86_64-linux | FileCheck %s --check-prefix=ITANIUM --implicit-check-not=DoNotEmit +// RUN: %clang_cc1 -std=c++20 %s -emit-llvm -o - -triple x86_64-windows | FileCheck %s --check-prefix=MSABI --implicit-check-not=DoNotEmit + +// FIXME: The MSVC ABI rule in use here was discussed with MS folks prior to +// them implementing virtual consteval functions, but we do not know for sure +// if this is the ABI rule they will use. + +// ITANIUM-DAG: @_ZTV1A = {{.*}} constant { [2 x i8*] } {{.*}} null, {{.*}} @_ZTI1A +// MSABI-DAG: @[[A_VFTABLE:.*]] = {{.*}} constant { [1 x i8*] } {{.*}} @"??_R4A@@6B@" +struct A { + virtual consteval void DoNotEmit_f() {} +}; +// ITANIUM-DAG: @a = {{.*}}global { i8** } { {{.*}} @_ZTV1A, +// MSABI-DAG: @"?a@@3UA@@A" = {{.*}}global { i8** } { i8** @"??_7A@@6B@" } +A a; + +// ITANIUM-DAG: @_ZTV1B = {{.*}} constant { [4 x i8*] } {{.*}} null, {{.*}} @_ZTI1B {{.*}} @_ZN1B1fEv {{.*}} @_ZN1B1hEv +// MSABI-DAG: @[[B_VFTABLE:.*]] = {{.*}} constant { [3 x i8*] } {{.*}} @"??_R4B@@6B@" {{.*}} @"?f@B@@UEAAXXZ" {{.*}} @"?h@B@@UEAAXXZ" +struct B { + virtual void f() {} + virtual consteval void DoNotEmit_g() {} + virtual void h() {} +}; +// ITANIUM-DAG: @b = {{.*}}global { i8** } { {{.*}} @_ZTV1B, +// MSABI-DAG: @"?b@@3UB@@A" = {{.*}}global { i8** } { i8** @"??_7B@@6B@" } +B b; + +// ITANIUM-DAG: @_ZTV1C = {{.*}} constant { [4 x i8*] } {{.*}} null, {{.*}} @_ZTI1C {{.*}} @_ZN1CD1Ev {{.*}} @_ZN1CD0Ev +// MSABI-DAG: @[[C_VFTABLE:.*]] = {{.*}} constant { [2 x i8*] } {{.*}} @"??_R4C@@6B@" {{.*}} @"??_GC@@UEAAPEAXI@Z" +struct C { + virtual ~C() = default; + virtual consteval C &operator=(const C&) = default; +}; +// ITANIUM-DAG: @c = {{.*}}global { i8** } { {{.*}} @_ZTV1C, +// MSABI-DAG: @"?c@@3UC@@A" = {{.*}}global { i8** } { i8** @"??_7C@@6B@" } +C c; + +// ITANIUM-DAG: @_ZTV1D = {{.*}} constant { [4 x i8*] } {{.*}} null, {{.*}} @_ZTI1D {{.*}} @_ZN1DD1Ev {{.*}} @_ZN1DD0Ev +// MSABI-DAG: @[[D_VFTABLE:.*]] = {{.*}} constant { [2 x i8*] } {{.*}} @"??_R4D@@6B@" {{.*}} @"??_GD@@UEAAPEAXI@Z" +struct D : C {}; +// ITANIUM-DAG: @d = {{.*}}global { i8** } { {{.*}} @_ZTV1D, +// MSABI-DAG: @"?d@@3UD@@A" = {{.*}}global { i8** } { i8** @"??_7D@@6B@" } +D d; + +// ITANIUM-DAG: @_ZTV1E = {{.*}} constant { [3 x i8*] } {{.*}} null, {{.*}} @_ZTI1E {{.*}} @_ZN1E1fEv +// MSABI-DAG: @[[E_VFTABLE:.*]] = {{.*}} constant { [2 x i8*] } {{.*}} @"??_R4E@@6B@" {{.*}} @"?f@E@@UEAAXXZ" +struct E { virtual void f() {} }; +// ITANIUM-DAG: @e = {{.*}}global { i8** } { {{.*}} @_ZTV1E, +// MSABI-DAG: @"?e@@3UE@@A" = {{.*}}global { i8** } { i8** @"??_7E@@6B@" } +E e; + +// ITANIUM-DAG: @_ZTV1F = {{.*}} constant { [3 x i8*] } {{.*}} null, {{.*}} @_ZTI1F {{.*}} @_ZN1E1fEv +// MSABI-DAG: @[[F_VFTABLE:.*]] = {{.*}} constant { [2 x i8*] } {{.*}} @"??_R4F@@6B@" {{.*}} @"?f@E@@UEAAXXZ" +struct F : E { virtual consteval void DoNotEmit_g(); }; +// ITANIUM-DAG: @f = {{.*}}global { i8** } { {{.*}} @_ZTV1F, +// MSABI-DAG: @"?f@@3UF@@A" = {{.*}}global { i8** } { i8** @"??_7F@@6B@" } +F f; + +// MSABI-DAG: @"??_7A@@6B@" = {{.*}} alias {{.*}} @[[A_VFTABLE]], +// MSABI-DAG: @"??_7B@@6B@" = {{.*}} alias {{.*}} @[[B_VFTABLE]], +// MSABI-DAG: @"??_7C@@6B@" = {{.*}} alias {{.*}} @[[C_VFTABLE]], +// MSABI-DAG: @"??_7D@@6B@" = {{.*}} alias {{.*}} @[[D_VFTABLE]], +// MSABI-DAG: @"??_7E@@6B@" = {{.*}} alias {{.*}} @[[E_VFTABLE]], +// MSABI-DAG: @"??_7F@@6B@" = {{.*}} alias {{.*}} @[[F_VFTABLE]], _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
