Skip to content

Avoid need for SLocEntryLoaded BitVector #67960

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions clang/include/clang/Basic/SourceManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -474,9 +474,10 @@ static_assert(sizeof(FileInfo) <= sizeof(ExpansionInfo),
/// SourceManager keeps an array of these objects, and they are uniquely
/// identified by the FileID datatype.
class SLocEntry {
static constexpr int OffsetBits = 8 * sizeof(SourceLocation::UIntTy) - 1;
static constexpr int OffsetBits = 8 * sizeof(SourceLocation::UIntTy) - 2;
SourceLocation::UIntTy Offset : OffsetBits;
SourceLocation::UIntTy IsExpansion : 1;
SourceLocation::UIntTy IsLoaded : 1;
union {
FileInfo File;
ExpansionInfo Expansion;
Expand All @@ -489,6 +490,8 @@ class SLocEntry {

bool isExpansion() const { return IsExpansion; }
bool isFile() const { return !isExpansion(); }
bool isLoaded() const { return IsLoaded; }
void setLoaded(bool Value) { IsLoaded = Value; }

const FileInfo &getFile() const {
assert(isFile() && "Not a file SLocEntry!");
Expand Down Expand Up @@ -716,13 +719,7 @@ class SourceManager : public RefCountedBase<SourceManager> {
/// The highest possible offset is 2^31-1 (2^63-1 for 64-bit source
/// locations), so CurrentLoadedOffset starts at 2^31 (2^63 resp.).
static const SourceLocation::UIntTy MaxLoadedOffset =
1ULL << (8 * sizeof(SourceLocation::UIntTy) - 1);

/// A bitmap that indicates whether the entries of LoadedSLocEntryTable
/// have already been loaded from the external source.
///
/// Same indexing as LoadedSLocEntryTable.
llvm::BitVector SLocEntryLoaded;
1ULL << (8 * sizeof(SourceLocation::UIntTy) - 2);

/// An external source for source location entries.
ExternalSLocEntrySource *ExternalSLocEntries = nullptr;
Expand Down Expand Up @@ -1710,7 +1707,8 @@ class SourceManager : public RefCountedBase<SourceManager> {
const SrcMgr::SLocEntry &getLoadedSLocEntry(unsigned Index,
bool *Invalid = nullptr) const {
assert(Index < LoadedSLocEntryTable.size() && "Invalid index");
if (SLocEntryLoaded[Index])
if (LoadedSLocEntryTable.isMaterialized(Index) &&
LoadedSLocEntryTable[Index].isLoaded())
return LoadedSLocEntryTable[Index];
return loadSLocEntry(Index, Invalid);
}
Expand Down
27 changes: 16 additions & 11 deletions clang/lib/Basic/SourceManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,6 @@ void SourceManager::clearIDTables() {
MainFileID = FileID();
LocalSLocEntryTable.clear();
LoadedSLocEntryTable.clear();
SLocEntryLoaded.clear();
LastLineNoFileIDQuery = FileID();
LastLineNoContentCache = nullptr;
LastFileIDLookup = FileID();
Expand Down Expand Up @@ -373,7 +372,8 @@ void SourceManager::initializeForReplay(const SourceManager &Old) {

// Ensure all SLocEntries are loaded from the external source.
for (unsigned I = 0, N = Old.LoadedSLocEntryTable.size(); I != N; ++I)
if (!Old.SLocEntryLoaded[I])
if (!Old.LoadedSLocEntryTable.isMaterialized(I) ||
!Old.LoadedSLocEntryTable[I].isLoaded())
Old.loadSLocEntry(I, nullptr);

// Inherit any content cache data from the old source manager.
Expand Down Expand Up @@ -430,12 +430,14 @@ ContentCache &SourceManager::createMemBufferContentCache(

const SrcMgr::SLocEntry &SourceManager::loadSLocEntry(unsigned Index,
bool *Invalid) const {
assert(!SLocEntryLoaded[Index]);
assert(!LoadedSLocEntryTable.isMaterialized(Index) ||
!LoadedSLocEntryTable[Index].isLoaded());
if (ExternalSLocEntries->ReadSLocEntry(-(static_cast<int>(Index) + 2))) {
if (Invalid)
*Invalid = true;
// If the file of the SLocEntry changed we could still have loaded it.
if (!SLocEntryLoaded[Index]) {
if (!LoadedSLocEntryTable.isMaterialized(Index) ||
!LoadedSLocEntryTable[Index].isLoaded()) {
// Try to recover; create a SLocEntry so the rest of clang can handle it.
if (!FakeSLocEntryForRecovery)
FakeSLocEntryForRecovery = std::make_unique<SLocEntry>(SLocEntry::get(
Expand All @@ -458,7 +460,6 @@ SourceManager::AllocateLoadedSLocEntries(unsigned NumSLocEntries,
return std::make_pair(0, 0);
}
LoadedSLocEntryTable.resize(LoadedSLocEntryTable.size() + NumSLocEntries);
SLocEntryLoaded.resize(LoadedSLocEntryTable.size());
CurrentLoadedOffset -= TotalSize;
int ID = LoadedSLocEntryTable.size();
return std::make_pair(-ID - 1, CurrentLoadedOffset);
Expand Down Expand Up @@ -604,10 +605,12 @@ FileID SourceManager::createFileIDImpl(ContentCache &File, StringRef Filename,
assert(LoadedID != -1 && "Loading sentinel FileID");
unsigned Index = unsigned(-LoadedID) - 2;
assert(Index < LoadedSLocEntryTable.size() && "FileID out of range");
assert(!SLocEntryLoaded[Index] && "FileID already loaded");
assert((!LoadedSLocEntryTable.isMaterialized(Index) ||
!LoadedSLocEntryTable[Index].isLoaded()) &&
"FileID already loaded");
LoadedSLocEntryTable[Index] = SLocEntry::get(
LoadedOffset, FileInfo::get(IncludePos, File, FileCharacter, Filename));
SLocEntryLoaded[Index] = true;
LoadedSLocEntryTable[Index].setLoaded(true);
return FileID::get(LoadedID);
}
unsigned FileSize = File.getSize();
Expand Down Expand Up @@ -665,9 +668,11 @@ SourceManager::createExpansionLocImpl(const ExpansionInfo &Info,
assert(LoadedID != -1 && "Loading sentinel FileID");
unsigned Index = unsigned(-LoadedID) - 2;
assert(Index < LoadedSLocEntryTable.size() && "FileID out of range");
assert(!SLocEntryLoaded[Index] && "FileID already loaded");
assert((!LoadedSLocEntryTable.isMaterialized(Index) ||
!LoadedSLocEntryTable[Index].isLoaded()) &&
"FileID already loaded");
LoadedSLocEntryTable[Index] = SLocEntry::get(LoadedOffset, Info);
SLocEntryLoaded[Index] = true;
LoadedSLocEntryTable[Index].setLoaded(true);
return SourceLocation::getMacroLoc(LoadedOffset);
}
LocalSLocEntryTable.push_back(SLocEntry::get(NextLocalOffset, Info));
Expand Down Expand Up @@ -2223,7 +2228,8 @@ LLVM_DUMP_METHOD void SourceManager::dump() const {
std::optional<SourceLocation::UIntTy> NextStart;
for (unsigned Index = 0; Index != LoadedSLocEntryTable.size(); ++Index) {
int ID = -(int)Index - 2;
if (SLocEntryLoaded[Index]) {
if (LoadedSLocEntryTable.isMaterialized(Index) &&
LoadedSLocEntryTable[Index].isLoaded()) {
DumpSLocEntry(ID, LoadedSLocEntryTable[Index], NextStart);
NextStart = LoadedSLocEntryTable[Index].getOffset();
} else {
Expand Down Expand Up @@ -2346,7 +2352,6 @@ size_t SourceManager::getDataStructureSizes() const {
size_t size = llvm::capacity_in_bytes(MemBufferInfos) +
llvm::capacity_in_bytes(LocalSLocEntryTable) +
llvm::capacity_in_bytes(LoadedSLocEntryTable) +
llvm::capacity_in_bytes(SLocEntryLoaded) +
llvm::capacity_in_bytes(FileInfos);

if (OverriddenFilesInfo)
Expand Down
8 changes: 8 additions & 0 deletions llvm/include/llvm/ADT/PagedVector.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,14 @@ template <typename T, size_t PageSize = 1024 / sizeof(T)> class PagedVector {
/// Return the size of the vector.
[[nodiscard]] size_t size() const { return Size; }

/// Return true if the element at `Index` belongs to a page which was already
/// materialized, i.e., had at least one element accessed.
[[nodiscard]] bool isMaterialized(size_t Index) const {
assert(Index < Size);
assert(Index / PageSize < PageToDataPtrs.size());
return PageToDataPtrs[Index / PageSize];
}

/// Resize the vector. Notice that the constructor of the elements will not
/// be invoked until an element of a given page is accessed, at which point
/// all the elements of the page will be constructed.
Expand Down
14 changes: 14 additions & 0 deletions llvm/unittests/ADT/PagedVectorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,20 @@ TEST(PagedVectorTest, FillNonTrivialConstructor) {
EXPECT_EQ(std::distance(V.materialized_begin(), V.materialized_end()), 10LL);
}

// Test that isMaterialized returns true for all the elements
// of the page, not only the one that was accessed.
TEST(PagedVectorTest, IsMaterialized) {
PagedVector<int, 10> V;
V.resize(20);
EXPECT_FALSE(V.isMaterialized(0));
EXPECT_FALSE(V.isMaterialized(1));
EXPECT_FALSE(V.isMaterialized(10));
V[0] = 0;
EXPECT_TRUE(V.isMaterialized(0));
EXPECT_TRUE(V.isMaterialized(1));
EXPECT_FALSE(V.isMaterialized(10));
}

// Elements are constructed, destructed in pages, so we expect
// the number of constructed / destructed elements to be a multiple of the
// page size and the constructor is invoked when the page is actually accessed
Expand Down