From ab443b9da5bffe755af0fd3f2447ad8cdbf0800f Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Sat, 20 Aug 2011 04:39:52 +0000 Subject: [PATCH] Introduce a module visitation function that starts at the top-level modules (those that no other module depends on) and performs a search over all of the modules, visiting a new module only when all of the modules that depend on it have already been visited. The visitor can abort the search for all modules that a module depends on, which allows us to minimize the number of lookups necessary when performing a search. Switch identifier lookup from a linear walk over the set of modules to this module visitation operation. The behavior is the same for simple PCH and chained PCH, but provides the proper search order for modules. Verified with printf debugging, since we don't have enough in place to actually test this. llvm-svn: 138187 --- clang/include/clang/Serialization/ASTReader.h | 20 +++ clang/lib/Serialization/ASTReader.cpp | 135 +++++++++++++++--- clang/test/Modules/Inputs/diamond_left.h | 3 + clang/test/Modules/Inputs/diamond_top.h | 3 + clang/test/Modules/diamond.c | 3 + 5 files changed, 145 insertions(+), 19 deletions(-) diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 5223bd8d1576..3948ea424b1a 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -515,6 +515,26 @@ public: /// \brief Add an in-memory buffer the list of known buffers void addInMemoryBuffer(StringRef FileName, llvm::MemoryBuffer *Buffer); + + /// \brief Visit each of the modules. + /// + /// This routine visits each of the modules, starting with the + /// "root" modules that no other loaded modules depend on, and + /// proceeding to the leaf modules, visiting each module only once + /// during the traversal. + /// + /// This traversal is intended to support various "lookup" + /// operations that can find data in any of the loaded modules. + /// + /// \param Visitor A visitor function that will be invoked with each + /// module and the given user data pointer. The return value must be + /// convertible to bool; when false, the visitation continues to + /// modules that the current module depends on. When true, the + /// visitation skips any modules that the current module depends on. + /// + /// \param UserData User data associated with the visitor object, which + /// will be passed along to the visitor. + void visit(bool (*Visitor)(Module &M, void *UserData), void *UserData); }; } // end namespace serialization diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index cc239c35eade..d3400bae5aeb 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -49,7 +49,7 @@ #include #include #include -#include +#include using namespace clang; using namespace clang::serialization; @@ -4574,25 +4574,47 @@ void ASTReader::InitializeSema(Sema &S) { } } -IdentifierInfo* ASTReader::get(const char *NameStart, const char *NameEnd) { - // Try to find this name within our on-disk hash tables. We start with the - // most recent one, since that one contains the most up-to-date info. - for (ModuleIterator I = ModuleMgr.begin(), E = ModuleMgr.end(); I != E; ++I) { - ASTIdentifierLookupTable *IdTable - = (ASTIdentifierLookupTable *)(*I)->IdentifierLookupTable; - if (!IdTable) - continue; - std::pair Key(NameStart, NameEnd - NameStart); - ASTIdentifierLookupTable::iterator Pos = IdTable->find(Key); - if (Pos == IdTable->end()) - continue; +namespace { + /// \brief Visitor class used to look up identifirs in + class IdentifierLookupVisitor { + StringRef Name; + IdentifierInfo *Found; + public: + explicit IdentifierLookupVisitor(StringRef Name) : Name(Name), Found() { } - // Dereferencing the iterator has the effect of building the - // IdentifierInfo node and populating it with the various - // declarations it needs. - return *Pos; - } - return 0; + static bool visit(Module &M, void *UserData) { + IdentifierLookupVisitor *This + = static_cast(UserData); + + ASTIdentifierLookupTable *IdTable + = (ASTIdentifierLookupTable *)M.IdentifierLookupTable; + if (!IdTable) + return false; + + std::pair Key(This->Name.begin(), + This->Name.size()); + ASTIdentifierLookupTable::iterator Pos = IdTable->find(Key); + if (Pos == IdTable->end()) + return false; + + // Dereferencing the iterator has the effect of building the + // IdentifierInfo node and populating it with the various + // declarations it needs. + This->Found = *Pos; + return true; + } + + // \brief Retrieve the identifier info found within the module + // files. + IdentifierInfo *getIdentifierInfo() const { return Found; } + }; +} + +IdentifierInfo* ASTReader::get(const char *NameStart, const char *NameEnd) { + std::string Name(NameStart, NameEnd); + IdentifierLookupVisitor Visitor(StringRef(NameStart, NameEnd - NameStart)); + ModuleMgr.visit(IdentifierLookupVisitor::visit, &Visitor); + return Visitor.getIdentifierInfo(); } namespace clang { @@ -5606,6 +5628,9 @@ ASTReader::~ASTReader() { } } +//===----------------------------------------------------------------------===// +// Module implementation +//===----------------------------------------------------------------------===// Module::Module(ModuleKind Kind) : Kind(Kind), DirectlyImported(false), SizeInBits(0), LocalNumSLocEntries(0), SLocEntryBaseID(0), @@ -5695,6 +5720,10 @@ void Module::dump() { dumpLocalRemap("Decl ID local -> global map", DeclRemap); } +//===----------------------------------------------------------------------===// +// Module manager implementation +//===----------------------------------------------------------------------===// + Module *ModuleManager::lookup(StringRef Name) { const FileEntry *Entry = FileMgr.getFile(Name); return Modules[Entry]; @@ -5772,3 +5801,71 @@ ModuleManager::~ModuleManager() { for (unsigned i = 0, e = Chain.size(); i != e; ++i) delete Chain[e - i - 1]; } + +void ModuleManager::visit(bool (*Visitor)(Module &M, void *UserData), + void *UserData) { + unsigned N = size(); + + // Record the number of incoming edges for each module. When we + // encounter a module with no incoming edges, push it into the queue + // to seed the queue. + SmallVector Queue; + Queue.reserve(N); + llvm::DenseMap UnusedIncomingEdges; + for (ModuleIterator M = begin(), MEnd = end(); M != MEnd; ++M) { + if (unsigned Size = (*M)->ImportedBy.size()) + UnusedIncomingEdges[*M] = Size; + else + Queue.push_back(*M); + } + + llvm::SmallPtrSet Skipped; + unsigned QueueStart = 0; + while (QueueStart < Queue.size()) { + Module *CurrentModule = Queue[QueueStart++]; + + // Check whether this module should be skipped. + if (Skipped.count(CurrentModule)) + continue; + + if (Visitor(*CurrentModule, UserData)) { + // The visitor has requested that cut off visitation of any + // module that the current module depends on. To indicate this + // behavior, we mark all of the reachable modules as having N + // incoming edges (which is impossible otherwise). + SmallVector Stack; + Stack.push_back(CurrentModule); + Skipped.insert(CurrentModule); + while (!Stack.empty()) { + Module *NextModule = Stack.back(); + Stack.pop_back(); + + // For any module that this module depends on, push it on the + // stack (if it hasn't already been marked as visited). + for (llvm::SetVector::iterator + M = NextModule->Imports.begin(), + MEnd = NextModule->Imports.end(); + M != MEnd; ++M) { + if (Skipped.insert(*M)) + Stack.push_back(*M); + } + } + continue; + } + + // For any module that this module depends on, push it on the + // stack (if it hasn't already been marked as visited). + for (llvm::SetVector::iterator M = CurrentModule->Imports.begin(), + MEnd = CurrentModule->Imports.end(); + M != MEnd; ++M) { + + // Remove our current module as an impediment to visiting the + // module we depend on. If we were the last unvisited module + // that depends on this particular module, push it into the + // queue to be visited. + unsigned &NumUnusedEdges = UnusedIncomingEdges[*M]; + if (NumUnusedEdges && (--NumUnusedEdges == 0)) + Queue.push_back(*M); + } + } +} diff --git a/clang/test/Modules/Inputs/diamond_left.h b/clang/test/Modules/Inputs/diamond_left.h index dfdd803a0ad7..3a97094c08f7 100644 --- a/clang/test/Modules/Inputs/diamond_left.h +++ b/clang/test/Modules/Inputs/diamond_left.h @@ -1 +1,4 @@ float left(float *); + +int top_left(char *c); + diff --git a/clang/test/Modules/Inputs/diamond_top.h b/clang/test/Modules/Inputs/diamond_top.h index 189687aab12f..34998cd4324b 100644 --- a/clang/test/Modules/Inputs/diamond_top.h +++ b/clang/test/Modules/Inputs/diamond_top.h @@ -1 +1,4 @@ int top(int *); + +int top_left(char *c); + diff --git a/clang/test/Modules/diamond.c b/clang/test/Modules/diamond.c index fdec7b3aab3e..220a27903800 100644 --- a/clang/test/Modules/diamond.c +++ b/clang/test/Modules/diamond.c @@ -5,6 +5,9 @@ void test_diamond(int i, float f, double d, char c) { right(&d); bottom(&c); bottom(&d); // expected-warning{{incompatible pointer types passing 'double *' to parameter of type 'char *'}} + + // Names in multiple places in the diamond. + top_left(&c); } // RUN: %clang_cc1 -emit-pch -o %t_top.h.pch %S/Inputs/diamond_top.h -- GitLab