[clang] 4eff2be - [c++20] consteval functions don't get vtable slots.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 30 18:22:56 PDT 2020
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 at B@@UEAAXXZ" {{.*}} @"?h at 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 at 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 at 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 at 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 at 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]],
More information about the cfe-commits
mailing list