Skip to content

Commit

Permalink
Revert "[C++20] [Modules] [Itanium ABI] Generate the vtable in the mo…
Browse files Browse the repository at this point in the history
…dule unit of dynamic classes (#75912)"

This reverts commit 18f3bcb, 15bb026 and
99873b3.

See the post commit message in
#75912 to see the reasons.
  • Loading branch information
ChuanqiXu9 committed Jul 10, 2024
1 parent 9af1f8f commit 91d40ef
Show file tree
Hide file tree
Showing 18 changed files with 64 additions and 347 deletions.
10 changes: 0 additions & 10 deletions clang/include/clang/AST/DeclBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -670,16 +670,6 @@ class alignas(8) Decl {
/// Whether this declaration comes from another module unit.
bool isInAnotherModuleUnit() const;

/// Whether this declaration comes from the same module unit being compiled.
bool isInCurrentModuleUnit() const;

/// Whether the definition of the declaration should be emitted in external
/// sources.
bool shouldEmitInExternalSource() const;

/// Whether this declaration comes from a named module;
bool isInNamedModule() const;

/// Whether this declaration comes from explicit global module.
bool isFromExplicitGlobalModule() const;

Expand Down
3 changes: 0 additions & 3 deletions clang/include/clang/Serialization/ASTBitCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -721,9 +721,6 @@ enum ASTRecordTypes {

/// Record code for \#pragma clang unsafe_buffer_usage begin/end
PP_UNSAFE_BUFFER_USAGE = 69,

/// Record code for vtables to emit.
VTABLES_TO_EMIT = 70,
};

/// Record types used within a source manager block.
Expand Down
6 changes: 0 additions & 6 deletions clang/include/clang/Serialization/ASTReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -790,11 +790,6 @@ class ASTReader
/// the consumer eagerly.
SmallVector<GlobalDeclID, 16> EagerlyDeserializedDecls;

/// The IDs of all vtables to emit. The referenced declarations are passed
/// to the consumers's HandleVTable eagerly after passing
/// EagerlyDeserializedDecls.
SmallVector<GlobalDeclID, 16> VTablesToEmit;

/// The IDs of all tentative definitions stored in the chain.
///
/// Sema keeps track of all tentative definitions in a TU because it has to
Expand Down Expand Up @@ -1505,7 +1500,6 @@ class ASTReader
bool isConsumerInterestedIn(Decl *D);
void PassInterestingDeclsToConsumer();
void PassInterestingDeclToConsumer(Decl *D);
void PassVTableToConsumer(CXXRecordDecl *RD);

void finishPendingActions();
void diagnoseOdrViolations();
Expand Down
7 changes: 0 additions & 7 deletions clang/include/clang/Serialization/ASTWriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -500,10 +500,6 @@ class ASTWriter : public ASTDeserializationListener,
std::vector<SourceRange> NonAffectingRanges;
std::vector<SourceLocation::UIntTy> NonAffectingOffsetAdjustments;

/// A list of classes which need to emit the VTable in the corresponding
/// object file.
llvm::SmallVector<CXXRecordDecl *> PendingEmittingVTables;

/// Computes input files that didn't affect compilation of the current module,
/// and initializes data structures necessary for leaving those files out
/// during \c SourceManager serialization.
Expand Down Expand Up @@ -861,8 +857,6 @@ class ASTWriter : public ASTDeserializationListener,
return PredefinedDecls.count(D);
}

void handleVTable(CXXRecordDecl *RD);

private:
// ASTDeserializationListener implementation
void ReaderInitialized(ASTReader *Reader) override;
Expand Down Expand Up @@ -957,7 +951,6 @@ class PCHGenerator : public SemaConsumer {

void InitializeSema(Sema &S) override { SemaPtr = &S; }
void HandleTranslationUnit(ASTContext &Ctx) override;
void HandleVTable(CXXRecordDecl *RD) override { Writer.handleVTable(RD); }
ASTMutationListener *GetASTMutationListener() override;
ASTDeserializationListener *GetASTDeserializationListener() override;
bool hasEmittedPCH() const { return Buffer->IsComplete; }
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12116,7 +12116,7 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
return false;

// Variables in other module units shouldn't be forced to be emitted.
if (VD->shouldEmitInExternalSource())
if (VD->isInAnotherModuleUnit())
return false;

// Variables that can be needed in other TUs are required.
Expand Down
11 changes: 9 additions & 2 deletions clang/lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1181,6 +1181,13 @@ Linkage NamedDecl::getLinkageInternal() const {
.getLinkage();
}

/// Determine whether D is attached to a named module.
static bool isInNamedModule(const NamedDecl *D) {
if (auto *M = D->getOwningModule())
return M->isNamedModule();
return false;
}

static bool isExportedFromModuleInterfaceUnit(const NamedDecl *D) {
// FIXME: Handle isModulePrivate.
switch (D->getModuleOwnershipKind()) {
Expand All @@ -1190,7 +1197,7 @@ static bool isExportedFromModuleInterfaceUnit(const NamedDecl *D) {
return false;
case Decl::ModuleOwnershipKind::Visible:
case Decl::ModuleOwnershipKind::VisibleWhenImported:
return D->isInNamedModule();
return isInNamedModule(D);
}
llvm_unreachable("unexpected module ownership kind");
}
Expand All @@ -1208,7 +1215,7 @@ Linkage NamedDecl::getFormalLinkage() const {
// [basic.namespace.general]/p2
// A namespace is never attached to a named module and never has a name with
// module linkage.
if (isInNamedModule() && InternalLinkage == Linkage::External &&
if (isInNamedModule(this) && InternalLinkage == Linkage::External &&
!isExportedFromModuleInterfaceUnit(
cast<NamedDecl>(this->getCanonicalDecl())) &&
!isa<NamespaceDecl>(this))
Expand Down
30 changes: 11 additions & 19 deletions clang/lib/AST/DeclBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1122,31 +1122,23 @@ bool Decl::isInExportDeclContext() const {
bool Decl::isInAnotherModuleUnit() const {
auto *M = getOwningModule();

if (!M || !M->isNamedModule())
if (!M)
return false;

return M != getASTContext().getCurrentNamedModule();
}

bool Decl::isInCurrentModuleUnit() const {
auto *M = getOwningModule();

if (!M || !M->isNamedModule())
M = M->getTopLevelModule();
// FIXME: It is problematic if the header module lives in another module
// unit. Consider to fix this by techniques like
// ExternalASTSource::hasExternalDefinitions.
if (M->isHeaderLikeModule())
return false;

return M == getASTContext().getCurrentNamedModule();
}

bool Decl::shouldEmitInExternalSource() const {
ExternalASTSource *Source = getASTContext().getExternalSource();
if (!Source)
// A global module without parent implies that we're parsing the global
// module. So it can't be in another module unit.
if (M->isGlobalModule())
return false;

return Source->hasExternalDefinitions(this) == ExternalASTSource::EK_Always;
}

bool Decl::isInNamedModule() const {
return getOwningModule() && getOwningModule()->isNamedModule();
assert(M->isNamedModule() && "New module kind?");
return M != getASTContext().getCurrentNamedModule();
}

bool Decl::isFromExplicitGlobalModule() const {
Expand Down
61 changes: 21 additions & 40 deletions clang/lib/CodeGen/CGVTables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1079,38 +1079,28 @@ CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) {
if (!RD->isExternallyVisible())
return llvm::GlobalVariable::InternalLinkage;

bool IsInNamedModule = RD->isInNamedModule();
// If the CXXRecordDecl are not in a module unit, we need to get
// its key function. We're at the end of the translation unit, so the current
// key function is fully correct.
const CXXMethodDecl *keyFunction =
IsInNamedModule ? nullptr : Context.getCurrentKeyFunction(RD);
if (IsInNamedModule || (keyFunction && !RD->hasAttr<DLLImportAttr>())) {
// We're at the end of the translation unit, so the current key
// function is fully correct.
const CXXMethodDecl *keyFunction = Context.getCurrentKeyFunction(RD);
if (keyFunction && !RD->hasAttr<DLLImportAttr>()) {
// If this class has a key function, use that to determine the
// linkage of the vtable.
const FunctionDecl *def = nullptr;
if (keyFunction && keyFunction->hasBody(def))
if (keyFunction->hasBody(def))
keyFunction = cast<CXXMethodDecl>(def);

bool IsExternalDefinition =
IsInNamedModule ? RD->shouldEmitInExternalSource() : !def;

TemplateSpecializationKind Kind =
IsInNamedModule ? RD->getTemplateSpecializationKind()
: keyFunction->getTemplateSpecializationKind();

switch (Kind) {
case TSK_Undeclared:
case TSK_ExplicitSpecialization:
switch (keyFunction->getTemplateSpecializationKind()) {
case TSK_Undeclared:
case TSK_ExplicitSpecialization:
assert(
(IsInNamedModule || def || CodeGenOpts.OptimizationLevel > 0 ||
(def || CodeGenOpts.OptimizationLevel > 0 ||
CodeGenOpts.getDebugInfo() != llvm::codegenoptions::NoDebugInfo) &&
"Shouldn't query vtable linkage without the class in module units, "
"key function, optimizations, or debug info");
if (IsExternalDefinition && CodeGenOpts.OptimizationLevel > 0)
"Shouldn't query vtable linkage without key function, "
"optimizations, or debug info");
if (!def && CodeGenOpts.OptimizationLevel > 0)
return llvm::GlobalVariable::AvailableExternallyLinkage;

if (keyFunction && keyFunction->isInlined())
if (keyFunction->isInlined())
return !Context.getLangOpts().AppleKext
? llvm::GlobalVariable::LinkOnceODRLinkage
: llvm::Function::InternalLinkage;
Expand All @@ -1129,7 +1119,7 @@ CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) {

case TSK_ExplicitInstantiationDeclaration:
llvm_unreachable("Should not have been asked to emit this");
}
}
}

// -fapple-kext mode does not support weak linkage, so we must use
Expand Down Expand Up @@ -1223,21 +1213,6 @@ bool CodeGenVTables::isVTableExternal(const CXXRecordDecl *RD) {
TSK == TSK_ExplicitInstantiationDefinition)
return false;

// Itanium C++ ABI [5.2.3]:
// Virtual tables for dynamic classes are emitted as follows:
//
// - If the class is templated, the tables are emitted in every object that
// references any of them.
// - Otherwise, if the class is attached to a module, the tables are uniquely
// emitted in the object for the module unit in which it is defined.
// - Otherwise, if the class has a key function (see below), the tables are
// emitted in the object for the translation unit containing the definition of
// the key function. This is unique if the key function is not inline.
// - Otherwise, the tables are emitted in every object that references any of
// them.
if (RD->isInNamedModule())
return RD->shouldEmitInExternalSource();

// Otherwise, if the class doesn't have a key function (possibly
// anymore), the vtable must be defined here.
const CXXMethodDecl *keyFunction = CGM.getContext().getCurrentKeyFunction(RD);
Expand All @@ -1247,7 +1222,13 @@ bool CodeGenVTables::isVTableExternal(const CXXRecordDecl *RD) {
const FunctionDecl *Def;
// Otherwise, if we don't have a definition of the key function, the
// vtable must be defined somewhere else.
return !keyFunction->hasBody(Def);
if (!keyFunction->hasBody(Def))
return true;

assert(Def && "The body of the key function is not assigned to Def?");
// If the non-inline key function comes from another module unit, the vtable
// must be defined there.
return Def->isInAnotherModuleUnit() && !Def->isInlineSpecified();
}

/// Given that we're currently at the end of the translation unit, and
Expand Down
3 changes: 0 additions & 3 deletions clang/lib/CodeGen/ItaniumCXXABI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2161,9 +2161,6 @@ bool ItaniumCXXABI::canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const {
if (!canSpeculativelyEmitVTableAsBaseClass(RD))
return false;

if (RD->shouldEmitInExternalSource())
return false;

// For a complete-object vtable (or more specifically, for the VTT), we need
// to be able to speculatively emit the vtables of all dynamic virtual bases.
for (const auto &B : RD->vbases()) {
Expand Down
11 changes: 1 addition & 10 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10094,7 +10094,7 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
// check at the end of the TU (or when the PMF starts) to see that we
// have a definition at that point.
if (isInline && !D.isFunctionDefinition() && getLangOpts().CPlusPlus20 &&
NewFD->isInNamedModule()) {
NewFD->hasOwningModule() && NewFD->getOwningModule()->isNamedModule()) {
PendingInlineFuncDecls.insert(NewFD);
}
}
Expand Down Expand Up @@ -18031,15 +18031,6 @@ void Sema::ActOnTagFinishDefinition(Scope *S, Decl *TagD,
if (NumInitMethods > 1 || !Def->hasInitMethod())
Diag(RD->getLocation(), diag::err_sycl_special_type_num_init_method);
}

// If we're defining a dynamic class in a module interface unit, we always
// need to produce the vtable for it even if the vtable is not used in the
// current TU.
//
// The case that the current class is not dynamic is handled in
// MarkVTableUsed.
if (getCurrentModule() && getCurrentModule()->isInterfaceOrPartition())
MarkVTableUsed(RD->getLocation(), RD, /*DefinitionRequired=*/true);
}

// Exit this scope of this tag's definition.
Expand Down
14 changes: 5 additions & 9 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18455,15 +18455,11 @@ bool Sema::DefineUsedVTables() {

bool DefineVTable = true;

// If this class has a key function, but that key function is
// defined in another translation unit, we don't need to emit the
// vtable even though we're using it.
const CXXMethodDecl *KeyFunction = Context.getCurrentKeyFunction(Class);
// V-tables for non-template classes with an owning module are always
// uniquely emitted in that module.
if (Class->isInCurrentModuleUnit())
DefineVTable = true;
else if (KeyFunction && !KeyFunction->hasBody()) {
// If this class has a key function, but that key function is
// defined in another translation unit, we don't need to emit the
// vtable even though we're using it.
if (KeyFunction && !KeyFunction->hasBody()) {
// The key function is in another translation unit.
DefineVTable = false;
TemplateSpecializationKind TSK =
Expand Down Expand Up @@ -18508,7 +18504,7 @@ bool Sema::DefineUsedVTables() {
DefinedAnything = true;
MarkVirtualMembersReferenced(Loc, Class);
CXXRecordDecl *Canonical = Class->getCanonicalDecl();
if (VTablesUsed[Canonical] && !Class->shouldEmitInExternalSource())
if (VTablesUsed[Canonical])
Consumer.HandleVTable(Class);

// Warn if we're emitting a weak vtable. The vtable will be weak if there is
Expand Down
11 changes: 0 additions & 11 deletions clang/lib/Serialization/ASTReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3921,13 +3921,6 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
}
break;

case VTABLES_TO_EMIT:
if (F.Kind == MK_MainFile ||
getContext().getLangOpts().BuildingPCHWithObjectFile)
for (unsigned I = 0, N = Record.size(); I != N; /*in loop*/)
VTablesToEmit.push_back(ReadDeclID(F, Record, I));
break;

case IMPORTED_MODULES:
if (!F.isModule()) {
// If we aren't loading a module (which has its own exports), make
Expand Down Expand Up @@ -8117,10 +8110,6 @@ void ASTReader::PassInterestingDeclToConsumer(Decl *D) {
Consumer->HandleInterestingDecl(DeclGroupRef(D));
}

void ASTReader::PassVTableToConsumer(CXXRecordDecl *RD) {
Consumer->HandleVTable(RD);
}

void ASTReader::StartTranslationUnit(ASTConsumer *Consumer) {
this->Consumer = Consumer;

Expand Down
7 changes: 0 additions & 7 deletions clang/lib/Serialization/ASTReaderDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4235,13 +4235,6 @@ void ASTReader::PassInterestingDeclsToConsumer() {

// If we add any new potential interesting decl in the last call, consume it.
ConsumingPotentialInterestingDecls();

for (GlobalDeclID ID : VTablesToEmit) {
auto *RD = cast<CXXRecordDecl>(GetDecl(ID));
assert(!RD->shouldEmitInExternalSource());
PassVTableToConsumer(RD);
}
VTablesToEmit.clear();
}

void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord &Record) {
Expand Down
Loading

0 comments on commit 91d40ef

Please sign in to comment.