Skip to content

Commit 2a551ef

Browse files
committed
Clean up external users of GlobalValue::getGUID(StringRef)
See https://discourse.llvm.org/t/rfc-keep-globalvalue-guids-stable/84801 for context. This is a non-functional change which just changes the interface of GlobalValue, in preparation for future functional changes. This part touches a fair few users, so is split out for ease of review. Future changes to the GlobalValue implementation can then be focused purely on that class. This does the following: * Make global identifier computation nominally private. There are a few users we can't remove yet, but we rename the method to make it clear that new users shouldn't be added. * Rename GlobalValue::getGUID(StringRef) to getGUIDAssumingExternalLinkage. This is simply making explicit at the callsite what is currently implicit. * Where possible, migrate users to directly calling getGUID on a GlobalValue instance. * Otherwise, where possible, have them call the newly renamed getGUIDAssumingExternalLinkage, to make the assumption explicit. * There are a few cases where neither of the above are possible, as the caller saves and reconstructs the necessary information to compute the GUID themselves. We want to migrate these callers eventually, but for this first step we leave them be and allow them to call our nominally private global identifier computation method.
1 parent e42ab4c commit 2a551ef

File tree

14 files changed

+47
-33
lines changed

14 files changed

+47
-33
lines changed

llvm/include/llvm/IR/GlobalValue.h

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,12 @@ class GlobalValue : public Constant {
570570
return Name;
571571
}
572572

573+
/// Declare a type to represent a global unique identifier for a global value.
574+
/// This is a 64 bits hash that is used by PGO and ThinLTO to have a compact
575+
/// unique way to identify a symbol.
576+
using GUID = uint64_t;
577+
578+
private:
573579
/// Return the modified name for a global value suitable to be
574580
/// used as the key for a global lookup (e.g. profile or ThinLTO).
575581
/// The value's original name is \c Name and has linkage of type
@@ -582,14 +588,18 @@ class GlobalValue : public Constant {
582588
/// used as the key for a global lookup (e.g. profile or ThinLTO).
583589
std::string getGlobalIdentifier() const;
584590

585-
/// Declare a type to represent a global unique identifier for a global value.
586-
/// This is a 64 bits hash that is used by PGO and ThinLTO to have a compact
587-
/// unique way to identify a symbol.
588-
using GUID = uint64_t;
589-
590591
/// Return a 64-bit global unique ID constructed from global value name
591592
/// (i.e. returned by getGlobalIdentifier()).
592-
static GUID getGUID(StringRef GlobalName);
593+
static GUID getGUID(StringRef GlobalIdentifier);
594+
595+
public:
596+
/// Return a 64-bit global unique ID constructed from the name of a global
597+
/// symbol. Since this call doesn't supply the linkage or defining filename,
598+
/// the GUID computation will assume that the global has external linkage.
599+
static GUID getGUIDAssumingExternalLinkage(StringRef GlobalName) {
600+
return getGUID(
601+
getGlobalIdentifier(GlobalName, GlobalValue::ExternalLinkage, ""));
602+
}
593603

594604
/// Return a 64-bit global unique ID constructed from global value name
595605
/// (i.e. returned by getGlobalIdentifier()).

llvm/include/llvm/IR/ModuleSummaryIndex.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1828,19 +1828,22 @@ class ModuleSummaryIndex {
18281828
/// This accessor can mutate the map and therefore should not be used in
18291829
/// the ThinLTO backends.
18301830
TypeIdSummary &getOrInsertTypeIdSummary(StringRef TypeId) {
1831-
auto TidIter = TypeIdMap.equal_range(GlobalValue::getGUID(TypeId));
1831+
auto TidIter = TypeIdMap.equal_range(
1832+
GlobalValue::getGUIDAssumingExternalLinkage(TypeId));
18321833
for (auto &[GUID, TypeIdPair] : make_range(TidIter))
18331834
if (TypeIdPair.first == TypeId)
18341835
return TypeIdPair.second;
1835-
auto It = TypeIdMap.insert({GlobalValue::getGUID(TypeId),
1836-
{TypeIdSaver.save(TypeId), TypeIdSummary()}});
1836+
auto It =
1837+
TypeIdMap.insert({GlobalValue::getGUIDAssumingExternalLinkage(TypeId),
1838+
{TypeIdSaver.save(TypeId), TypeIdSummary()}});
18371839
return It->second.second;
18381840
}
18391841

18401842
/// This returns either a pointer to the type id summary (if present in the
18411843
/// summary map) or null (if not present). This may be used when importing.
18421844
const TypeIdSummary *getTypeIdSummary(StringRef TypeId) const {
1843-
auto TidIter = TypeIdMap.equal_range(GlobalValue::getGUID(TypeId));
1845+
auto TidIter = TypeIdMap.equal_range(
1846+
GlobalValue::getGUIDAssumingExternalLinkage(TypeId));
18441847
for (const auto &[GUID, TypeIdPair] : make_range(TidIter))
18451848
if (TypeIdPair.first == TypeId)
18461849
return &TypeIdPair.second;

llvm/include/llvm/ProfileData/SampleProf.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1299,7 +1299,7 @@ class FunctionSamples {
12991299
static inline FunctionId getRepInFormat(StringRef Name) {
13001300
if (Name.empty() || !FunctionSamples::UseMD5)
13011301
return FunctionId(Name);
1302-
return FunctionId(Function::getGUID(Name));
1302+
return FunctionId(Function::getGUIDAssumingExternalLinkage(Name));
13031303
}
13041304

13051305
raw_ostream &operator<<(raw_ostream &OS, const FunctionSamples &FS);

llvm/lib/Analysis/CtxProfAnalysis.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ PreservedAnalyses AssignGUIDPass::run(Module &M, ModuleAnalysisManager &MAM) {
5757
GlobalValue::GUID AssignGUIDPass::getGUID(const Function &F) {
5858
if (F.isDeclaration()) {
5959
assert(GlobalValue::isExternalLinkage(F.getLinkage()));
60-
return GlobalValue::getGUID(F.getGlobalIdentifier());
60+
return F.getGUID();
6161
}
6262
auto *MD = F.getMetadata(GUIDMetadataName);
6363
assert(MD && "guid not found for defined function");

llvm/lib/Analysis/ModuleSummaryAnalysis.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,8 @@ static void addIntrinsicToSummary(
222222
auto *TypeId = dyn_cast<MDString>(TypeMDVal->getMetadata());
223223
if (!TypeId)
224224
break;
225-
GlobalValue::GUID Guid = GlobalValue::getGUID(TypeId->getString());
225+
GlobalValue::GUID Guid =
226+
GlobalValue::getGUIDAssumingExternalLinkage(TypeId->getString());
226227

227228
// Produce a summary from type.test intrinsics. We only summarize type.test
228229
// intrinsics that are used other than by an llvm.assume intrinsic.
@@ -250,7 +251,8 @@ static void addIntrinsicToSummary(
250251
auto *TypeId = dyn_cast<MDString>(TypeMDVal->getMetadata());
251252
if (!TypeId)
252253
break;
253-
GlobalValue::GUID Guid = GlobalValue::getGUID(TypeId->getString());
254+
GlobalValue::GUID Guid =
255+
GlobalValue::getGUIDAssumingExternalLinkage(TypeId->getString());
254256

255257
SmallVector<DevirtCallSite, 4> DevirtCalls;
256258
SmallVector<Instruction *, 4> LoadedPtrs;
@@ -906,7 +908,8 @@ static void computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A,
906908

907909
// Set LiveRoot flag on entries matching the given value name.
908910
static void setLiveRoot(ModuleSummaryIndex &Index, StringRef Name) {
909-
if (ValueInfo VI = Index.getValueInfo(GlobalValue::getGUID(Name)))
911+
if (ValueInfo VI =
912+
Index.getValueInfo(GlobalValue::getGUIDAssumingExternalLinkage(Name)))
910913
for (const auto &Summary : VI.getSummaryList())
911914
Summary->setLive(true);
912915
}

llvm/lib/AsmParser/LLParser.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9391,11 +9391,9 @@ bool LLParser::addGlobalValueToIndex(
93919391

93929392
VI = Index->getOrInsertValueInfo(GV);
93939393
} else {
9394-
assert(
9395-
(!GlobalValue::isLocalLinkage(Linkage) || !SourceFileName.empty()) &&
9396-
"Need a source_filename to compute GUID for local");
9397-
GUID = GlobalValue::getGUID(
9398-
GlobalValue::getGlobalIdentifier(Name, Linkage, SourceFileName));
9394+
assert(GlobalValue::isExternalLinkage(Linkage) &&
9395+
"Cannot compute GUID for local");
9396+
GUID = GlobalValue::getGUIDAssumingExternalLinkage(Name);
93999397
VI = Index->getOrInsertValueInfo(GUID, Index->saveString(Name));
94009398
}
94019399
}

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7221,7 +7221,7 @@ void ModuleSummaryIndexBitcodeReader::setValueGUID(
72217221
auto ValueGUID = GlobalValue::getGUID(GlobalId);
72227222
auto OriginalNameID = ValueGUID;
72237223
if (GlobalValue::isLocalLinkage(Linkage))
7224-
OriginalNameID = GlobalValue::getGUID(ValueName);
7224+
OriginalNameID = GlobalValue::getGUIDAssumingExternalLinkage(ValueName);
72257225
if (PrintSummaryGUIDs)
72267226
dbgs() << "GUID " << ValueGUID << "(" << OriginalNameID << ") is "
72277227
<< ValueName << "\n";

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5066,8 +5066,8 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
50665066

50675067
if (!Index.cfiFunctionDefs().empty()) {
50685068
for (auto &S : Index.cfiFunctionDefs()) {
5069-
if (DefOrUseGUIDs.contains(
5070-
GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(S)))) {
5069+
if (DefOrUseGUIDs.contains(GlobalValue::getGUIDAssumingExternalLinkage(
5070+
GlobalValue::dropLLVMManglingEscape(S)))) {
50715071
NameVals.push_back(StrtabBuilder.add(S));
50725072
NameVals.push_back(S.size());
50735073
}
@@ -5080,8 +5080,8 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
50805080

50815081
if (!Index.cfiFunctionDecls().empty()) {
50825082
for (auto &S : Index.cfiFunctionDecls()) {
5083-
if (DefOrUseGUIDs.contains(
5084-
GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(S)))) {
5083+
if (DefOrUseGUIDs.contains(GlobalValue::getGUIDAssumingExternalLinkage(
5084+
GlobalValue::dropLLVMManglingEscape(S)))) {
50855085
NameVals.push_back(StrtabBuilder.add(S));
50865086
NameVals.push_back(S.size());
50875087
}

llvm/lib/CodeGen/PseudoProbeInserter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ class PseudoProbeInserter : public MachineFunctionPass {
129129
private:
130130
uint64_t getFuncGUID(Module *M, DILocation *DL) {
131131
auto Name = DL->getSubprogramLinkageName();
132-
return Function::getGUID(Name);
132+
return Function::getGUIDAssumingExternalLinkage(Name);
133133
}
134134

135135
bool ShouldRun = false;

llvm/lib/IR/AsmWriter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3154,7 +3154,7 @@ void AssemblyWriter::printModuleSummaryIndex() {
31543154

31553155
// Print the TypeIdCompatibleVtableMap entries.
31563156
for (auto &TId : TheIndex->typeIdCompatibleVtableMap()) {
3157-
auto GUID = GlobalValue::getGUID(TId.first);
3157+
auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(TId.first);
31583158
Out << "^" << Machine.getTypeIdCompatibleVtableSlot(TId.first)
31593159
<< " = typeidCompatibleVTable: (name: \"" << TId.first << "\"";
31603160
printTypeIdCompatibleVtableSummary(TId.second);

0 commit comments

Comments
 (0)