Clean up external users of GlobalValue::getGUID(StringRef) #129644
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. |
orodley
force-pushed
the
users/orodley/guid-1
branch
from
2a551ef to
1cf9bc2
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
orodley
force-pushed
the
users/orodley/guid-1
branch
2 times, most recently
from
2f7c1d8 to
cee3576
Compare
orodley
force-pushed
the
users/orodley/guid-1
branch
5 times, most recently
from
5c54156 to
22dd4ce
Compare
orodley
requested review from
aaupov,
maksfb,
rafaelauler,
ayermolo,
dcci and
yota9
as code owners
llvmbot
added
backend:PowerPC
PGO
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-lto Author: Owen Rodley (orodley) ChangesSee https://discourse.llvm.org/t/rfc-keep-globalvalue-guids-stable/84801 This is a non-functional change which just changes the interface of This does the following:
There are a few cases where neither of the above are possible, as the Patch is 41.29 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/129644.diff 31 Files Affected:
diff --git a/bolt/lib/Rewrite/PseudoProbeRewriter.cpp b/bolt/lib/Rewrite/PseudoProbeRewriter.cpp
index 9d6e914624a33..ee021fee3cea5 100644
--- a/bolt/lib/Rewrite/PseudoProbeRewriter.cpp
+++ b/bolt/lib/Rewrite/PseudoProbeRewriter.cpp
@@ -147,7 +147,7 @@ void PseudoProbeRewriter::parsePseudoProbe(bool ProfiledOnly) {
if (!Name)
continue;
SymName = *Name;
- uint64_t GUID = Function::getGUID(SymName);
+ uint64_t GUID = Function::getGUIDAssumingExternalLinkage(SymName);
FuncStartAddrs[GUID] = F->getAddress();
if (ProfiledOnly && HasProfile)
GuidFilter.insert(GUID);
@@ -173,7 +173,7 @@ void PseudoProbeRewriter::parsePseudoProbe(bool ProfiledOnly) {
const GUIDProbeFunctionMap &GUID2Func = ProbeDecoder.getGUID2FuncDescMap();
// Checks GUID in GUID2Func and returns it if it's present or null otherwise.
auto checkGUID = [&](StringRef SymName) -> uint64_t {
- uint64_t GUID = Function::getGUID(SymName);
+ uint64_t GUID = Function::getGUIDAssumingExternalLinkage(SymName);
if (GUID2Func.find(GUID) == GUID2Func.end())
return 0;
return GUID;
@@ -435,7 +435,7 @@ void PseudoProbeRewriter::encodePseudoProbes() {
for (const BinaryFunction *F : BC.getAllBinaryFunctions()) {
const uint64_t Addr =
F->isEmitted() ? F->getOutputAddress() : F->getAddress();
- FuncStartAddrs[Function::getGUID(
+ FuncStartAddrs[Function::getGUIDAssumingExternalLinkage(
NameResolver::restore(F->getOneName()))] = Addr;
}
DummyDecoder.buildAddress2ProbeMap(
diff --git a/llvm/examples/OrcV2Examples/LLJITWithThinLTOSummaries/LLJITWithThinLTOSummaries.cpp b/llvm/examples/OrcV2Examples/LLJITWithThinLTOSummaries/LLJITWithThinLTOSummaries.cpp
index 78152af052c07..c55aa73d50277 100644
--- a/llvm/examples/OrcV2Examples/LLJITWithThinLTOSummaries/LLJITWithThinLTOSummaries.cpp
+++ b/llvm/examples/OrcV2Examples/LLJITWithThinLTOSummaries/LLJITWithThinLTOSummaries.cpp
@@ -77,7 +77,9 @@ class DuplicateDefinitionInSummary
void log(raw_ostream &OS) const override {
OS << "Duplicate symbol for global value '" << GlobalValueName
- << "' (GUID: " << GlobalValue::getGUID(GlobalValueName) << ") in:\n";
+ << "' (GUID: "
+ << GlobalValue::getGUIDAssumingExternalLinkage(GlobalValueName)
+ << ") in:\n";
for (const std::string &Path : ModulePaths) {
OS << " " << Path << "\n";
}
@@ -110,8 +112,9 @@ class DefinitionNotFoundInSummary
}
void log(raw_ostream &OS) const override {
- OS << "No symbol for global value '" << GlobalValueName
- << "' (GUID: " << GlobalValue::getGUID(GlobalValueName) << ") in:\n";
+ OS << "No symbol for global value '" << GlobalValueName << "' (GUID: "
+ << GlobalValue::getGUIDAssumingExternalLinkage(GlobalValueName)
+ << ") in:\n";
for (const std::string &Path : ModulePaths) {
OS << " " << Path << "\n";
}
@@ -135,7 +138,8 @@ char DefinitionNotFoundInSummary::ID = 0;
Expected<StringRef> getMainModulePath(StringRef FunctionName,
ModuleSummaryIndex &Index) {
// Summaries use unmangled names.
- GlobalValue::GUID G = GlobalValue::getGUID(FunctionName);
+ GlobalValue::GUID G =
+ GlobalValue::getGUIDAssumingExternalLinkage(FunctionName);
ValueInfo VI = Index.getValueInfo(G);
// We need a unique definition, otherwise don't try further.
diff --git a/llvm/include/llvm/IR/GlobalValue.h b/llvm/include/llvm/IR/GlobalValue.h
index 2176e2c2cfbfc..04907b3d17eb0 100644
--- a/llvm/include/llvm/IR/GlobalValue.h
+++ b/llvm/include/llvm/IR/GlobalValue.h
@@ -570,6 +570,11 @@ class GlobalValue : public Constant {
return Name;
}
+ /// Declare a type to represent a global unique identifier for a global value.
+ /// This is a 64 bits hash that is used by PGO and ThinLTO to have a compact
+ /// unique way to identify a symbol.
+ using GUID = uint64_t;
+
/// Return the modified name for a global value suitable to be
/// used as the key for a global lookup (e.g. profile or ThinLTO).
/// The value's original name is \c Name and has linkage of type
@@ -578,18 +583,23 @@ class GlobalValue : public Constant {
GlobalValue::LinkageTypes Linkage,
StringRef FileName);
+private:
/// Return the modified name for this global value suitable to be
/// used as the key for a global lookup (e.g. profile or ThinLTO).
std::string getGlobalIdentifier() const;
- /// Declare a type to represent a global unique identifier for a global value.
- /// This is a 64 bits hash that is used by PGO and ThinLTO to have a compact
- /// unique way to identify a symbol.
- using GUID = uint64_t;
-
/// Return a 64-bit global unique ID constructed from global value name
/// (i.e. returned by getGlobalIdentifier()).
- static GUID getGUID(StringRef GlobalName);
+ static GUID getGUID(StringRef GlobalIdentifier);
+
+public:
+ /// Return a 64-bit global unique ID constructed from the name of a global
+ /// symbol. Since this call doesn't supply the linkage or defining filename,
+ /// the GUID computation will assume that the global has external linkage.
+ static GUID getGUIDAssumingExternalLinkage(StringRef GlobalName) {
+ return getGUID(
+ getGlobalIdentifier(GlobalName, GlobalValue::ExternalLinkage, ""));
+ }
/// Return a 64-bit global unique ID constructed from global value name
/// (i.e. returned by getGlobalIdentifier()).
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 2650456c49841..32b4fa630b7bb 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1339,14 +1339,14 @@ class CfiFunctionIndex {
template <typename... Args> void emplace(Args &&...A) {
StringRef S(std::forward<Args>(A)...);
- GlobalValue::GUID GUID =
- GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(S));
+ GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::dropLLVMManglingEscape(S));
Index[GUID].emplace(S);
}
size_t count(StringRef S) const {
- GlobalValue::GUID GUID =
- GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(S));
+ GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::dropLLVMManglingEscape(S));
auto I = Index.find(GUID);
if (I == Index.end())
return 0;
@@ -1749,8 +1749,10 @@ class ModuleSummaryIndex {
/// Add a global value summary for a value of the given name.
void addGlobalValueSummary(StringRef ValueName,
std::unique_ptr<GlobalValueSummary> Summary) {
- addGlobalValueSummary(getOrInsertValueInfo(GlobalValue::getGUID(ValueName)),
- std::move(Summary));
+ addGlobalValueSummary(
+ getOrInsertValueInfo(
+ GlobalValue::getGUIDAssumingExternalLinkage(ValueName)),
+ std::move(Summary));
}
/// Add a global value summary for the given ValueInfo.
@@ -1887,19 +1889,22 @@ class ModuleSummaryIndex {
/// This accessor can mutate the map and therefore should not be used in
/// the ThinLTO backends.
TypeIdSummary &getOrInsertTypeIdSummary(StringRef TypeId) {
- auto TidIter = TypeIdMap.equal_range(GlobalValue::getGUID(TypeId));
+ auto TidIter = TypeIdMap.equal_range(
+ GlobalValue::getGUIDAssumingExternalLinkage(TypeId));
for (auto &[GUID, TypeIdPair] : make_range(TidIter))
if (TypeIdPair.first == TypeId)
return TypeIdPair.second;
- auto It = TypeIdMap.insert({GlobalValue::getGUID(TypeId),
- {TypeIdSaver.save(TypeId), TypeIdSummary()}});
+ auto It =
+ TypeIdMap.insert({GlobalValue::getGUIDAssumingExternalLinkage(TypeId),
+ {TypeIdSaver.save(TypeId), TypeIdSummary()}});
return It->second.second;
}
/// This returns either a pointer to the type id summary (if present in the
/// summary map) or null (if not present). This may be used when importing.
const TypeIdSummary *getTypeIdSummary(StringRef TypeId) const {
- auto TidIter = TypeIdMap.equal_range(GlobalValue::getGUID(TypeId));
+ auto TidIter = TypeIdMap.equal_range(
+ GlobalValue::getGUIDAssumingExternalLinkage(TypeId));
for (const auto &[GUID, TypeIdPair] : make_range(TidIter))
if (TypeIdPair.first == TypeId)
return &TypeIdPair.second;
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
index d5a91763a981c..531de514822e8 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
@@ -314,7 +314,7 @@ template <> struct CustomMappingTraits<TypeIdSummaryMapTy> {
static void inputOne(IO &io, StringRef Key, TypeIdSummaryMapTy &V) {
TypeIdSummary TId;
io.mapRequired(Key.str().c_str(), TId);
- V.insert({GlobalValue::getGUID(Key), {Key, TId}});
+ V.insert({GlobalValue::getGUIDAssumingExternalLinkage(Key), {Key, TId}});
}
static void output(IO &io, TypeIdSummaryMapTy &V) {
for (auto &TidIter : V)
diff --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h
index e7b154dff0697..008d464d0c582 100644
--- a/llvm/include/llvm/ProfileData/SampleProf.h
+++ b/llvm/include/llvm/ProfileData/SampleProf.h
@@ -1299,7 +1299,7 @@ class FunctionSamples {
static inline FunctionId getRepInFormat(StringRef Name) {
if (Name.empty() || !FunctionSamples::UseMD5)
return FunctionId(Name);
- return FunctionId(Function::getGUID(Name));
+ return FunctionId(Function::getGUIDAssumingExternalLinkage(Name));
}
raw_ostream &operator<<(raw_ostream &OS, const FunctionSamples &FS);
diff --git a/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h b/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
index d714dba4a9687..12d1031fd37a0 100644
--- a/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
+++ b/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
@@ -109,11 +109,12 @@ class PseudoProbeManager {
}
const PseudoProbeDescriptor *getDesc(StringRef FProfileName) const {
- return getDesc(Function::getGUID(FProfileName));
+ return getDesc(Function::getGUIDAssumingExternalLinkage(FProfileName));
}
const PseudoProbeDescriptor *getDesc(const Function &F) const {
- return getDesc(Function::getGUID(FunctionSamples::getCanonicalFnName(F)));
+ return getDesc(Function::getGUIDAssumingExternalLinkage(
+ FunctionSamples::getCanonicalFnName(F)));
}
bool profileIsHashMismatched(const PseudoProbeDescriptor &FuncDesc,
diff --git a/llvm/lib/Analysis/CtxProfAnalysis.cpp b/llvm/lib/Analysis/CtxProfAnalysis.cpp
index e021e2a801006..f980ca7a94b40 100644
--- a/llvm/lib/Analysis/CtxProfAnalysis.cpp
+++ b/llvm/lib/Analysis/CtxProfAnalysis.cpp
@@ -57,7 +57,7 @@ PreservedAnalyses AssignGUIDPass::run(Module &M, ModuleAnalysisManager &MAM) {
GlobalValue::GUID AssignGUIDPass::getGUID(const Function &F) {
if (F.isDeclaration()) {
assert(GlobalValue::isExternalLinkage(F.getLinkage()));
- return GlobalValue::getGUID(F.getGlobalIdentifier());
+ return F.getGUID();
}
auto *MD = F.getMetadata(GUIDMetadataName);
assert(MD && "guid not found for defined function");
diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index 611d4bfbc69e8..1f2d1fcce2923 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -222,7 +222,8 @@ static void addIntrinsicToSummary(
auto *TypeId = dyn_cast<MDString>(TypeMDVal->getMetadata());
if (!TypeId)
break;
- GlobalValue::GUID Guid = GlobalValue::getGUID(TypeId->getString());
+ GlobalValue::GUID Guid =
+ GlobalValue::getGUIDAssumingExternalLinkage(TypeId->getString());
// Produce a summary from type.test intrinsics. We only summarize type.test
// intrinsics that are used other than by an llvm.assume intrinsic.
@@ -250,7 +251,8 @@ static void addIntrinsicToSummary(
auto *TypeId = dyn_cast<MDString>(TypeMDVal->getMetadata());
if (!TypeId)
break;
- GlobalValue::GUID Guid = GlobalValue::getGUID(TypeId->getString());
+ GlobalValue::GUID Guid =
+ GlobalValue::getGUIDAssumingExternalLinkage(TypeId->getString());
SmallVector<DevirtCallSite, 4> DevirtCalls;
SmallVector<Instruction *, 4> LoadedPtrs;
@@ -906,7 +908,8 @@ static void computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A,
// Set LiveRoot flag on entries matching the given value name.
static void setLiveRoot(ModuleSummaryIndex &Index, StringRef Name) {
- if (ValueInfo VI = Index.getValueInfo(GlobalValue::getGUID(Name)))
+ if (ValueInfo VI =
+ Index.getValueInfo(GlobalValue::getGUIDAssumingExternalLinkage(Name)))
for (const auto &Summary : VI.getSummaryList())
Summary->setLive(true);
}
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index c8d792981793d..faf4109ff09f0 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -9005,7 +9005,7 @@ bool LLParser::parseTypeIdEntry(unsigned ID) {
for (auto TIDRef : FwdRefTIDs->second) {
assert(!*TIDRef.first &&
"Forward referenced type id GUID expected to be 0");
- *TIDRef.first = GlobalValue::getGUID(Name);
+ *TIDRef.first = GlobalValue::getGUIDAssumingExternalLinkage(Name);
}
ForwardRefTypeIds.erase(FwdRefTIDs);
}
@@ -9110,7 +9110,7 @@ bool LLParser::parseTypeIdCompatibleVtableEntry(unsigned ID) {
for (auto TIDRef : FwdRefTIDs->second) {
assert(!*TIDRef.first &&
"Forward referenced type id GUID expected to be 0");
- *TIDRef.first = GlobalValue::getGUID(Name);
+ *TIDRef.first = GlobalValue::getGUIDAssumingExternalLinkage(Name);
}
ForwardRefTypeIds.erase(FwdRefTIDs);
}
@@ -9426,7 +9426,7 @@ bool LLParser::addGlobalValueToIndex(
assert(
(!GlobalValue::isLocalLinkage(Linkage) || !SourceFileName.empty()) &&
"Need a source_filename to compute GUID for local");
- GUID = GlobalValue::getGUID(
+ GUID = GlobalValue::getGUIDAssumingExternalLinkage(
GlobalValue::getGlobalIdentifier(Name, Linkage, SourceFileName));
VI = Index->getOrInsertValueInfo(GUID, Index->saveString(Name));
}
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 40e755902b724..07b92fc9efd1c 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -7218,10 +7218,10 @@ void ModuleSummaryIndexBitcodeReader::setValueGUID(
StringRef SourceFileName) {
std::string GlobalId =
GlobalValue::getGlobalIdentifier(ValueName, Linkage, SourceFileName);
- auto ValueGUID = GlobalValue::getGUID(GlobalId);
+ auto ValueGUID = GlobalValue::getGUIDAssumingExternalLinkage(GlobalId);
auto OriginalNameID = ValueGUID;
if (GlobalValue::isLocalLinkage(Linkage))
- OriginalNameID = GlobalValue::getGUID(ValueName);
+ OriginalNameID = GlobalValue::getGUIDAssumingExternalLinkage(ValueName);
if (PrintSummaryGUIDs)
dbgs() << "GUID " << ValueGUID << "(" << OriginalNameID << ") is "
<< ValueName << "\n";
diff --git a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
index 5dda38383a656..618deef2a74ea 100644
--- a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
@@ -34,7 +34,7 @@ void PseudoProbeHandler::emitPseudoProbe(uint64_t Guid, uint64_t Index,
// Use caching to avoid redundant md5 computation for build speed.
uint64_t &CallerGuid = NameGuidMap[Name];
if (!CallerGuid)
- CallerGuid = Function::getGUID(Name);
+ CallerGuid = Function::getGUIDAssumingExternalLinkage(Name);
uint64_t CallerProbeId = PseudoProbeDwarfDiscriminator::extractProbeIndex(
InlinedAt->getDiscriminator());
ReversedInlineStack.emplace_back(CallerGuid, CallerProbeId);
diff --git a/llvm/lib/CodeGen/PseudoProbeInserter.cpp b/llvm/lib/CodeGen/PseudoProbeInserter.cpp
index 913e0035b046f..c911e8453613d 100644
--- a/llvm/lib/CodeGen/PseudoProbeInserter.cpp
+++ b/llvm/lib/CodeGen/PseudoProbeInserter.cpp
@@ -129,7 +129,7 @@ class PseudoProbeInserter : public MachineFunctionPass {
private:
uint64_t getFuncGUID(Module *M, DILocation *DL) {
auto Name = DL->getSubprogramLinkageName();
- return Function::getGUID(Name);
+ return Function::getGUIDAssumingExternalLinkage(Name);
}
bool ShouldRun = false;
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index ae68da0182dc4..b23969e89e9cb 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -3172,7 +3172,7 @@ void AssemblyWriter::printModuleSummaryIndex() {
// Print the TypeIdCompatibleVtableMap entries.
for (auto &TId : TheIndex->typeIdCompatibleVtableMap()) {
- auto GUID = GlobalValue::getGUID(TId.first);
+ auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(TId.first);
Out << "^" << Machine.getTypeIdCompatibleVtableSlot(TId.first)
<< " = typeidCompatibleVTable: (name: \"" << TId.first << "\"";
printTypeIdCompatibleVtableSummary(TId.second);
diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp
index 8ca44719a3f94..91c4a8fc387a0 100644
--- a/llvm/lib/IR/Globals.cpp
+++ b/llvm/lib/IR/Globals.cpp
@@ -73,8 +73,8 @@ void GlobalValue::copyAttributesFrom(const GlobalValue *Src) {
removeSanitizerMetadata();
}
-GlobalValue::GUID GlobalValue::getGUID(StringRef GlobalName) {
- return MD5Hash(GlobalName);
+GlobalValue::GUID GlobalValue::getGUID(StringRef GlobalIdentifier) {
+ return MD5Hash(GlobalIdentifier);
}
void GlobalValue::removeFromParent() {
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index e895a46b8cd77..d91212691f051 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -1038,8 +1038,9 @@ Error LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
SymbolResolution Res = *ResITmp++;
if (!Sym.getIRName().empty()) {
- auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
- Sym.getIRName(), GlobalValue::ExternalLinkage, ""));
+ auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::getGlobalIdentifier(Sym.getIRName(),
+ GlobalValue::ExternalLinkage, ""));
if (Res.Prevailing)
ThinLTO.PrevailingModuleForGUID[GUID] = BM.getModuleIdentifier();
}
@@ -1059,8 +1060,9 @@ Error LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
SymbolResolution Res = *ResI++;
if (!Sym.getIRName().empty()) {
- auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
- Sym.getIRName(), GlobalValue::ExternalLinkage, ""));
+ auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::getGlobalIdentifier(Sym.getIRName(),
+ GlobalValue::ExternalLinkage, ""));
if (Res.Prevailing) {
assert(ThinLTO.PrevailingModuleForGUID[GUID] ==
BM.getModuleIdentifier());
@@ -1169,7 +1171,7 @@ Error LTO::run(AddStreamFn AddStream, FileCache Cache) {
if (Res.second.IRName.empty())
continue;
- GlobalValue::GUID GUID = GlobalValue::getGUID(
+ GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
GlobalValue::dropLLVMManglingEscape(Res.second.IRName));
if (Res.second.VisibleOutsideSummary && Res.second.Prevailing)
@@ -1944,7 +1946,7 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
if (Res.second.Partition != GlobalResolution::External ||
!Res.second.isPrevailingIRSymbol())
continue;
- auto GUID = GlobalValue::getGUID(
+ auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(
GlobalValue::dropLLVMManglingEscape(Res.second.IRName));
// Mark exported unless index-based analysis determined it to be dead.
if (ThinLTO.CombinedIndex.isGUIDLive(GUID))
diff --git a/llvm/lib/LTO/ThinLTOCodeGener...
[truncated]
|
|
@llvm/pr-subscribers-llvm-analysis Author: Owen Rodley (orodley) ChangesSee https://discourse.llvm.org/t/rfc-keep-globalvalue-guids-stable/84801 This is a non-functional change which just changes the interface of This does the following:
There are a few cases where neither of the above are possible, as the Patch is 41.29 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/129644.diff 31 Files Affected:
diff --git a/bolt/lib/Rewrite/PseudoProbeRewriter.cpp b/bolt/lib/Rewrite/PseudoProbeRewriter.cpp
index 9d6e914624a33..ee021fee3cea5 100644
--- a/bolt/lib/Rewrite/PseudoProbeRewriter.cpp
+++ b/bolt/lib/Rewrite/PseudoProbeRewriter.cpp
@@ -147,7 +147,7 @@ void PseudoProbeRewriter::parsePseudoProbe(bool ProfiledOnly) {
if (!Name)
continue;
SymName = *Name;
- uint64_t GUID = Function::getGUID(SymName);
+ uint64_t GUID = Function::getGUIDAssumingExternalLinkage(SymName);
FuncStartAddrs[GUID] = F->getAddress();
if (ProfiledOnly && HasProfile)
GuidFilter.insert(GUID);
@@ -173,7 +173,7 @@ void PseudoProbeRewriter::parsePseudoProbe(bool ProfiledOnly) {
const GUIDProbeFunctionMap &GUID2Func = ProbeDecoder.getGUID2FuncDescMap();
// Checks GUID in GUID2Func and returns it if it's present or null otherwise.
auto checkGUID = [&](StringRef SymName) -> uint64_t {
- uint64_t GUID = Function::getGUID(SymName);
+ uint64_t GUID = Function::getGUIDAssumingExternalLinkage(SymName);
if (GUID2Func.find(GUID) == GUID2Func.end())
return 0;
return GUID;
@@ -435,7 +435,7 @@ void PseudoProbeRewriter::encodePseudoProbes() {
for (const BinaryFunction *F : BC.getAllBinaryFunctions()) {
const uint64_t Addr =
F->isEmitted() ? F->getOutputAddress() : F->getAddress();
- FuncStartAddrs[Function::getGUID(
+ FuncStartAddrs[Function::getGUIDAssumingExternalLinkage(
NameResolver::restore(F->getOneName()))] = Addr;
}
DummyDecoder.buildAddress2ProbeMap(
diff --git a/llvm/examples/OrcV2Examples/LLJITWithThinLTOSummaries/LLJITWithThinLTOSummaries.cpp b/llvm/examples/OrcV2Examples/LLJITWithThinLTOSummaries/LLJITWithThinLTOSummaries.cpp
index 78152af052c07..c55aa73d50277 100644
--- a/llvm/examples/OrcV2Examples/LLJITWithThinLTOSummaries/LLJITWithThinLTOSummaries.cpp
+++ b/llvm/examples/OrcV2Examples/LLJITWithThinLTOSummaries/LLJITWithThinLTOSummaries.cpp
@@ -77,7 +77,9 @@ class DuplicateDefinitionInSummary
void log(raw_ostream &OS) const override {
OS << "Duplicate symbol for global value '" << GlobalValueName
- << "' (GUID: " << GlobalValue::getGUID(GlobalValueName) << ") in:\n";
+ << "' (GUID: "
+ << GlobalValue::getGUIDAssumingExternalLinkage(GlobalValueName)
+ << ") in:\n";
for (const std::string &Path : ModulePaths) {
OS << " " << Path << "\n";
}
@@ -110,8 +112,9 @@ class DefinitionNotFoundInSummary
}
void log(raw_ostream &OS) const override {
- OS << "No symbol for global value '" << GlobalValueName
- << "' (GUID: " << GlobalValue::getGUID(GlobalValueName) << ") in:\n";
+ OS << "No symbol for global value '" << GlobalValueName << "' (GUID: "
+ << GlobalValue::getGUIDAssumingExternalLinkage(GlobalValueName)
+ << ") in:\n";
for (const std::string &Path : ModulePaths) {
OS << " " << Path << "\n";
}
@@ -135,7 +138,8 @@ char DefinitionNotFoundInSummary::ID = 0;
Expected<StringRef> getMainModulePath(StringRef FunctionName,
ModuleSummaryIndex &Index) {
// Summaries use unmangled names.
- GlobalValue::GUID G = GlobalValue::getGUID(FunctionName);
+ GlobalValue::GUID G =
+ GlobalValue::getGUIDAssumingExternalLinkage(FunctionName);
ValueInfo VI = Index.getValueInfo(G);
// We need a unique definition, otherwise don't try further.
diff --git a/llvm/include/llvm/IR/GlobalValue.h b/llvm/include/llvm/IR/GlobalValue.h
index 2176e2c2cfbfc..04907b3d17eb0 100644
--- a/llvm/include/llvm/IR/GlobalValue.h
+++ b/llvm/include/llvm/IR/GlobalValue.h
@@ -570,6 +570,11 @@ class GlobalValue : public Constant {
return Name;
}
+ /// Declare a type to represent a global unique identifier for a global value.
+ /// This is a 64 bits hash that is used by PGO and ThinLTO to have a compact
+ /// unique way to identify a symbol.
+ using GUID = uint64_t;
+
/// Return the modified name for a global value suitable to be
/// used as the key for a global lookup (e.g. profile or ThinLTO).
/// The value's original name is \c Name and has linkage of type
@@ -578,18 +583,23 @@ class GlobalValue : public Constant {
GlobalValue::LinkageTypes Linkage,
StringRef FileName);
+private:
/// Return the modified name for this global value suitable to be
/// used as the key for a global lookup (e.g. profile or ThinLTO).
std::string getGlobalIdentifier() const;
- /// Declare a type to represent a global unique identifier for a global value.
- /// This is a 64 bits hash that is used by PGO and ThinLTO to have a compact
- /// unique way to identify a symbol.
- using GUID = uint64_t;
-
/// Return a 64-bit global unique ID constructed from global value name
/// (i.e. returned by getGlobalIdentifier()).
- static GUID getGUID(StringRef GlobalName);
+ static GUID getGUID(StringRef GlobalIdentifier);
+
+public:
+ /// Return a 64-bit global unique ID constructed from the name of a global
+ /// symbol. Since this call doesn't supply the linkage or defining filename,
+ /// the GUID computation will assume that the global has external linkage.
+ static GUID getGUIDAssumingExternalLinkage(StringRef GlobalName) {
+ return getGUID(
+ getGlobalIdentifier(GlobalName, GlobalValue::ExternalLinkage, ""));
+ }
/// Return a 64-bit global unique ID constructed from global value name
/// (i.e. returned by getGlobalIdentifier()).
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 2650456c49841..32b4fa630b7bb 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1339,14 +1339,14 @@ class CfiFunctionIndex {
template <typename... Args> void emplace(Args &&...A) {
StringRef S(std::forward<Args>(A)...);
- GlobalValue::GUID GUID =
- GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(S));
+ GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::dropLLVMManglingEscape(S));
Index[GUID].emplace(S);
}
size_t count(StringRef S) const {
- GlobalValue::GUID GUID =
- GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(S));
+ GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::dropLLVMManglingEscape(S));
auto I = Index.find(GUID);
if (I == Index.end())
return 0;
@@ -1749,8 +1749,10 @@ class ModuleSummaryIndex {
/// Add a global value summary for a value of the given name.
void addGlobalValueSummary(StringRef ValueName,
std::unique_ptr<GlobalValueSummary> Summary) {
- addGlobalValueSummary(getOrInsertValueInfo(GlobalValue::getGUID(ValueName)),
- std::move(Summary));
+ addGlobalValueSummary(
+ getOrInsertValueInfo(
+ GlobalValue::getGUIDAssumingExternalLinkage(ValueName)),
+ std::move(Summary));
}
/// Add a global value summary for the given ValueInfo.
@@ -1887,19 +1889,22 @@ class ModuleSummaryIndex {
/// This accessor can mutate the map and therefore should not be used in
/// the ThinLTO backends.
TypeIdSummary &getOrInsertTypeIdSummary(StringRef TypeId) {
- auto TidIter = TypeIdMap.equal_range(GlobalValue::getGUID(TypeId));
+ auto TidIter = TypeIdMap.equal_range(
+ GlobalValue::getGUIDAssumingExternalLinkage(TypeId));
for (auto &[GUID, TypeIdPair] : make_range(TidIter))
if (TypeIdPair.first == TypeId)
return TypeIdPair.second;
- auto It = TypeIdMap.insert({GlobalValue::getGUID(TypeId),
- {TypeIdSaver.save(TypeId), TypeIdSummary()}});
+ auto It =
+ TypeIdMap.insert({GlobalValue::getGUIDAssumingExternalLinkage(TypeId),
+ {TypeIdSaver.save(TypeId), TypeIdSummary()}});
return It->second.second;
}
/// This returns either a pointer to the type id summary (if present in the
/// summary map) or null (if not present). This may be used when importing.
const TypeIdSummary *getTypeIdSummary(StringRef TypeId) const {
- auto TidIter = TypeIdMap.equal_range(GlobalValue::getGUID(TypeId));
+ auto TidIter = TypeIdMap.equal_range(
+ GlobalValue::getGUIDAssumingExternalLinkage(TypeId));
for (const auto &[GUID, TypeIdPair] : make_range(TidIter))
if (TypeIdPair.first == TypeId)
return &TypeIdPair.second;
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
index d5a91763a981c..531de514822e8 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
@@ -314,7 +314,7 @@ template <> struct CustomMappingTraits<TypeIdSummaryMapTy> {
static void inputOne(IO &io, StringRef Key, TypeIdSummaryMapTy &V) {
TypeIdSummary TId;
io.mapRequired(Key.str().c_str(), TId);
- V.insert({GlobalValue::getGUID(Key), {Key, TId}});
+ V.insert({GlobalValue::getGUIDAssumingExternalLinkage(Key), {Key, TId}});
}
static void output(IO &io, TypeIdSummaryMapTy &V) {
for (auto &TidIter : V)
diff --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h
index e7b154dff0697..008d464d0c582 100644
--- a/llvm/include/llvm/ProfileData/SampleProf.h
+++ b/llvm/include/llvm/ProfileData/SampleProf.h
@@ -1299,7 +1299,7 @@ class FunctionSamples {
static inline FunctionId getRepInFormat(StringRef Name) {
if (Name.empty() || !FunctionSamples::UseMD5)
return FunctionId(Name);
- return FunctionId(Function::getGUID(Name));
+ return FunctionId(Function::getGUIDAssumingExternalLinkage(Name));
}
raw_ostream &operator<<(raw_ostream &OS, const FunctionSamples &FS);
diff --git a/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h b/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
index d714dba4a9687..12d1031fd37a0 100644
--- a/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
+++ b/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
@@ -109,11 +109,12 @@ class PseudoProbeManager {
}
const PseudoProbeDescriptor *getDesc(StringRef FProfileName) const {
- return getDesc(Function::getGUID(FProfileName));
+ return getDesc(Function::getGUIDAssumingExternalLinkage(FProfileName));
}
const PseudoProbeDescriptor *getDesc(const Function &F) const {
- return getDesc(Function::getGUID(FunctionSamples::getCanonicalFnName(F)));
+ return getDesc(Function::getGUIDAssumingExternalLinkage(
+ FunctionSamples::getCanonicalFnName(F)));
}
bool profileIsHashMismatched(const PseudoProbeDescriptor &FuncDesc,
diff --git a/llvm/lib/Analysis/CtxProfAnalysis.cpp b/llvm/lib/Analysis/CtxProfAnalysis.cpp
index e021e2a801006..f980ca7a94b40 100644
--- a/llvm/lib/Analysis/CtxProfAnalysis.cpp
+++ b/llvm/lib/Analysis/CtxProfAnalysis.cpp
@@ -57,7 +57,7 @@ PreservedAnalyses AssignGUIDPass::run(Module &M, ModuleAnalysisManager &MAM) {
GlobalValue::GUID AssignGUIDPass::getGUID(const Function &F) {
if (F.isDeclaration()) {
assert(GlobalValue::isExternalLinkage(F.getLinkage()));
- return GlobalValue::getGUID(F.getGlobalIdentifier());
+ return F.getGUID();
}
auto *MD = F.getMetadata(GUIDMetadataName);
assert(MD && "guid not found for defined function");
diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index 611d4bfbc69e8..1f2d1fcce2923 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -222,7 +222,8 @@ static void addIntrinsicToSummary(
auto *TypeId = dyn_cast<MDString>(TypeMDVal->getMetadata());
if (!TypeId)
break;
- GlobalValue::GUID Guid = GlobalValue::getGUID(TypeId->getString());
+ GlobalValue::GUID Guid =
+ GlobalValue::getGUIDAssumingExternalLinkage(TypeId->getString());
// Produce a summary from type.test intrinsics. We only summarize type.test
// intrinsics that are used other than by an llvm.assume intrinsic.
@@ -250,7 +251,8 @@ static void addIntrinsicToSummary(
auto *TypeId = dyn_cast<MDString>(TypeMDVal->getMetadata());
if (!TypeId)
break;
- GlobalValue::GUID Guid = GlobalValue::getGUID(TypeId->getString());
+ GlobalValue::GUID Guid =
+ GlobalValue::getGUIDAssumingExternalLinkage(TypeId->getString());
SmallVector<DevirtCallSite, 4> DevirtCalls;
SmallVector<Instruction *, 4> LoadedPtrs;
@@ -906,7 +908,8 @@ static void computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A,
// Set LiveRoot flag on entries matching the given value name.
static void setLiveRoot(ModuleSummaryIndex &Index, StringRef Name) {
- if (ValueInfo VI = Index.getValueInfo(GlobalValue::getGUID(Name)))
+ if (ValueInfo VI =
+ Index.getValueInfo(GlobalValue::getGUIDAssumingExternalLinkage(Name)))
for (const auto &Summary : VI.getSummaryList())
Summary->setLive(true);
}
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index c8d792981793d..faf4109ff09f0 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -9005,7 +9005,7 @@ bool LLParser::parseTypeIdEntry(unsigned ID) {
for (auto TIDRef : FwdRefTIDs->second) {
assert(!*TIDRef.first &&
"Forward referenced type id GUID expected to be 0");
- *TIDRef.first = GlobalValue::getGUID(Name);
+ *TIDRef.first = GlobalValue::getGUIDAssumingExternalLinkage(Name);
}
ForwardRefTypeIds.erase(FwdRefTIDs);
}
@@ -9110,7 +9110,7 @@ bool LLParser::parseTypeIdCompatibleVtableEntry(unsigned ID) {
for (auto TIDRef : FwdRefTIDs->second) {
assert(!*TIDRef.first &&
"Forward referenced type id GUID expected to be 0");
- *TIDRef.first = GlobalValue::getGUID(Name);
+ *TIDRef.first = GlobalValue::getGUIDAssumingExternalLinkage(Name);
}
ForwardRefTypeIds.erase(FwdRefTIDs);
}
@@ -9426,7 +9426,7 @@ bool LLParser::addGlobalValueToIndex(
assert(
(!GlobalValue::isLocalLinkage(Linkage) || !SourceFileName.empty()) &&
"Need a source_filename to compute GUID for local");
- GUID = GlobalValue::getGUID(
+ GUID = GlobalValue::getGUIDAssumingExternalLinkage(
GlobalValue::getGlobalIdentifier(Name, Linkage, SourceFileName));
VI = Index->getOrInsertValueInfo(GUID, Index->saveString(Name));
}
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 40e755902b724..07b92fc9efd1c 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -7218,10 +7218,10 @@ void ModuleSummaryIndexBitcodeReader::setValueGUID(
StringRef SourceFileName) {
std::string GlobalId =
GlobalValue::getGlobalIdentifier(ValueName, Linkage, SourceFileName);
- auto ValueGUID = GlobalValue::getGUID(GlobalId);
+ auto ValueGUID = GlobalValue::getGUIDAssumingExternalLinkage(GlobalId);
auto OriginalNameID = ValueGUID;
if (GlobalValue::isLocalLinkage(Linkage))
- OriginalNameID = GlobalValue::getGUID(ValueName);
+ OriginalNameID = GlobalValue::getGUIDAssumingExternalLinkage(ValueName);
if (PrintSummaryGUIDs)
dbgs() << "GUID " << ValueGUID << "(" << OriginalNameID << ") is "
<< ValueName << "\n";
diff --git a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
index 5dda38383a656..618deef2a74ea 100644
--- a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
@@ -34,7 +34,7 @@ void PseudoProbeHandler::emitPseudoProbe(uint64_t Guid, uint64_t Index,
// Use caching to avoid redundant md5 computation for build speed.
uint64_t &CallerGuid = NameGuidMap[Name];
if (!CallerGuid)
- CallerGuid = Function::getGUID(Name);
+ CallerGuid = Function::getGUIDAssumingExternalLinkage(Name);
uint64_t CallerProbeId = PseudoProbeDwarfDiscriminator::extractProbeIndex(
InlinedAt->getDiscriminator());
ReversedInlineStack.emplace_back(CallerGuid, CallerProbeId);
diff --git a/llvm/lib/CodeGen/PseudoProbeInserter.cpp b/llvm/lib/CodeGen/PseudoProbeInserter.cpp
index 913e0035b046f..c911e8453613d 100644
--- a/llvm/lib/CodeGen/PseudoProbeInserter.cpp
+++ b/llvm/lib/CodeGen/PseudoProbeInserter.cpp
@@ -129,7 +129,7 @@ class PseudoProbeInserter : public MachineFunctionPass {
private:
uint64_t getFuncGUID(Module *M, DILocation *DL) {
auto Name = DL->getSubprogramLinkageName();
- return Function::getGUID(Name);
+ return Function::getGUIDAssumingExternalLinkage(Name);
}
bool ShouldRun = false;
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index ae68da0182dc4..b23969e89e9cb 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -3172,7 +3172,7 @@ void AssemblyWriter::printModuleSummaryIndex() {
// Print the TypeIdCompatibleVtableMap entries.
for (auto &TId : TheIndex->typeIdCompatibleVtableMap()) {
- auto GUID = GlobalValue::getGUID(TId.first);
+ auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(TId.first);
Out << "^" << Machine.getTypeIdCompatibleVtableSlot(TId.first)
<< " = typeidCompatibleVTable: (name: \"" << TId.first << "\"";
printTypeIdCompatibleVtableSummary(TId.second);
diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp
index 8ca44719a3f94..91c4a8fc387a0 100644
--- a/llvm/lib/IR/Globals.cpp
+++ b/llvm/lib/IR/Globals.cpp
@@ -73,8 +73,8 @@ void GlobalValue::copyAttributesFrom(const GlobalValue *Src) {
removeSanitizerMetadata();
}
-GlobalValue::GUID GlobalValue::getGUID(StringRef GlobalName) {
- return MD5Hash(GlobalName);
+GlobalValue::GUID GlobalValue::getGUID(StringRef GlobalIdentifier) {
+ return MD5Hash(GlobalIdentifier);
}
void GlobalValue::removeFromParent() {
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index e895a46b8cd77..d91212691f051 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -1038,8 +1038,9 @@ Error LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
SymbolResolution Res = *ResITmp++;
if (!Sym.getIRName().empty()) {
- auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
- Sym.getIRName(), GlobalValue::ExternalLinkage, ""));
+ auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::getGlobalIdentifier(Sym.getIRName(),
+ GlobalValue::ExternalLinkage, ""));
if (Res.Prevailing)
ThinLTO.PrevailingModuleForGUID[GUID] = BM.getModuleIdentifier();
}
@@ -1059,8 +1060,9 @@ Error LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
SymbolResolution Res = *ResI++;
if (!Sym.getIRName().empty()) {
- auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
- Sym.getIRName(), GlobalValue::ExternalLinkage, ""));
+ auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::getGlobalIdentifier(Sym.getIRName(),
+ GlobalValue::ExternalLinkage, ""));
if (Res.Prevailing) {
assert(ThinLTO.PrevailingModuleForGUID[GUID] ==
BM.getModuleIdentifier());
@@ -1169,7 +1171,7 @@ Error LTO::run(AddStreamFn AddStream, FileCache Cache) {
if (Res.second.IRName.empty())
continue;
- GlobalValue::GUID GUID = GlobalValue::getGUID(
+ GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
GlobalValue::dropLLVMManglingEscape(Res.second.IRName));
if (Res.second.VisibleOutsideSummary && Res.second.Prevailing)
@@ -1944,7 +1946,7 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
if (Res.second.Partition != GlobalResolution::External ||
!Res.second.isPrevailingIRSymbol())
continue;
- auto GUID = GlobalValue::getGUID(
+ auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(
GlobalValue::dropLLVMManglingEscape(Res.second.IRName));
// Mark exported unless index-based analysis determined it to be dead.
if (ThinLTO.CombinedIndex.isGUIDLive(GUID))
diff --git a/llvm/lib/LTO/ThinLTOCodeGener...
[truncated]
|
|
@llvm/pr-subscribers-bolt Author: Owen Rodley (orodley) ChangesSee https://discourse.llvm.org/t/rfc-keep-globalvalue-guids-stable/84801 This is a non-functional change which just changes the interface of This does the following:
There are a few cases where neither of the above are possible, as the Patch is 41.29 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/129644.diff 31 Files Affected:
diff --git a/bolt/lib/Rewrite/PseudoProbeRewriter.cpp b/bolt/lib/Rewrite/PseudoProbeRewriter.cpp
index 9d6e914624a33..ee021fee3cea5 100644
--- a/bolt/lib/Rewrite/PseudoProbeRewriter.cpp
+++ b/bolt/lib/Rewrite/PseudoProbeRewriter.cpp
@@ -147,7 +147,7 @@ void PseudoProbeRewriter::parsePseudoProbe(bool ProfiledOnly) {
if (!Name)
continue;
SymName = *Name;
- uint64_t GUID = Function::getGUID(SymName);
+ uint64_t GUID = Function::getGUIDAssumingExternalLinkage(SymName);
FuncStartAddrs[GUID] = F->getAddress();
if (ProfiledOnly && HasProfile)
GuidFilter.insert(GUID);
@@ -173,7 +173,7 @@ void PseudoProbeRewriter::parsePseudoProbe(bool ProfiledOnly) {
const GUIDProbeFunctionMap &GUID2Func = ProbeDecoder.getGUID2FuncDescMap();
// Checks GUID in GUID2Func and returns it if it's present or null otherwise.
auto checkGUID = [&](StringRef SymName) -> uint64_t {
- uint64_t GUID = Function::getGUID(SymName);
+ uint64_t GUID = Function::getGUIDAssumingExternalLinkage(SymName);
if (GUID2Func.find(GUID) == GUID2Func.end())
return 0;
return GUID;
@@ -435,7 +435,7 @@ void PseudoProbeRewriter::encodePseudoProbes() {
for (const BinaryFunction *F : BC.getAllBinaryFunctions()) {
const uint64_t Addr =
F->isEmitted() ? F->getOutputAddress() : F->getAddress();
- FuncStartAddrs[Function::getGUID(
+ FuncStartAddrs[Function::getGUIDAssumingExternalLinkage(
NameResolver::restore(F->getOneName()))] = Addr;
}
DummyDecoder.buildAddress2ProbeMap(
diff --git a/llvm/examples/OrcV2Examples/LLJITWithThinLTOSummaries/LLJITWithThinLTOSummaries.cpp b/llvm/examples/OrcV2Examples/LLJITWithThinLTOSummaries/LLJITWithThinLTOSummaries.cpp
index 78152af052c07..c55aa73d50277 100644
--- a/llvm/examples/OrcV2Examples/LLJITWithThinLTOSummaries/LLJITWithThinLTOSummaries.cpp
+++ b/llvm/examples/OrcV2Examples/LLJITWithThinLTOSummaries/LLJITWithThinLTOSummaries.cpp
@@ -77,7 +77,9 @@ class DuplicateDefinitionInSummary
void log(raw_ostream &OS) const override {
OS << "Duplicate symbol for global value '" << GlobalValueName
- << "' (GUID: " << GlobalValue::getGUID(GlobalValueName) << ") in:\n";
+ << "' (GUID: "
+ << GlobalValue::getGUIDAssumingExternalLinkage(GlobalValueName)
+ << ") in:\n";
for (const std::string &Path : ModulePaths) {
OS << " " << Path << "\n";
}
@@ -110,8 +112,9 @@ class DefinitionNotFoundInSummary
}
void log(raw_ostream &OS) const override {
- OS << "No symbol for global value '" << GlobalValueName
- << "' (GUID: " << GlobalValue::getGUID(GlobalValueName) << ") in:\n";
+ OS << "No symbol for global value '" << GlobalValueName << "' (GUID: "
+ << GlobalValue::getGUIDAssumingExternalLinkage(GlobalValueName)
+ << ") in:\n";
for (const std::string &Path : ModulePaths) {
OS << " " << Path << "\n";
}
@@ -135,7 +138,8 @@ char DefinitionNotFoundInSummary::ID = 0;
Expected<StringRef> getMainModulePath(StringRef FunctionName,
ModuleSummaryIndex &Index) {
// Summaries use unmangled names.
- GlobalValue::GUID G = GlobalValue::getGUID(FunctionName);
+ GlobalValue::GUID G =
+ GlobalValue::getGUIDAssumingExternalLinkage(FunctionName);
ValueInfo VI = Index.getValueInfo(G);
// We need a unique definition, otherwise don't try further.
diff --git a/llvm/include/llvm/IR/GlobalValue.h b/llvm/include/llvm/IR/GlobalValue.h
index 2176e2c2cfbfc..04907b3d17eb0 100644
--- a/llvm/include/llvm/IR/GlobalValue.h
+++ b/llvm/include/llvm/IR/GlobalValue.h
@@ -570,6 +570,11 @@ class GlobalValue : public Constant {
return Name;
}
+ /// Declare a type to represent a global unique identifier for a global value.
+ /// This is a 64 bits hash that is used by PGO and ThinLTO to have a compact
+ /// unique way to identify a symbol.
+ using GUID = uint64_t;
+
/// Return the modified name for a global value suitable to be
/// used as the key for a global lookup (e.g. profile or ThinLTO).
/// The value's original name is \c Name and has linkage of type
@@ -578,18 +583,23 @@ class GlobalValue : public Constant {
GlobalValue::LinkageTypes Linkage,
StringRef FileName);
+private:
/// Return the modified name for this global value suitable to be
/// used as the key for a global lookup (e.g. profile or ThinLTO).
std::string getGlobalIdentifier() const;
- /// Declare a type to represent a global unique identifier for a global value.
- /// This is a 64 bits hash that is used by PGO and ThinLTO to have a compact
- /// unique way to identify a symbol.
- using GUID = uint64_t;
-
/// Return a 64-bit global unique ID constructed from global value name
/// (i.e. returned by getGlobalIdentifier()).
- static GUID getGUID(StringRef GlobalName);
+ static GUID getGUID(StringRef GlobalIdentifier);
+
+public:
+ /// Return a 64-bit global unique ID constructed from the name of a global
+ /// symbol. Since this call doesn't supply the linkage or defining filename,
+ /// the GUID computation will assume that the global has external linkage.
+ static GUID getGUIDAssumingExternalLinkage(StringRef GlobalName) {
+ return getGUID(
+ getGlobalIdentifier(GlobalName, GlobalValue::ExternalLinkage, ""));
+ }
/// Return a 64-bit global unique ID constructed from global value name
/// (i.e. returned by getGlobalIdentifier()).
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 2650456c49841..32b4fa630b7bb 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1339,14 +1339,14 @@ class CfiFunctionIndex {
template <typename... Args> void emplace(Args &&...A) {
StringRef S(std::forward<Args>(A)...);
- GlobalValue::GUID GUID =
- GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(S));
+ GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::dropLLVMManglingEscape(S));
Index[GUID].emplace(S);
}
size_t count(StringRef S) const {
- GlobalValue::GUID GUID =
- GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(S));
+ GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::dropLLVMManglingEscape(S));
auto I = Index.find(GUID);
if (I == Index.end())
return 0;
@@ -1749,8 +1749,10 @@ class ModuleSummaryIndex {
/// Add a global value summary for a value of the given name.
void addGlobalValueSummary(StringRef ValueName,
std::unique_ptr<GlobalValueSummary> Summary) {
- addGlobalValueSummary(getOrInsertValueInfo(GlobalValue::getGUID(ValueName)),
- std::move(Summary));
+ addGlobalValueSummary(
+ getOrInsertValueInfo(
+ GlobalValue::getGUIDAssumingExternalLinkage(ValueName)),
+ std::move(Summary));
}
/// Add a global value summary for the given ValueInfo.
@@ -1887,19 +1889,22 @@ class ModuleSummaryIndex {
/// This accessor can mutate the map and therefore should not be used in
/// the ThinLTO backends.
TypeIdSummary &getOrInsertTypeIdSummary(StringRef TypeId) {
- auto TidIter = TypeIdMap.equal_range(GlobalValue::getGUID(TypeId));
+ auto TidIter = TypeIdMap.equal_range(
+ GlobalValue::getGUIDAssumingExternalLinkage(TypeId));
for (auto &[GUID, TypeIdPair] : make_range(TidIter))
if (TypeIdPair.first == TypeId)
return TypeIdPair.second;
- auto It = TypeIdMap.insert({GlobalValue::getGUID(TypeId),
- {TypeIdSaver.save(TypeId), TypeIdSummary()}});
+ auto It =
+ TypeIdMap.insert({GlobalValue::getGUIDAssumingExternalLinkage(TypeId),
+ {TypeIdSaver.save(TypeId), TypeIdSummary()}});
return It->second.second;
}
/// This returns either a pointer to the type id summary (if present in the
/// summary map) or null (if not present). This may be used when importing.
const TypeIdSummary *getTypeIdSummary(StringRef TypeId) const {
- auto TidIter = TypeIdMap.equal_range(GlobalValue::getGUID(TypeId));
+ auto TidIter = TypeIdMap.equal_range(
+ GlobalValue::getGUIDAssumingExternalLinkage(TypeId));
for (const auto &[GUID, TypeIdPair] : make_range(TidIter))
if (TypeIdPair.first == TypeId)
return &TypeIdPair.second;
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
index d5a91763a981c..531de514822e8 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
@@ -314,7 +314,7 @@ template <> struct CustomMappingTraits<TypeIdSummaryMapTy> {
static void inputOne(IO &io, StringRef Key, TypeIdSummaryMapTy &V) {
TypeIdSummary TId;
io.mapRequired(Key.str().c_str(), TId);
- V.insert({GlobalValue::getGUID(Key), {Key, TId}});
+ V.insert({GlobalValue::getGUIDAssumingExternalLinkage(Key), {Key, TId}});
}
static void output(IO &io, TypeIdSummaryMapTy &V) {
for (auto &TidIter : V)
diff --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h
index e7b154dff0697..008d464d0c582 100644
--- a/llvm/include/llvm/ProfileData/SampleProf.h
+++ b/llvm/include/llvm/ProfileData/SampleProf.h
@@ -1299,7 +1299,7 @@ class FunctionSamples {
static inline FunctionId getRepInFormat(StringRef Name) {
if (Name.empty() || !FunctionSamples::UseMD5)
return FunctionId(Name);
- return FunctionId(Function::getGUID(Name));
+ return FunctionId(Function::getGUIDAssumingExternalLinkage(Name));
}
raw_ostream &operator<<(raw_ostream &OS, const FunctionSamples &FS);
diff --git a/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h b/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
index d714dba4a9687..12d1031fd37a0 100644
--- a/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
+++ b/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
@@ -109,11 +109,12 @@ class PseudoProbeManager {
}
const PseudoProbeDescriptor *getDesc(StringRef FProfileName) const {
- return getDesc(Function::getGUID(FProfileName));
+ return getDesc(Function::getGUIDAssumingExternalLinkage(FProfileName));
}
const PseudoProbeDescriptor *getDesc(const Function &F) const {
- return getDesc(Function::getGUID(FunctionSamples::getCanonicalFnName(F)));
+ return getDesc(Function::getGUIDAssumingExternalLinkage(
+ FunctionSamples::getCanonicalFnName(F)));
}
bool profileIsHashMismatched(const PseudoProbeDescriptor &FuncDesc,
diff --git a/llvm/lib/Analysis/CtxProfAnalysis.cpp b/llvm/lib/Analysis/CtxProfAnalysis.cpp
index e021e2a801006..f980ca7a94b40 100644
--- a/llvm/lib/Analysis/CtxProfAnalysis.cpp
+++ b/llvm/lib/Analysis/CtxProfAnalysis.cpp
@@ -57,7 +57,7 @@ PreservedAnalyses AssignGUIDPass::run(Module &M, ModuleAnalysisManager &MAM) {
GlobalValue::GUID AssignGUIDPass::getGUID(const Function &F) {
if (F.isDeclaration()) {
assert(GlobalValue::isExternalLinkage(F.getLinkage()));
- return GlobalValue::getGUID(F.getGlobalIdentifier());
+ return F.getGUID();
}
auto *MD = F.getMetadata(GUIDMetadataName);
assert(MD && "guid not found for defined function");
diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index 611d4bfbc69e8..1f2d1fcce2923 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -222,7 +222,8 @@ static void addIntrinsicToSummary(
auto *TypeId = dyn_cast<MDString>(TypeMDVal->getMetadata());
if (!TypeId)
break;
- GlobalValue::GUID Guid = GlobalValue::getGUID(TypeId->getString());
+ GlobalValue::GUID Guid =
+ GlobalValue::getGUIDAssumingExternalLinkage(TypeId->getString());
// Produce a summary from type.test intrinsics. We only summarize type.test
// intrinsics that are used other than by an llvm.assume intrinsic.
@@ -250,7 +251,8 @@ static void addIntrinsicToSummary(
auto *TypeId = dyn_cast<MDString>(TypeMDVal->getMetadata());
if (!TypeId)
break;
- GlobalValue::GUID Guid = GlobalValue::getGUID(TypeId->getString());
+ GlobalValue::GUID Guid =
+ GlobalValue::getGUIDAssumingExternalLinkage(TypeId->getString());
SmallVector<DevirtCallSite, 4> DevirtCalls;
SmallVector<Instruction *, 4> LoadedPtrs;
@@ -906,7 +908,8 @@ static void computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A,
// Set LiveRoot flag on entries matching the given value name.
static void setLiveRoot(ModuleSummaryIndex &Index, StringRef Name) {
- if (ValueInfo VI = Index.getValueInfo(GlobalValue::getGUID(Name)))
+ if (ValueInfo VI =
+ Index.getValueInfo(GlobalValue::getGUIDAssumingExternalLinkage(Name)))
for (const auto &Summary : VI.getSummaryList())
Summary->setLive(true);
}
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index c8d792981793d..faf4109ff09f0 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -9005,7 +9005,7 @@ bool LLParser::parseTypeIdEntry(unsigned ID) {
for (auto TIDRef : FwdRefTIDs->second) {
assert(!*TIDRef.first &&
"Forward referenced type id GUID expected to be 0");
- *TIDRef.first = GlobalValue::getGUID(Name);
+ *TIDRef.first = GlobalValue::getGUIDAssumingExternalLinkage(Name);
}
ForwardRefTypeIds.erase(FwdRefTIDs);
}
@@ -9110,7 +9110,7 @@ bool LLParser::parseTypeIdCompatibleVtableEntry(unsigned ID) {
for (auto TIDRef : FwdRefTIDs->second) {
assert(!*TIDRef.first &&
"Forward referenced type id GUID expected to be 0");
- *TIDRef.first = GlobalValue::getGUID(Name);
+ *TIDRef.first = GlobalValue::getGUIDAssumingExternalLinkage(Name);
}
ForwardRefTypeIds.erase(FwdRefTIDs);
}
@@ -9426,7 +9426,7 @@ bool LLParser::addGlobalValueToIndex(
assert(
(!GlobalValue::isLocalLinkage(Linkage) || !SourceFileName.empty()) &&
"Need a source_filename to compute GUID for local");
- GUID = GlobalValue::getGUID(
+ GUID = GlobalValue::getGUIDAssumingExternalLinkage(
GlobalValue::getGlobalIdentifier(Name, Linkage, SourceFileName));
VI = Index->getOrInsertValueInfo(GUID, Index->saveString(Name));
}
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 40e755902b724..07b92fc9efd1c 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -7218,10 +7218,10 @@ void ModuleSummaryIndexBitcodeReader::setValueGUID(
StringRef SourceFileName) {
std::string GlobalId =
GlobalValue::getGlobalIdentifier(ValueName, Linkage, SourceFileName);
- auto ValueGUID = GlobalValue::getGUID(GlobalId);
+ auto ValueGUID = GlobalValue::getGUIDAssumingExternalLinkage(GlobalId);
auto OriginalNameID = ValueGUID;
if (GlobalValue::isLocalLinkage(Linkage))
- OriginalNameID = GlobalValue::getGUID(ValueName);
+ OriginalNameID = GlobalValue::getGUIDAssumingExternalLinkage(ValueName);
if (PrintSummaryGUIDs)
dbgs() << "GUID " << ValueGUID << "(" << OriginalNameID << ") is "
<< ValueName << "\n";
diff --git a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
index 5dda38383a656..618deef2a74ea 100644
--- a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
@@ -34,7 +34,7 @@ void PseudoProbeHandler::emitPseudoProbe(uint64_t Guid, uint64_t Index,
// Use caching to avoid redundant md5 computation for build speed.
uint64_t &CallerGuid = NameGuidMap[Name];
if (!CallerGuid)
- CallerGuid = Function::getGUID(Name);
+ CallerGuid = Function::getGUIDAssumingExternalLinkage(Name);
uint64_t CallerProbeId = PseudoProbeDwarfDiscriminator::extractProbeIndex(
InlinedAt->getDiscriminator());
ReversedInlineStack.emplace_back(CallerGuid, CallerProbeId);
diff --git a/llvm/lib/CodeGen/PseudoProbeInserter.cpp b/llvm/lib/CodeGen/PseudoProbeInserter.cpp
index 913e0035b046f..c911e8453613d 100644
--- a/llvm/lib/CodeGen/PseudoProbeInserter.cpp
+++ b/llvm/lib/CodeGen/PseudoProbeInserter.cpp
@@ -129,7 +129,7 @@ class PseudoProbeInserter : public MachineFunctionPass {
private:
uint64_t getFuncGUID(Module *M, DILocation *DL) {
auto Name = DL->getSubprogramLinkageName();
- return Function::getGUID(Name);
+ return Function::getGUIDAssumingExternalLinkage(Name);
}
bool ShouldRun = false;
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index ae68da0182dc4..b23969e89e9cb 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -3172,7 +3172,7 @@ void AssemblyWriter::printModuleSummaryIndex() {
// Print the TypeIdCompatibleVtableMap entries.
for (auto &TId : TheIndex->typeIdCompatibleVtableMap()) {
- auto GUID = GlobalValue::getGUID(TId.first);
+ auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(TId.first);
Out << "^" << Machine.getTypeIdCompatibleVtableSlot(TId.first)
<< " = typeidCompatibleVTable: (name: \"" << TId.first << "\"";
printTypeIdCompatibleVtableSummary(TId.second);
diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp
index 8ca44719a3f94..91c4a8fc387a0 100644
--- a/llvm/lib/IR/Globals.cpp
+++ b/llvm/lib/IR/Globals.cpp
@@ -73,8 +73,8 @@ void GlobalValue::copyAttributesFrom(const GlobalValue *Src) {
removeSanitizerMetadata();
}
-GlobalValue::GUID GlobalValue::getGUID(StringRef GlobalName) {
- return MD5Hash(GlobalName);
+GlobalValue::GUID GlobalValue::getGUID(StringRef GlobalIdentifier) {
+ return MD5Hash(GlobalIdentifier);
}
void GlobalValue::removeFromParent() {
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index e895a46b8cd77..d91212691f051 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -1038,8 +1038,9 @@ Error LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
SymbolResolution Res = *ResITmp++;
if (!Sym.getIRName().empty()) {
- auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
- Sym.getIRName(), GlobalValue::ExternalLinkage, ""));
+ auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::getGlobalIdentifier(Sym.getIRName(),
+ GlobalValue::ExternalLinkage, ""));
if (Res.Prevailing)
ThinLTO.PrevailingModuleForGUID[GUID] = BM.getModuleIdentifier();
}
@@ -1059,8 +1060,9 @@ Error LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
SymbolResolution Res = *ResI++;
if (!Sym.getIRName().empty()) {
- auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
- Sym.getIRName(), GlobalValue::ExternalLinkage, ""));
+ auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::getGlobalIdentifier(Sym.getIRName(),
+ GlobalValue::ExternalLinkage, ""));
if (Res.Prevailing) {
assert(ThinLTO.PrevailingModuleForGUID[GUID] ==
BM.getModuleIdentifier());
@@ -1169,7 +1171,7 @@ Error LTO::run(AddStreamFn AddStream, FileCache Cache) {
if (Res.second.IRName.empty())
continue;
- GlobalValue::GUID GUID = GlobalValue::getGUID(
+ GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
GlobalValue::dropLLVMManglingEscape(Res.second.IRName));
if (Res.second.VisibleOutsideSummary && Res.second.Prevailing)
@@ -1944,7 +1946,7 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
if (Res.second.Partition != GlobalResolution::External ||
!Res.second.isPrevailingIRSymbol())
continue;
- auto GUID = GlobalValue::getGUID(
+ auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(
GlobalValue::dropLLVMManglingEscape(Res.second.IRName));
// Mark exported unless index-based analysis determined it to be dead.
if (ThinLTO.CombinedIndex.isGUIDLive(GUID))
diff --git a/llvm/lib/LTO/ThinLTOCodeGener...
[truncated]
|
|
@llvm/pr-subscribers-backend-spir-v Author: Owen Rodley (orodley) ChangesSee https://discourse.llvm.org/t/rfc-keep-globalvalue-guids-stable/84801 This is a non-functional change which just changes the interface of This does the following:
There are a few cases where neither of the above are possible, as the Patch is 41.29 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/129644.diff 31 Files Affected:
diff --git a/bolt/lib/Rewrite/PseudoProbeRewriter.cpp b/bolt/lib/Rewrite/PseudoProbeRewriter.cpp
index 9d6e914624a33..ee021fee3cea5 100644
--- a/bolt/lib/Rewrite/PseudoProbeRewriter.cpp
+++ b/bolt/lib/Rewrite/PseudoProbeRewriter.cpp
@@ -147,7 +147,7 @@ void PseudoProbeRewriter::parsePseudoProbe(bool ProfiledOnly) {
if (!Name)
continue;
SymName = *Name;
- uint64_t GUID = Function::getGUID(SymName);
+ uint64_t GUID = Function::getGUIDAssumingExternalLinkage(SymName);
FuncStartAddrs[GUID] = F->getAddress();
if (ProfiledOnly && HasProfile)
GuidFilter.insert(GUID);
@@ -173,7 +173,7 @@ void PseudoProbeRewriter::parsePseudoProbe(bool ProfiledOnly) {
const GUIDProbeFunctionMap &GUID2Func = ProbeDecoder.getGUID2FuncDescMap();
// Checks GUID in GUID2Func and returns it if it's present or null otherwise.
auto checkGUID = [&](StringRef SymName) -> uint64_t {
- uint64_t GUID = Function::getGUID(SymName);
+ uint64_t GUID = Function::getGUIDAssumingExternalLinkage(SymName);
if (GUID2Func.find(GUID) == GUID2Func.end())
return 0;
return GUID;
@@ -435,7 +435,7 @@ void PseudoProbeRewriter::encodePseudoProbes() {
for (const BinaryFunction *F : BC.getAllBinaryFunctions()) {
const uint64_t Addr =
F->isEmitted() ? F->getOutputAddress() : F->getAddress();
- FuncStartAddrs[Function::getGUID(
+ FuncStartAddrs[Function::getGUIDAssumingExternalLinkage(
NameResolver::restore(F->getOneName()))] = Addr;
}
DummyDecoder.buildAddress2ProbeMap(
diff --git a/llvm/examples/OrcV2Examples/LLJITWithThinLTOSummaries/LLJITWithThinLTOSummaries.cpp b/llvm/examples/OrcV2Examples/LLJITWithThinLTOSummaries/LLJITWithThinLTOSummaries.cpp
index 78152af052c07..c55aa73d50277 100644
--- a/llvm/examples/OrcV2Examples/LLJITWithThinLTOSummaries/LLJITWithThinLTOSummaries.cpp
+++ b/llvm/examples/OrcV2Examples/LLJITWithThinLTOSummaries/LLJITWithThinLTOSummaries.cpp
@@ -77,7 +77,9 @@ class DuplicateDefinitionInSummary
void log(raw_ostream &OS) const override {
OS << "Duplicate symbol for global value '" << GlobalValueName
- << "' (GUID: " << GlobalValue::getGUID(GlobalValueName) << ") in:\n";
+ << "' (GUID: "
+ << GlobalValue::getGUIDAssumingExternalLinkage(GlobalValueName)
+ << ") in:\n";
for (const std::string &Path : ModulePaths) {
OS << " " << Path << "\n";
}
@@ -110,8 +112,9 @@ class DefinitionNotFoundInSummary
}
void log(raw_ostream &OS) const override {
- OS << "No symbol for global value '" << GlobalValueName
- << "' (GUID: " << GlobalValue::getGUID(GlobalValueName) << ") in:\n";
+ OS << "No symbol for global value '" << GlobalValueName << "' (GUID: "
+ << GlobalValue::getGUIDAssumingExternalLinkage(GlobalValueName)
+ << ") in:\n";
for (const std::string &Path : ModulePaths) {
OS << " " << Path << "\n";
}
@@ -135,7 +138,8 @@ char DefinitionNotFoundInSummary::ID = 0;
Expected<StringRef> getMainModulePath(StringRef FunctionName,
ModuleSummaryIndex &Index) {
// Summaries use unmangled names.
- GlobalValue::GUID G = GlobalValue::getGUID(FunctionName);
+ GlobalValue::GUID G =
+ GlobalValue::getGUIDAssumingExternalLinkage(FunctionName);
ValueInfo VI = Index.getValueInfo(G);
// We need a unique definition, otherwise don't try further.
diff --git a/llvm/include/llvm/IR/GlobalValue.h b/llvm/include/llvm/IR/GlobalValue.h
index 2176e2c2cfbfc..04907b3d17eb0 100644
--- a/llvm/include/llvm/IR/GlobalValue.h
+++ b/llvm/include/llvm/IR/GlobalValue.h
@@ -570,6 +570,11 @@ class GlobalValue : public Constant {
return Name;
}
+ /// Declare a type to represent a global unique identifier for a global value.
+ /// This is a 64 bits hash that is used by PGO and ThinLTO to have a compact
+ /// unique way to identify a symbol.
+ using GUID = uint64_t;
+
/// Return the modified name for a global value suitable to be
/// used as the key for a global lookup (e.g. profile or ThinLTO).
/// The value's original name is \c Name and has linkage of type
@@ -578,18 +583,23 @@ class GlobalValue : public Constant {
GlobalValue::LinkageTypes Linkage,
StringRef FileName);
+private:
/// Return the modified name for this global value suitable to be
/// used as the key for a global lookup (e.g. profile or ThinLTO).
std::string getGlobalIdentifier() const;
- /// Declare a type to represent a global unique identifier for a global value.
- /// This is a 64 bits hash that is used by PGO and ThinLTO to have a compact
- /// unique way to identify a symbol.
- using GUID = uint64_t;
-
/// Return a 64-bit global unique ID constructed from global value name
/// (i.e. returned by getGlobalIdentifier()).
- static GUID getGUID(StringRef GlobalName);
+ static GUID getGUID(StringRef GlobalIdentifier);
+
+public:
+ /// Return a 64-bit global unique ID constructed from the name of a global
+ /// symbol. Since this call doesn't supply the linkage or defining filename,
+ /// the GUID computation will assume that the global has external linkage.
+ static GUID getGUIDAssumingExternalLinkage(StringRef GlobalName) {
+ return getGUID(
+ getGlobalIdentifier(GlobalName, GlobalValue::ExternalLinkage, ""));
+ }
/// Return a 64-bit global unique ID constructed from global value name
/// (i.e. returned by getGlobalIdentifier()).
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 2650456c49841..32b4fa630b7bb 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1339,14 +1339,14 @@ class CfiFunctionIndex {
template <typename... Args> void emplace(Args &&...A) {
StringRef S(std::forward<Args>(A)...);
- GlobalValue::GUID GUID =
- GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(S));
+ GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::dropLLVMManglingEscape(S));
Index[GUID].emplace(S);
}
size_t count(StringRef S) const {
- GlobalValue::GUID GUID =
- GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(S));
+ GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::dropLLVMManglingEscape(S));
auto I = Index.find(GUID);
if (I == Index.end())
return 0;
@@ -1749,8 +1749,10 @@ class ModuleSummaryIndex {
/// Add a global value summary for a value of the given name.
void addGlobalValueSummary(StringRef ValueName,
std::unique_ptr<GlobalValueSummary> Summary) {
- addGlobalValueSummary(getOrInsertValueInfo(GlobalValue::getGUID(ValueName)),
- std::move(Summary));
+ addGlobalValueSummary(
+ getOrInsertValueInfo(
+ GlobalValue::getGUIDAssumingExternalLinkage(ValueName)),
+ std::move(Summary));
}
/// Add a global value summary for the given ValueInfo.
@@ -1887,19 +1889,22 @@ class ModuleSummaryIndex {
/// This accessor can mutate the map and therefore should not be used in
/// the ThinLTO backends.
TypeIdSummary &getOrInsertTypeIdSummary(StringRef TypeId) {
- auto TidIter = TypeIdMap.equal_range(GlobalValue::getGUID(TypeId));
+ auto TidIter = TypeIdMap.equal_range(
+ GlobalValue::getGUIDAssumingExternalLinkage(TypeId));
for (auto &[GUID, TypeIdPair] : make_range(TidIter))
if (TypeIdPair.first == TypeId)
return TypeIdPair.second;
- auto It = TypeIdMap.insert({GlobalValue::getGUID(TypeId),
- {TypeIdSaver.save(TypeId), TypeIdSummary()}});
+ auto It =
+ TypeIdMap.insert({GlobalValue::getGUIDAssumingExternalLinkage(TypeId),
+ {TypeIdSaver.save(TypeId), TypeIdSummary()}});
return It->second.second;
}
/// This returns either a pointer to the type id summary (if present in the
/// summary map) or null (if not present). This may be used when importing.
const TypeIdSummary *getTypeIdSummary(StringRef TypeId) const {
- auto TidIter = TypeIdMap.equal_range(GlobalValue::getGUID(TypeId));
+ auto TidIter = TypeIdMap.equal_range(
+ GlobalValue::getGUIDAssumingExternalLinkage(TypeId));
for (const auto &[GUID, TypeIdPair] : make_range(TidIter))
if (TypeIdPair.first == TypeId)
return &TypeIdPair.second;
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
index d5a91763a981c..531de514822e8 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
@@ -314,7 +314,7 @@ template <> struct CustomMappingTraits<TypeIdSummaryMapTy> {
static void inputOne(IO &io, StringRef Key, TypeIdSummaryMapTy &V) {
TypeIdSummary TId;
io.mapRequired(Key.str().c_str(), TId);
- V.insert({GlobalValue::getGUID(Key), {Key, TId}});
+ V.insert({GlobalValue::getGUIDAssumingExternalLinkage(Key), {Key, TId}});
}
static void output(IO &io, TypeIdSummaryMapTy &V) {
for (auto &TidIter : V)
diff --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h
index e7b154dff0697..008d464d0c582 100644
--- a/llvm/include/llvm/ProfileData/SampleProf.h
+++ b/llvm/include/llvm/ProfileData/SampleProf.h
@@ -1299,7 +1299,7 @@ class FunctionSamples {
static inline FunctionId getRepInFormat(StringRef Name) {
if (Name.empty() || !FunctionSamples::UseMD5)
return FunctionId(Name);
- return FunctionId(Function::getGUID(Name));
+ return FunctionId(Function::getGUIDAssumingExternalLinkage(Name));
}
raw_ostream &operator<<(raw_ostream &OS, const FunctionSamples &FS);
diff --git a/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h b/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
index d714dba4a9687..12d1031fd37a0 100644
--- a/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
+++ b/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
@@ -109,11 +109,12 @@ class PseudoProbeManager {
}
const PseudoProbeDescriptor *getDesc(StringRef FProfileName) const {
- return getDesc(Function::getGUID(FProfileName));
+ return getDesc(Function::getGUIDAssumingExternalLinkage(FProfileName));
}
const PseudoProbeDescriptor *getDesc(const Function &F) const {
- return getDesc(Function::getGUID(FunctionSamples::getCanonicalFnName(F)));
+ return getDesc(Function::getGUIDAssumingExternalLinkage(
+ FunctionSamples::getCanonicalFnName(F)));
}
bool profileIsHashMismatched(const PseudoProbeDescriptor &FuncDesc,
diff --git a/llvm/lib/Analysis/CtxProfAnalysis.cpp b/llvm/lib/Analysis/CtxProfAnalysis.cpp
index e021e2a801006..f980ca7a94b40 100644
--- a/llvm/lib/Analysis/CtxProfAnalysis.cpp
+++ b/llvm/lib/Analysis/CtxProfAnalysis.cpp
@@ -57,7 +57,7 @@ PreservedAnalyses AssignGUIDPass::run(Module &M, ModuleAnalysisManager &MAM) {
GlobalValue::GUID AssignGUIDPass::getGUID(const Function &F) {
if (F.isDeclaration()) {
assert(GlobalValue::isExternalLinkage(F.getLinkage()));
- return GlobalValue::getGUID(F.getGlobalIdentifier());
+ return F.getGUID();
}
auto *MD = F.getMetadata(GUIDMetadataName);
assert(MD && "guid not found for defined function");
diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index 611d4bfbc69e8..1f2d1fcce2923 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -222,7 +222,8 @@ static void addIntrinsicToSummary(
auto *TypeId = dyn_cast<MDString>(TypeMDVal->getMetadata());
if (!TypeId)
break;
- GlobalValue::GUID Guid = GlobalValue::getGUID(TypeId->getString());
+ GlobalValue::GUID Guid =
+ GlobalValue::getGUIDAssumingExternalLinkage(TypeId->getString());
// Produce a summary from type.test intrinsics. We only summarize type.test
// intrinsics that are used other than by an llvm.assume intrinsic.
@@ -250,7 +251,8 @@ static void addIntrinsicToSummary(
auto *TypeId = dyn_cast<MDString>(TypeMDVal->getMetadata());
if (!TypeId)
break;
- GlobalValue::GUID Guid = GlobalValue::getGUID(TypeId->getString());
+ GlobalValue::GUID Guid =
+ GlobalValue::getGUIDAssumingExternalLinkage(TypeId->getString());
SmallVector<DevirtCallSite, 4> DevirtCalls;
SmallVector<Instruction *, 4> LoadedPtrs;
@@ -906,7 +908,8 @@ static void computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A,
// Set LiveRoot flag on entries matching the given value name.
static void setLiveRoot(ModuleSummaryIndex &Index, StringRef Name) {
- if (ValueInfo VI = Index.getValueInfo(GlobalValue::getGUID(Name)))
+ if (ValueInfo VI =
+ Index.getValueInfo(GlobalValue::getGUIDAssumingExternalLinkage(Name)))
for (const auto &Summary : VI.getSummaryList())
Summary->setLive(true);
}
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index c8d792981793d..faf4109ff09f0 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -9005,7 +9005,7 @@ bool LLParser::parseTypeIdEntry(unsigned ID) {
for (auto TIDRef : FwdRefTIDs->second) {
assert(!*TIDRef.first &&
"Forward referenced type id GUID expected to be 0");
- *TIDRef.first = GlobalValue::getGUID(Name);
+ *TIDRef.first = GlobalValue::getGUIDAssumingExternalLinkage(Name);
}
ForwardRefTypeIds.erase(FwdRefTIDs);
}
@@ -9110,7 +9110,7 @@ bool LLParser::parseTypeIdCompatibleVtableEntry(unsigned ID) {
for (auto TIDRef : FwdRefTIDs->second) {
assert(!*TIDRef.first &&
"Forward referenced type id GUID expected to be 0");
- *TIDRef.first = GlobalValue::getGUID(Name);
+ *TIDRef.first = GlobalValue::getGUIDAssumingExternalLinkage(Name);
}
ForwardRefTypeIds.erase(FwdRefTIDs);
}
@@ -9426,7 +9426,7 @@ bool LLParser::addGlobalValueToIndex(
assert(
(!GlobalValue::isLocalLinkage(Linkage) || !SourceFileName.empty()) &&
"Need a source_filename to compute GUID for local");
- GUID = GlobalValue::getGUID(
+ GUID = GlobalValue::getGUIDAssumingExternalLinkage(
GlobalValue::getGlobalIdentifier(Name, Linkage, SourceFileName));
VI = Index->getOrInsertValueInfo(GUID, Index->saveString(Name));
}
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 40e755902b724..07b92fc9efd1c 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -7218,10 +7218,10 @@ void ModuleSummaryIndexBitcodeReader::setValueGUID(
StringRef SourceFileName) {
std::string GlobalId =
GlobalValue::getGlobalIdentifier(ValueName, Linkage, SourceFileName);
- auto ValueGUID = GlobalValue::getGUID(GlobalId);
+ auto ValueGUID = GlobalValue::getGUIDAssumingExternalLinkage(GlobalId);
auto OriginalNameID = ValueGUID;
if (GlobalValue::isLocalLinkage(Linkage))
- OriginalNameID = GlobalValue::getGUID(ValueName);
+ OriginalNameID = GlobalValue::getGUIDAssumingExternalLinkage(ValueName);
if (PrintSummaryGUIDs)
dbgs() << "GUID " << ValueGUID << "(" << OriginalNameID << ") is "
<< ValueName << "\n";
diff --git a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
index 5dda38383a656..618deef2a74ea 100644
--- a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
@@ -34,7 +34,7 @@ void PseudoProbeHandler::emitPseudoProbe(uint64_t Guid, uint64_t Index,
// Use caching to avoid redundant md5 computation for build speed.
uint64_t &CallerGuid = NameGuidMap[Name];
if (!CallerGuid)
- CallerGuid = Function::getGUID(Name);
+ CallerGuid = Function::getGUIDAssumingExternalLinkage(Name);
uint64_t CallerProbeId = PseudoProbeDwarfDiscriminator::extractProbeIndex(
InlinedAt->getDiscriminator());
ReversedInlineStack.emplace_back(CallerGuid, CallerProbeId);
diff --git a/llvm/lib/CodeGen/PseudoProbeInserter.cpp b/llvm/lib/CodeGen/PseudoProbeInserter.cpp
index 913e0035b046f..c911e8453613d 100644
--- a/llvm/lib/CodeGen/PseudoProbeInserter.cpp
+++ b/llvm/lib/CodeGen/PseudoProbeInserter.cpp
@@ -129,7 +129,7 @@ class PseudoProbeInserter : public MachineFunctionPass {
private:
uint64_t getFuncGUID(Module *M, DILocation *DL) {
auto Name = DL->getSubprogramLinkageName();
- return Function::getGUID(Name);
+ return Function::getGUIDAssumingExternalLinkage(Name);
}
bool ShouldRun = false;
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index ae68da0182dc4..b23969e89e9cb 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -3172,7 +3172,7 @@ void AssemblyWriter::printModuleSummaryIndex() {
// Print the TypeIdCompatibleVtableMap entries.
for (auto &TId : TheIndex->typeIdCompatibleVtableMap()) {
- auto GUID = GlobalValue::getGUID(TId.first);
+ auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(TId.first);
Out << "^" << Machine.getTypeIdCompatibleVtableSlot(TId.first)
<< " = typeidCompatibleVTable: (name: \"" << TId.first << "\"";
printTypeIdCompatibleVtableSummary(TId.second);
diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp
index 8ca44719a3f94..91c4a8fc387a0 100644
--- a/llvm/lib/IR/Globals.cpp
+++ b/llvm/lib/IR/Globals.cpp
@@ -73,8 +73,8 @@ void GlobalValue::copyAttributesFrom(const GlobalValue *Src) {
removeSanitizerMetadata();
}
-GlobalValue::GUID GlobalValue::getGUID(StringRef GlobalName) {
- return MD5Hash(GlobalName);
+GlobalValue::GUID GlobalValue::getGUID(StringRef GlobalIdentifier) {
+ return MD5Hash(GlobalIdentifier);
}
void GlobalValue::removeFromParent() {
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index e895a46b8cd77..d91212691f051 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -1038,8 +1038,9 @@ Error LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
SymbolResolution Res = *ResITmp++;
if (!Sym.getIRName().empty()) {
- auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
- Sym.getIRName(), GlobalValue::ExternalLinkage, ""));
+ auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::getGlobalIdentifier(Sym.getIRName(),
+ GlobalValue::ExternalLinkage, ""));
if (Res.Prevailing)
ThinLTO.PrevailingModuleForGUID[GUID] = BM.getModuleIdentifier();
}
@@ -1059,8 +1060,9 @@ Error LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
SymbolResolution Res = *ResI++;
if (!Sym.getIRName().empty()) {
- auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
- Sym.getIRName(), GlobalValue::ExternalLinkage, ""));
+ auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::getGlobalIdentifier(Sym.getIRName(),
+ GlobalValue::ExternalLinkage, ""));
if (Res.Prevailing) {
assert(ThinLTO.PrevailingModuleForGUID[GUID] ==
BM.getModuleIdentifier());
@@ -1169,7 +1171,7 @@ Error LTO::run(AddStreamFn AddStream, FileCache Cache) {
if (Res.second.IRName.empty())
continue;
- GlobalValue::GUID GUID = GlobalValue::getGUID(
+ GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
GlobalValue::dropLLVMManglingEscape(Res.second.IRName));
if (Res.second.VisibleOutsideSummary && Res.second.Prevailing)
@@ -1944,7 +1946,7 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
if (Res.second.Partition != GlobalResolution::External ||
!Res.second.isPrevailingIRSymbol())
continue;
- auto GUID = GlobalValue::getGUID(
+ auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(
GlobalValue::dropLLVMManglingEscape(Res.second.IRName));
// Mark exported unless index-based analysis determined it to be dead.
if (ThinLTO.CombinedIndex.isGUIDLive(GUID))
diff --git a/llvm/lib/LTO/ThinLTOCodeGener...
[truncated]
|
| @@ -176,7 +176,7 @@ void MipsSEDAGToDAGISel::processFunctionAfterISel(MachineFunction &MF) { | |||
| case Mips::JAL: | |||
| case Mips::JAL_MM: | |||
| if (MI.getOperand(0).isGlobal() && | |||
| MI.getOperand(0).getGlobal()->getGlobalIdentifier() == "_mcount") | |||
| MI.getOperand(0).getGlobal()->getName() == "_mcount") | |||
There was a problem hiding this comment.
drive-by comment, the change from getGlobalIdentifier() [1] to getName() doesn't seem always equivalent here. llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp has a similar change (for error reporting).
Is there a plan to move way from std::string GlobalValue::getGlobalIdentifier() to getName()?
[1]
llvm-project/llvm/lib/IR/Globals.cpp
Lines 185 to 188 in 7eccafc
There was a problem hiding this comment.
Thanks, you're right, this isn't equivalent. I've added a check that it's external before the check on the name, which should make it equivalent now.
By the way, I notice that the other references to "_mcount" in this file just check the symbol name, which IIUC would also have the issue of matching a locally defined "_mcount" symbol. Not sure if that's a problem here.
Is there a plan to move way from std::string GlobalValue::getGlobalIdentifier() to getName()?
We want to move away from getGlobalIdentifier() insofar as it's used to uniquely identify a global, and towards getGUID() instead. Cases like this which care about matching a specific global symbol aren't such a problem.
orodley
force-pushed
the
users/orodley/guid-1
branch
from
22dd4ce to
8ef355a
Compare
llvm/include/llvm/IR/GlobalValue.h
Outdated
| /// Return a 64-bit global unique ID constructed from global value name | ||
| /// (i.e. returned by getGlobalIdentifier()). | ||
| static GUID getGUID(StringRef GlobalName); | ||
| static GUID getGUID(StringRef GlobalIdentifier); |
There was a problem hiding this comment.
Where are the remaining callers of this function, besides from the new getGUIDAssumingExternalLinkage below?
There was a problem hiding this comment.
Good point -- there are no remaining users so these two functions are redundant now. I've removed the static getGUID now.
llvm/include/llvm/IR/GlobalValue.h
Outdated
| /// the GUID computation will assume that the global has external linkage. | ||
| static GUID getGUIDAssumingExternalLinkage(StringRef GlobalName) { | ||
| return getGUID( | ||
| getGlobalIdentifier(GlobalName, GlobalValue::ExternalLinkage, "")); |
There was a problem hiding this comment.
Why add this invocation of getGlobalIdentifier which doesn't do anything other than pass through GlobalName?
| buildOpDecorate( | ||
| FuncVReg, MIRBuilder, SPIRV::Decoration::LinkageAttributes, | ||
| {static_cast<uint32_t>(LnkTy)}, | ||
| Function::getGlobalIdentifier(F.getName(), F.getLinkage(), |
There was a problem hiding this comment.
Can this just use F.getName() ? Not clear why they would need the global identifier with the file name. Also, I found this was added in https://reviews.llvm.org/D116464, which also added a call to getGlobalIdentifier from SPIRVInstructionSelector::selectGlobalValue, and that has since been changed to use getName().
There was a problem hiding this comment.
Changed to F.getName()
orodley
force-pushed
the
users/orodley/guid-1
branch
from
8ef355a to
49e4618
Compare
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. * Where users don't actually need the gloalIdentifier and just need the name, call getName() instead. * There are a few cases where none 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.
orodley
force-pushed
the
users/orodley/guid-1
branch
from
49e4618 to
3441370
Compare
jyli0116
pushed a commit
to jyli0116/llvm-project
that referenced
this pull request
Apr 28, 2025
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: * 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.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
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: * 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.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
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: * 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.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
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: * 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
getGUIDAssumingExternalLinkage. This is simply making explicit at the
callsite what is currently implicit.
GlobalValue instance.
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.