[llvm] [IR] Handle `.__uniq.` in `getGlobalIndentifier` for local linkage objects (PR #98163)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 9 15:55:19 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-ir
Author: Mircea Trofin (mtrofin)
<details>
<summary>Changes</summary>
The `.__uniq.` particle is used when the name of an object with local linkage incorporates a hash of its defining module - for example, if clang is passed `-funique-internal-linkage-names`. We don't need to add the module name in `getGlobalIndentifier` in this case.
We also don't need to apply the `.llvm.` particle when importing post-thinlink, if the function names were already uniq-ed - the former achieves the same goal the latter achieves.
This simplifies matters for carrying out information (like profiling data) between pre and post-thinlink: a function's ID stays the same from frontend through thinlink to the backend.
---
Full diff: https://github.com/llvm/llvm-project/pull/98163.diff
11 Files Affected:
- (modified) llvm/include/llvm/IR/GlobalValue.h (+6)
- (modified) llvm/include/llvm/IR/ModuleSummaryIndex.h (+5-2)
- (modified) llvm/include/llvm/ProfileData/SampleProf.h (+6-9)
- (modified) llvm/lib/IR/Globals.cpp (+5-4)
- (modified) llvm/lib/ProfileData/InstrProf.cpp (+2-3)
- (modified) llvm/lib/ProfileData/SampleProfWriter.cpp (+2-1)
- (modified) llvm/tools/dsymutil/MachODebugMapParser.cpp (+2-1)
- (modified) llvm/tools/gold/gold-plugin.cpp (+2-1)
- (modified) llvm/tools/llvm-profdata/llvm-profdata.cpp (+2-2)
- (modified) llvm/unittests/IR/FunctionTest.cpp (+22)
- (modified) llvm/unittests/IR/ModuleSummaryIndexTest.cpp (+9)
``````````diff
diff --git a/llvm/include/llvm/IR/GlobalValue.h b/llvm/include/llvm/IR/GlobalValue.h
index 53eddebdd6ae6..2c7f008fe53ba 100644
--- a/llvm/include/llvm/IR/GlobalValue.h
+++ b/llvm/include/llvm/IR/GlobalValue.h
@@ -45,6 +45,12 @@ typedef unsigned ID;
// Objective-C functions which commonly have :'s in their names.
inline constexpr char GlobalIdentifierDelimiter = ';';
+struct NameParticles final {
+ static constexpr const char *LLVMSuffix = ".llvm.";
+ static constexpr const char *PartSuffix = ".part.";
+ static constexpr const char *UniqSuffix = ".__uniq.";
+};
+
class GlobalValue : public Constant {
public:
/// An enumeration for the kinds of linkage for global values.
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 31271ed388e54..926636d9d571b 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1743,8 +1743,10 @@ class ModuleSummaryIndex {
}
static std::string getGlobalNameForLocal(StringRef Name, StringRef Suffix) {
+ if (Name.contains(NameParticles::UniqSuffix))
+ return std::string(Name);
SmallString<256> NewName(Name);
- NewName += ".llvm.";
+ NewName += NameParticles::LLVMSuffix;
NewName += Suffix;
return std::string(NewName);
}
@@ -1754,7 +1756,8 @@ class ModuleSummaryIndex {
/// because it is possible in certain clients (not clang at the moment) for
/// two rounds of ThinLTO optimization and therefore promotion to occur.
static StringRef getOriginalNameBeforePromote(StringRef Name) {
- std::pair<StringRef, StringRef> Pair = Name.rsplit(".llvm.");
+ std::pair<StringRef, StringRef> Pair =
+ Name.rsplit(NameParticles::LLVMSuffix);
return Pair.first;
}
diff --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h
index 5c2a78c14efd0..a05cd40455ddf 100644
--- a/llvm/include/llvm/ProfileData/SampleProf.h
+++ b/llvm/include/llvm/ProfileData/SampleProf.h
@@ -1091,18 +1091,14 @@ class FunctionSamples {
return getCanonicalFnName(F.getName(), Attr);
}
- /// Name suffixes which canonicalization should handle to avoid
- /// profile mismatch.
- static constexpr const char *LLVMSuffix = ".llvm.";
- static constexpr const char *PartSuffix = ".part.";
- static constexpr const char *UniqSuffix = ".__uniq.";
-
static StringRef getCanonicalFnName(StringRef FnName,
StringRef Attr = "selected") {
// Note the sequence of the suffixes in the knownSuffixes array matters.
// If suffix "A" is appended after the suffix "B", "A" should be in front
// of "B" in knownSuffixes.
- const char *KnownSuffixes[] = {LLVMSuffix, PartSuffix, UniqSuffix};
+ const char *KnownSuffixes[] = {NameParticles::LLVMSuffix,
+ NameParticles::PartSuffix,
+ NameParticles::UniqSuffix};
if (Attr == "" || Attr == "all")
return FnName.split('.').first;
if (Attr == "selected") {
@@ -1111,7 +1107,8 @@ class FunctionSamples {
StringRef Suffix(Suf);
// If the profile contains ".__uniq." suffix, don't strip the
// suffix for names in the IR.
- if (Suffix == UniqSuffix && FunctionSamples::HasUniqSuffix)
+ if (Suffix == NameParticles::UniqSuffix &&
+ FunctionSamples::HasUniqSuffix)
continue;
auto It = Cand.rfind(Suffix);
if (It == StringRef::npos)
@@ -1578,7 +1575,7 @@ inline std::string getUniqueInternalLinkagePostfix(const StringRef &FName) {
// numbers or characters but not both.
llvm::APInt IntHash(128, Str.str(), 16);
return toString(IntHash, /* Radix = */ 10, /* Signed = */ false)
- .insert(0, FunctionSamples::UniqSuffix);
+ .insert(0, NameParticles::UniqSuffix);
}
} // end namespace llvm
diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp
index cc37d7371cce3..fe766554c28e5 100644
--- a/llvm/lib/IR/Globals.cpp
+++ b/llvm/lib/IR/Globals.cpp
@@ -159,11 +159,12 @@ std::string GlobalValue::getGlobalIdentifier(StringRef Name,
Name.consume_front("\1");
std::string GlobalName;
- if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
+ if (llvm::GlobalValue::isLocalLinkage(Linkage) &&
+ Name.find(NameParticles::UniqSuffix) == StringRef::npos) {
// For local symbols, prepend the main file name to distinguish them.
- // Do not include the full path in the file name since there's no guarantee
- // that it will stay the same, e.g., if the files are checked out from
- // version control in different locations.
+ // Do not include the full path in the file name since there's no
+ // guarantee that it will stay the same, e.g., if the files are checked
+ // out from version control in different locations.
if (FileName.empty())
GlobalName += "<unknown>";
else
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index 93876e87f20b3..e99cd4365ea30 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -599,10 +599,9 @@ StringRef InstrProfSymtab::getCanonicalName(StringRef PGOName) {
// different modules and should be kept. This is the only suffix with the
// pattern ".xxx" which is kept before matching, other suffixes similar as
// ".llvm." will be stripped.
- const std::string UniqSuffix = ".__uniq.";
- size_t Pos = PGOName.find(UniqSuffix);
+ size_t Pos = PGOName.find(NameParticles::UniqSuffix);
if (Pos != StringRef::npos)
- Pos += UniqSuffix.length();
+ Pos += strlen(NameParticles::UniqSuffix);
else
Pos = 0;
diff --git a/llvm/lib/ProfileData/SampleProfWriter.cpp b/llvm/lib/ProfileData/SampleProfWriter.cpp
index 1630fefb4fcfb..4a10e7310c444 100644
--- a/llvm/lib/ProfileData/SampleProfWriter.cpp
+++ b/llvm/lib/ProfileData/SampleProfWriter.cpp
@@ -19,6 +19,7 @@
#include "llvm/ProfileData/SampleProfWriter.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/IR/GlobalValue.h"
#include "llvm/ProfileData/ProfileCommon.h"
#include "llvm/ProfileData/SampleProf.h"
#include "llvm/Support/Compression.h"
@@ -372,7 +373,7 @@ std::error_code SampleProfileWriterExtBinaryBase::writeNameTableSection(
// Original names are unavailable if using MD5, so this option has no use.
if (!UseMD5) {
for (const auto &I : NameTable) {
- if (I.first.stringRef().contains(FunctionSamples::UniqSuffix)) {
+ if (I.first.stringRef().contains(NameParticles::UniqSuffix)) {
addSectionFlag(SecNameTable, SecNameTableFlags::SecFlagUniqSuffix);
break;
}
diff --git a/llvm/tools/dsymutil/MachODebugMapParser.cpp b/llvm/tools/dsymutil/MachODebugMapParser.cpp
index e28c976d6ace3..4014c5c9ebedf 100644
--- a/llvm/tools/dsymutil/MachODebugMapParser.cpp
+++ b/llvm/tools/dsymutil/MachODebugMapParser.cpp
@@ -12,6 +12,7 @@
#include "RelocationMap.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/SmallSet.h"
+#include "llvm/IR/GlobalValue.h"
#include "llvm/Object/MachO.h"
#include "llvm/Support/Chrono.h"
#include "llvm/Support/Path.h"
@@ -719,7 +720,7 @@ void MachODebugMapParser::handleStabSymbolTableEntry(
for (auto Iter = CurrentObjectAddresses.begin();
Iter != CurrentObjectAddresses.end(); ++Iter) {
llvm::StringRef SymbolName = Iter->getKey();
- auto Pos = SymbolName.rfind(".llvm.");
+ auto Pos = SymbolName.rfind(NameParticles::LLVMSuffix);
if (Pos != llvm::StringRef::npos && SymbolName.substr(0, Pos) == Name) {
ObjectSymIt = Iter;
break;
diff --git a/llvm/tools/gold/gold-plugin.cpp b/llvm/tools/gold/gold-plugin.cpp
index 0b175a3852e42..3e6d1900350d8 100644
--- a/llvm/tools/gold/gold-plugin.cpp
+++ b/llvm/tools/gold/gold-plugin.cpp
@@ -20,6 +20,7 @@
#include "llvm/Config/llvm-config.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/DiagnosticPrinter.h"
+#include "llvm/IR/GlobalValue.h"
#include "llvm/LTO/LTO.h"
#include "llvm/Object/Error.h"
#include "llvm/Remarks/HotnessThresholdParser.h"
@@ -592,7 +593,7 @@ static ld_plugin_status claim_file_hook(const ld_plugin_input_file *file,
// unique name.
cf.name = file->name;
if (file->offset)
- cf.name += ".llvm." + std::to_string(file->offset) + "." +
+ cf.name += NameParticles::LLVMSuffix + std::to_string(file->offset) + "." +
sys::path::filename(Obj->getSourceFileName()).str();
for (auto &Sym : Obj->symbols()) {
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 1f6c4c604d57b..bd296d83bcb45 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -1108,7 +1108,7 @@ adjustInstrProfile(std::unique_ptr<WriterContext> &WC,
auto checkSampleProfileHasFUnique = [&Reader]() {
for (const auto &PD : Reader->getProfiles()) {
auto &FContext = PD.second.getContext();
- if (FContext.toString().find(FunctionSamples::UniqSuffix) !=
+ if (FContext.toString().find(NameParticles::UniqSuffix) !=
std::string::npos) {
return true;
}
@@ -1142,7 +1142,7 @@ adjustInstrProfile(std::unique_ptr<WriterContext> &WC,
}
// This name should have a static linkage.
- size_t PostfixPos = NewName.find(FunctionSamples::UniqSuffix);
+ size_t PostfixPos = NewName.find(NameParticles::UniqSuffix);
bool ProfileHasFUnique = (PostfixPos != StringRef::npos);
// If sample profile and instrumented profile do not agree on symbol
diff --git a/llvm/unittests/IR/FunctionTest.cpp b/llvm/unittests/IR/FunctionTest.cpp
index 9aaff3ea33830..5828b30172c1d 100644
--- a/llvm/unittests/IR/FunctionTest.cpp
+++ b/llvm/unittests/IR/FunctionTest.cpp
@@ -509,4 +509,26 @@ TEST(FunctionTest, UWTable) {
EXPECT_FALSE(F.hasUWTable());
EXPECT_TRUE(F.getUWTableKind() == UWTableKind::None);
}
+
+TEST(FunctionTest, GlobalIdentifier) {
+ // Technically a GlobalValue test, but using Functions as test subject.
+ LLVMContext Ctx;
+ Module M1("test1", Ctx);
+ Module M2("test2", Ctx);
+ Module M3("test3", Ctx);
+
+ static const char *Name = "F.__uniq.1234";
+
+ auto *G = Function::Create(
+ llvm::FunctionType::get(llvm::Type::getVoidTy(Ctx), false),
+ llvm::GlobalValue::ExternalLinkage, Name, &M1);
+ auto *UniqL1 = Function::Create(
+ llvm::FunctionType::get(llvm::Type::getVoidTy(Ctx), false),
+ llvm::GlobalValue::InternalLinkage, Name, &M2);
+ auto *UniqL2 = Function::Create(
+ llvm::FunctionType::get(llvm::Type::getVoidTy(Ctx), false),
+ llvm::GlobalValue::InternalLinkage, Name, &M3);
+ EXPECT_EQ(G->getGUID(), UniqL1->getGUID());
+ EXPECT_EQ(UniqL1->getGUID(), UniqL2->getGUID());
+}
} // end namespace
diff --git a/llvm/unittests/IR/ModuleSummaryIndexTest.cpp b/llvm/unittests/IR/ModuleSummaryIndexTest.cpp
index f716a04ad3dcc..0f036faf2596d 100644
--- a/llvm/unittests/IR/ModuleSummaryIndexTest.cpp
+++ b/llvm/unittests/IR/ModuleSummaryIndexTest.cpp
@@ -8,6 +8,7 @@
#include "llvm/IR/ModuleSummaryIndex.h"
#include "llvm/AsmParser/Parser.h"
+#include "llvm/IR/GlobalValue.h"
#include "llvm/Support/SourceMgr.h"
#include "gtest/gtest.h"
@@ -54,4 +55,12 @@ Versions: 0 MIB:
AllocType 2 StackIds: 0, 1, 2, 4
)");
}
+
+TEST(ModuleSummaryIndexTest, LocalPromotion) {
+ EXPECT_EQ(ModuleSummaryIndex::getGlobalNameForLocal("Hello", "SomeSuffix"),
+ "Hello.llvm.SomeSuffix");
+ EXPECT_EQ(ModuleSummaryIndex::getGlobalNameForLocal("Hello.__uniq.1234",
+ "SomeSuffix"),
+ "Hello.__uniq.1234");
+}
} // end anonymous namespace
``````````
</details>
https://github.com/llvm/llvm-project/pull/98163
More information about the llvm-commits
mailing list