Skip to content

Commit 0b5b2ad

Browse files
teresajohnsonnga888
authored andcommitted
[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. (cherry picked from commit 0341fb6)
1 parent 4aaab27 commit 0b5b2ad

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
@@ -464,6 +464,19 @@ class LTO {
464464
ModuleMapType ModuleMap;
465465
// The bitcode modules to compile, if specified by the LTO Config.
466466
std::optional<ModuleMapType> ModulesToCompile;
467+
468+
void setPrevailingModuleForGUID(GlobalValue::GUID GUID, StringRef Module) {
469+
PrevailingModuleForGUID[GUID] = Module;
470+
}
471+
bool isPrevailingModuleForGUID(GlobalValue::GUID GUID,
472+
StringRef Module) const {
473+
auto It = PrevailingModuleForGUID.find(GUID);
474+
return It != PrevailingModuleForGUID.end() && It->second == Module;
475+
}
476+
477+
private:
478+
// Make this private so all accesses must go through above accessor methods
479+
// to avoid inadvertently creating new entries on lookups.
467480
DenseMap<GlobalValue::GUID, StringRef> PrevailingModuleForGUID;
468481
} ThinLTO;
469482

llvm/lib/LTO/LTO.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,15 +1045,15 @@ Error LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
10451045
GlobalValue::getGlobalIdentifier(Sym.getIRName(),
10461046
GlobalValue::ExternalLinkage, ""));
10471047
if (Res.Prevailing)
1048-
ThinLTO.PrevailingModuleForGUID[GUID] = BM.getModuleIdentifier();
1048+
ThinLTO.setPrevailingModuleForGUID(GUID, BM.getModuleIdentifier());
10491049
}
10501050
}
10511051

10521052
if (Error Err =
10531053
BM.readSummary(ThinLTO.CombinedIndex, BM.getModuleIdentifier(),
10541054
[&](GlobalValue::GUID GUID) {
1055-
return ThinLTO.PrevailingModuleForGUID[GUID] ==
1056-
BM.getModuleIdentifier();
1055+
return ThinLTO.isPrevailingModuleForGUID(
1056+
GUID, BM.getModuleIdentifier());
10571057
}))
10581058
return Err;
10591059
LLVM_DEBUG(dbgs() << "Module " << BM.getModuleIdentifier() << "\n");
@@ -1067,8 +1067,8 @@ Error LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
10671067
GlobalValue::getGlobalIdentifier(Sym.getIRName(),
10681068
GlobalValue::ExternalLinkage, ""));
10691069
if (Res.Prevailing) {
1070-
assert(ThinLTO.PrevailingModuleForGUID[GUID] ==
1071-
BM.getModuleIdentifier());
1070+
assert(
1071+
ThinLTO.isPrevailingModuleForGUID(GUID, BM.getModuleIdentifier()));
10721072

10731073
// For linker redefined symbols (via --wrap or --defsym) we want to
10741074
// switch the linkage to `weak` to prevent IPOs from happening.
@@ -1974,7 +1974,7 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
19741974
LocalWPDTargetsMap);
19751975

19761976
auto isPrevailing = [&](GlobalValue::GUID GUID, const GlobalValueSummary *S) {
1977-
return ThinLTO.PrevailingModuleForGUID[GUID] == S->modulePath();
1977+
return ThinLTO.isPrevailingModuleForGUID(GUID, S->modulePath());
19781978
};
19791979
if (EnableMemProfContextDisambiguation) {
19801980
MemProfContextDisambiguation ContextDisambiguation;

0 commit comments

Comments
 (0)