nickdesaulniers added inline comments.
================
Comment at: clang/include/clang/Basic/SourceManager.h:696
+ /// An in-parallel SlocEntry offset table, merely used for speeding up the
+ /// FileID lookup.
+ SmallVector<SourceLocation::UIntTy> LocalLocOffsetTable;
----------------
Add to this comment that the size is expected to stay in sync with
`LocalSLocEntryTable`.
================
Comment at: clang/include/clang/Basic/SourceManager.h:697
+ /// FileID lookup.
+ SmallVector<SourceLocation::UIntTy> LocalLocOffsetTable;
----------------
`LocalSLocEntryTable` is a vec of `SrcMgr::SLocEntry`. `SrcMgr::SLocEntry`'s
`Offset` is:
```
static constexpr int OffsetBits = 8 * sizeof(SourceLocation::UIntTy) - 1;
SourceLocation::UIntTy Offset : OffsetBits;
```
So this is a vector of 32b values vs the 24b values we need. Does packing this
by declaring a struct improve locality further?
```
diff --git a/clang/include/clang/Basic/SourceManager.h
b/clang/include/clang/Basic/SourceManager.h
index 70ba85bbe948..518c6ed59120 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -692,9 +692,15 @@ class SourceManager : public RefCountedBase<SourceManager>
{
/// Positive FileIDs are indexes into this table. Entry 0 indicates an
invalid
/// expansion.
SmallVector<SrcMgr::SLocEntry, 0> LocalSLocEntryTable;
+
+ struct SMOffset {
+ SourceLocation::UIntTy Offset : 24;
+ SMOffset (SourceLocation::UIntTy O): Offset(O){}
+ operator SourceLocation::UIntTy() const { return Offset; }
+ };
/// An in-parallel SlocEntry offset table, merely used for speeding up the
/// FileID lookup.
- SmallVector<SourceLocation::UIntTy> LocalLocOffsetTable;
+ SmallVector<SMOffset> LocalLocOffsetTable;
/// The table of SLocEntries that are loaded from other modules.
///
```
================
Comment at: clang/lib/Basic/SourceManager.cpp:674
LocalSLocEntryTable.push_back(SLocEntry::get(NextLocalOffset, Info));
+ LocalLocOffsetTable.push_back(NextLocalOffset);
assert(NextLocalOffset + Length + 1 > NextLocalOffset &&
----------------
should we add some asserts that `LocalSLocEntryTable.size() ==
LocalLocOffsetTable.size()` somewhere?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135440/new/
https://reviews.llvm.org/D135440
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
