Skip to content

Commit 3aa5d71

Browse files
Revert "[PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles." (#75835)
Reverts #74008 The compiler-rt test failed due to `llvm-dis` not found (https://lab.llvm.org/buildbot/#/builders/127/builds/59884) Will revert and investigate how to require the proper dependency.
1 parent 3768039 commit 3aa5d71

17 files changed

+128
-363
lines changed

compiler-rt/test/profile/instrprof-thinlto-indirect-call-promotion.cpp

Lines changed: 0 additions & 115 deletions
This file was deleted.

llvm/include/llvm/IR/GlobalValue.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,6 @@ namespace Intrinsic {
4141
typedef unsigned ID;
4242
} // end namespace Intrinsic
4343

44-
// Choose ';' as the delimiter. ':' was used once but it doesn't work well for
45-
// Objective-C functions which commonly have :'s in their names.
46-
inline constexpr char kGlobalIdentifierDelimiter = ';';
47-
4844
class GlobalValue : public Constant {
4945
public:
5046
/// An enumeration for the kinds of linkage for global values.

llvm/include/llvm/ProfileData/InstrProf.h

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,6 @@ inline StringRef getInstrProfCounterBiasVarName() {
171171
/// Return the marker used to separate PGO names during serialization.
172172
inline StringRef getInstrProfNameSeparator() { return "\01"; }
173173

174-
/// Please use getIRPGOFuncName for LLVM IR instrumentation. This function is
175-
/// for front-end (Clang, etc) instrumentation.
176174
/// Return the modified name for function \c F suitable to be
177175
/// used the key for profile lookup. Variable \c InLTO indicates if this
178176
/// is called in LTO optimization passes.
@@ -198,22 +196,20 @@ std::string getIRPGOFuncName(const Function &F, bool InLTO = false);
198196
std::pair<StringRef, StringRef> getParsedIRPGOFuncName(StringRef IRPGOFuncName);
199197

200198
/// Return the name of the global variable used to store a function
201-
/// name in PGO instrumentation. \c FuncName is the IRPGO function name
202-
/// (returned by \c getIRPGOFuncName) for LLVM IR instrumentation and PGO
203-
/// function name (returned by \c getPGOFuncName) for front-end instrumentation.
199+
/// name in PGO instrumentation. \c FuncName is the name of the function
200+
/// returned by the \c getPGOFuncName call.
204201
std::string getPGOFuncNameVarName(StringRef FuncName,
205202
GlobalValue::LinkageTypes Linkage);
206203

207204
/// Create and return the global variable for function name used in PGO
208-
/// instrumentation. \c FuncName is the IRPGO function name (returned by
209-
/// \c getIRPGOFuncName) for LLVM IR instrumentation and PGO function name
210-
/// (returned by \c getPGOFuncName) for front-end instrumentation.
205+
/// instrumentation. \c FuncName is the name of the function returned
206+
/// by \c getPGOFuncName call.
211207
GlobalVariable *createPGOFuncNameVar(Function &F, StringRef PGOFuncName);
212208

213209
/// Create and return the global variable for function name used in PGO
214-
/// instrumentation. \c FuncName is the IRPGO function name (returned by
215-
/// \c getIRPGOFuncName) for LLVM IR instrumentation and PGO function name
216-
/// (returned by \c getPGOFuncName) for front-end instrumentation.
210+
/// instrumentation. /// \c FuncName is the name of the function
211+
/// returned by \c getPGOFuncName call, \c M is the owning module,
212+
/// and \c Linkage is the linkage of the instrumented function.
217213
GlobalVariable *createPGOFuncNameVar(Module &M,
218214
GlobalValue::LinkageTypes Linkage,
219215
StringRef PGOFuncName);
@@ -421,11 +417,11 @@ uint64_t ComputeHash(StringRef K);
421417

422418
} // end namespace IndexedInstrProf
423419

424-
/// A symbol table used for function [IR]PGO name look-up with keys
420+
/// A symbol table used for function PGO name look-up with keys
425421
/// (such as pointers, md5hash values) to the function. A function's
426-
/// [IR]PGO name or name's md5hash are used in retrieving the profile
427-
/// data of the function. See \c getIRPGOFuncName() and \c getPGOFuncName
428-
/// methods for details how [IR]PGO name is formed.
422+
/// PGO name or name's md5hash are used in retrieving the profile
423+
/// data of the function. See \c getPGOFuncName() method for details
424+
/// on how PGO name is formed.
429425
class InstrProfSymtab {
430426
public:
431427
using AddrHashMap = std::vector<std::pair<uint64_t, uint64_t>>;

llvm/lib/IR/Globals.cpp

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -144,27 +144,25 @@ void GlobalObject::copyAttributesFrom(const GlobalObject *Src) {
144144
std::string GlobalValue::getGlobalIdentifier(StringRef Name,
145145
GlobalValue::LinkageTypes Linkage,
146146
StringRef FileName) {
147+
147148
// Value names may be prefixed with a binary '1' to indicate
148149
// that the backend should not modify the symbols due to any platform
149150
// naming convention. Do not include that '1' in the PGO profile name.
150151
if (Name[0] == '\1')
151152
Name = Name.substr(1);
152153

153-
std::string GlobalName;
154+
std::string NewName = std::string(Name);
154155
if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
155156
// For local symbols, prepend the main file name to distinguish them.
156157
// Do not include the full path in the file name since there's no guarantee
157158
// that it will stay the same, e.g., if the files are checked out from
158159
// version control in different locations.
159160
if (FileName.empty())
160-
GlobalName += "<unknown>";
161+
NewName = NewName.insert(0, "<unknown>:");
161162
else
162-
GlobalName += FileName;
163-
164-
GlobalName += kGlobalIdentifierDelimiter;
163+
NewName = NewName.insert(0, FileName.str() + ":");
165164
}
166-
GlobalName += Name;
167-
return GlobalName;
165+
return NewName;
168166
}
169167

170168
std::string GlobalValue::getGlobalIdentifier() const {

llvm/lib/ProfileData/InstrProf.cpp

Lines changed: 10 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -246,27 +246,11 @@ std::string InstrProfError::message() const {
246246

247247
char InstrProfError::ID = 0;
248248

249-
std::string getPGOFuncName(StringRef Name, GlobalValue::LinkageTypes Linkage,
249+
std::string getPGOFuncName(StringRef RawFuncName,
250+
GlobalValue::LinkageTypes Linkage,
250251
StringRef FileName,
251252
uint64_t Version LLVM_ATTRIBUTE_UNUSED) {
252-
// Value names may be prefixed with a binary '1' to indicate
253-
// that the backend should not modify the symbols due to any platform
254-
// naming convention. Do not include that '1' in the PGO profile name.
255-
if (Name[0] == '\1')
256-
Name = Name.substr(1);
257-
258-
std::string NewName = std::string(Name);
259-
if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
260-
// For local symbols, prepend the main file name to distinguish them.
261-
// Do not include the full path in the file name since there's no guarantee
262-
// that it will stay the same, e.g., if the files are checked out from
263-
// version control in different locations.
264-
if (FileName.empty())
265-
NewName = NewName.insert(0, "<unknown>:");
266-
else
267-
NewName = NewName.insert(0, FileName.str() + ":");
268-
}
269-
return NewName;
253+
return GlobalValue::getGlobalIdentifier(RawFuncName, Linkage, FileName);
270254
}
271255

272256
// Strip NumPrefix level of directory name from PathNameStr. If the number of
@@ -316,10 +300,12 @@ getIRPGONameForGlobalObject(const GlobalObject &GO,
316300
GlobalValue::LinkageTypes Linkage,
317301
StringRef FileName) {
318302
SmallString<64> Name;
319-
// FIXME: Mangler's handling is kept outside of `getGlobalIdentifier` for now.
320-
// For more details please check issue #74565.
303+
if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
304+
Name.append(FileName.empty() ? "<unknown>" : FileName);
305+
Name.append(";");
306+
}
321307
Mangler().getNameWithPrefix(Name, &GO, /*CannotUsePrivateLabel=*/true);
322-
return GlobalValue::getGlobalIdentifier(Name, Linkage, FileName);
308+
return Name.str().str();
323309
}
324310

325311
static std::optional<std::string> lookupPGONameFromMetadata(MDNode *MD) {
@@ -366,9 +352,6 @@ std::string getIRPGOFuncName(const Function &F, bool InLTO) {
366352
return getIRPGOObjectName(F, InLTO, getPGOFuncNameMetadata(F));
367353
}
368354

369-
// Please use getIRPGOFuncName for LLVM IR instrumentation. This function is
370-
// for front-end (Clang, etc) instrumentation.
371-
// The implementation is kept for profile matching from older profiles.
372355
// This is similar to `getIRPGOFuncName` except that this function calls
373356
// 'getPGOFuncName' to get a name and `getIRPGOFuncName` calls
374357
// 'getIRPGONameForGlobalObject'. See the difference between two callees in the
@@ -401,9 +384,8 @@ getParsedIRPGOFuncName(StringRef IRPGOFuncName) {
401384
StringRef getFuncNameWithoutPrefix(StringRef PGOFuncName, StringRef FileName) {
402385
if (FileName.empty())
403386
return PGOFuncName;
404-
// Drop the file name including ':' or ';'. See getIRPGONameForGlobalObject as
405-
// well.
406-
if (PGOFuncName.startswith(FileName))
387+
// Drop the file name including ':'. See also getPGOFuncName.
388+
if (PGOFuncName.starts_with(FileName))
407389
PGOFuncName = PGOFuncName.drop_front(FileName.size() + 1);
408390
return PGOFuncName;
409391
}

llvm/lib/ProfileData/InstrProfReader.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,14 +1008,13 @@ class llvm::InstrProfReaderItaniumRemapper
10081008

10091009
/// Extract the original function name from a PGO function name.
10101010
static StringRef extractName(StringRef Name) {
1011-
// We can have multiple pieces separated by kGlobalIdentifierDelimiter (
1012-
// semicolon now and colon in older profiles); there can be pieces both
1013-
// before and after the mangled name. Find the first part that starts with
1014-
// '_Z'; we'll assume that's the mangled name we want.
1011+
// We can have multiple :-separated pieces; there can be pieces both
1012+
// before and after the mangled name. Find the first part that starts
1013+
// with '_Z'; we'll assume that's the mangled name we want.
10151014
std::pair<StringRef, StringRef> Parts = {StringRef(), Name};
10161015
while (true) {
1017-
Parts = Parts.second.split(kGlobalIdentifierDelimiter);
1018-
if (Parts.first.startswith("_Z"))
1016+
Parts = Parts.second.split(':');
1017+
if (Parts.first.starts_with("_Z"))
10191018
return Parts.first;
10201019
if (Parts.second.empty())
10211020
return Name;

llvm/test/Bitcode/thinlto-function-summary-originalnames.ll

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@
66
; COMBINED: <GLOBALVAL_SUMMARY_BLOCK
77
; COMBINED-NEXT: <VERSION
88
; COMBINED-NEXT: <FLAGS
9-
; COMBINED-NEXT: <VALUE_GUID {{.*}} op1=686735765308251824/>
10-
; COMBINED-NEXT: <VALUE_GUID {{.*}} op1=4507502870619175775/>
11-
; COMBINED-NEXT: <VALUE_GUID {{.*}} op1=-8118561185538785069/>
9+
; COMBINED-NEXT: <VALUE_GUID {{.*}} op1=4947176790635855146/>
10+
; COMBINED-NEXT: <VALUE_GUID {{.*}} op1=-6591587165810580810/>
11+
; COMBINED-NEXT: <VALUE_GUID {{.*}} op1=-4377693495213223786/>
1212
; COMBINED-DAG: <COMBINED_PROFILE{{ }}
13-
; COMBINED-DAG: <COMBINED_ORIGINAL_NAME op0=-2012135647395072713/>
14-
; COMBINED-DAG: <COMBINED_GLOBALVAR_INIT_REFS
1513
; COMBINED-DAG: <COMBINED_ORIGINAL_NAME op0=6699318081062747564/>
14+
; COMBINED-DAG: <COMBINED_GLOBALVAR_INIT_REFS
15+
; COMBINED-DAG: <COMBINED_ORIGINAL_NAME op0=-2012135647395072713/>
1616
; COMBINED-DAG: <COMBINED_ALIAS
1717
; COMBINED-DAG: <COMBINED_ORIGINAL_NAME op0=-4170563161550796836/>
1818
; COMBINED-NEXT: </GLOBALVAL_SUMMARY_BLOCK>

0 commit comments

Comments
 (0)