Skip to content

Commit 0341fb6

Browse files
[ThinLTO] Avoid creating map entries on lookup (NFCI) (#164873)
We could inadvertently create new entries in the PrevailingModuleForGUID map during lookup, which was always using operator[]. In most cases we will have one for external symbols, but not in cases where the prevailing copy is in a native object. Or if this happened to be looked up for a local. Make the map private and create and use accessors.
1 parent bcee0ee commit 0341fb6

File tree

2 files changed

+19
-6
lines changed

2 files changed

+19
-6
lines changed

llvm/include/llvm/LTO/LTO.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,19 @@ class LTO {
462462
ModuleMapType ModuleMap;
463463
// The bitcode modules to compile, if specified by the LTO Config.
464464
std::optional<ModuleMapType> ModulesToCompile;
465+
466+
void setPrevailingModuleForGUID(GlobalValue::GUID GUID, StringRef Module) {
467+
PrevailingModuleForGUID[GUID] = Module;
468+
}
469+
bool isPrevailingModuleForGUID(GlobalValue::GUID GUID,
470+
StringRef Module) const {
471+
auto It = PrevailingModuleForGUID.find(GUID);
472+
return It != PrevailingModuleForGUID.end() && It->second == Module;
473+
}
474+
475+
private:
476+
// Make this private so all accesses must go through above accessor methods
477+
// to avoid inadvertently creating new entries on lookups.
465478
DenseMap<GlobalValue::GUID, StringRef> PrevailingModuleForGUID;
466479
} ThinLTO;
467480

llvm/lib/LTO/LTO.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,15 +1086,15 @@ LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
10861086
GlobalValue::getGlobalIdentifier(Sym.getIRName(),
10871087
GlobalValue::ExternalLinkage, ""));
10881088
if (R.Prevailing)
1089-
ThinLTO.PrevailingModuleForGUID[GUID] = BM.getModuleIdentifier();
1089+
ThinLTO.setPrevailingModuleForGUID(GUID, BM.getModuleIdentifier());
10901090
}
10911091
}
10921092

10931093
if (Error Err =
10941094
BM.readSummary(ThinLTO.CombinedIndex, BM.getModuleIdentifier(),
10951095
[&](GlobalValue::GUID GUID) {
1096-
return ThinLTO.PrevailingModuleForGUID[GUID] ==
1097-
BM.getModuleIdentifier();
1096+
return ThinLTO.isPrevailingModuleForGUID(
1097+
GUID, BM.getModuleIdentifier());
10981098
}))
10991099
return Err;
11001100
LLVM_DEBUG(dbgs() << "Module " << BM.getModuleIdentifier() << "\n");
@@ -1108,8 +1108,8 @@ LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
11081108
GlobalValue::getGlobalIdentifier(Sym.getIRName(),
11091109
GlobalValue::ExternalLinkage, ""));
11101110
if (R.Prevailing) {
1111-
assert(ThinLTO.PrevailingModuleForGUID[GUID] ==
1112-
BM.getModuleIdentifier());
1111+
assert(
1112+
ThinLTO.isPrevailingModuleForGUID(GUID, BM.getModuleIdentifier()));
11131113

11141114
// For linker redefined symbols (via --wrap or --defsym) we want to
11151115
// switch the linkage to `weak` to prevent IPOs from happening.
@@ -1988,7 +1988,7 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
19881988
LocalWPDTargetsMap);
19891989

19901990
auto isPrevailing = [&](GlobalValue::GUID GUID, const GlobalValueSummary *S) {
1991-
return ThinLTO.PrevailingModuleForGUID[GUID] == S->modulePath();
1991+
return ThinLTO.isPrevailingModuleForGUID(GUID, S->modulePath());
19921992
};
19931993
if (EnableMemProfContextDisambiguation) {
19941994
MemProfContextDisambiguation ContextDisambiguation;

0 commit comments

Comments
 (0)