shylie/llvm-project
1
0
Fork 0
Code Issues Pull Requests Actions 6 Packages Projects Releases Wiki Activity

[InstrProf] No linkage prefixes in IRPGO names (#76994)

Browse Source 
Change the format of IRPGO counter names to
`[<filepath>;]<mangled-name>` which is computed by
`GlobalValue::getGlobalIdentifier()` to fix #74565.

In fe051934cbb0aaf25d960d7d45305135635d650b
(https://reviews.llvm.org/D156569) the format of IRPGO counter names was
changed to be `[<filepath>;]<linkage-name>` where `<linkage-name>` is
basically `F.getName()` with some prefix, e.g., `_` or `l_` on Mach-O
(yes, it is confusing that `<linkage-name>` is computed with
`Mangler().getNameWithPrefix()` while `<mangled-name>` is just
`F.getName()`). We discovered in #74565 that this causes some missed
import issues on some targets and #74008 is a partial fix.

Since `<mangled-name>` may not match the `<linkage-name>` on some
targets like Mach-O, we will need to post-process the output of
`llvm-profdata order` before passing to the linker via `-order_file`.

Profiles generated after fe051934cbb0aaf25d960d7d45305135635d650b will
become stale after this diff, but I think this is acceptable since that
patch landed after the LLVM 18 cut which hasn't been released yet.
This commit is contained in:
Ellis Hoag 2024-01-04 16:13:57 -08:00 committed by GitHub
parent 786cf76f43
commit 9a2df55f47
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 16 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -28,20 +28,6 @@
// LTO if default linker is GNU ld or gold anyway.
// REQUIRES: lld-available
// Test should fail where linkage-name and mangled-name diverges, see issue https://github.com/llvm/llvm-project/issues/74565).
// Currently, this name divergence happens on Mach-O object file format, or on
// many (but not all) 32-bit Windows systems.
//
// XFAIL: system-darwin
//
// Mark 32-bit Windows as UNSUPPORTED for now as opposed to XFAIL. This test
// should fail on many (but not all) 32-bit Windows systems and succeed on the
// rest. The flexibility in triple string parsing makes it tricky to capture
// both sets accurately. i[3-9]86 specifies arch as Triple::ArchType::x86, (win32|windows)
// specifies OS as Triple::OS::Win32
//
// UNSUPPORTED: target={{i.86.*windows.*}}
// RUN: rm -rf %t && split-file %s %t && cd %t
// Do setup work for all below tests.

2
lld/test/MachO/pgo-warn-mismatch.ll
View File

@ -24,7 +24,7 @@ entry:
;--- cs.proftext
:csir
_foo
foo
# Func Hash:
2277602155505015273
# Num Counters:

27
llvm/lib/ProfileData/InstrProf.cpp
View File

@ -14,7 +14,6 @@
#include "llvm/ProfileData/InstrProf.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
@ -27,7 +26,6 @@
#include "llvm/IR/Instruction.h"
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/MDBuilder.h"
#include "llvm/IR/Mangler.h"
#include "llvm/IR/Metadata.h"
#include "llvm/IR/Module.h"
#include "llvm/IR/Type.h"
@ -297,29 +295,20 @@ static StringRef getStrippedSourceFileName(const GlobalObject &GO) {
return FileName;
}
// The PGO name has the format [<filepath>;]<linkage-name> where <filepath>; is
// provided if linkage is local and <linkage-name> is the mangled function
// name. The filepath is used to discriminate possibly identical function names.
// ; is used because it is unlikely to be found in either <filepath> or
// <linkage-name>.
// The PGO name has the format [<filepath>;]<mangled-name> where <filepath>; is
// provided if linkage is local and is used to discriminate possibly identical
// mangled names. ";" is used because it is unlikely to be found in either
// <filepath> or <mangled-name>.
//
// Older compilers used getPGOFuncName() which has the format
// [<filepath>:]<function-name>. <filepath> is used to discriminate between
// possibly identical function names when linkage is local and <function-name>
// simply comes from F.getName(). This caused trouble for Objective-C functions
// which commonly have :'s in their names. Also, since <function-name> is not
// mangled, they cannot be passed to Mach-O linkers via -order_file. We still
// need to compute this name to lookup functions from profiles built by older
// compilers.
// [<filepath>:]<mangled-name>. This caused trouble for Objective-C functions
// which commonly have :'s in their names. We still need to compute this name to
// lookup functions from profiles built by older compilers.
static std::string
getIRPGONameForGlobalObject(const GlobalObject &GO,
GlobalValue::LinkageTypes Linkage,
StringRef FileName) {
SmallString<64> Name;
// FIXME: Mangler's handling is kept outside of `getGlobalIdentifier` for now.
// For more details please check issue #74565.
Mangler().getNameWithPrefix(Name, &GO, /*CannotUsePrivateLabel=*/true);
return GlobalValue::getGlobalIdentifier(Name, Linkage, FileName);
return GlobalValue::getGlobalIdentifier(GO.getName(), Linkage, FileName);
}
static std::optional<std::string> lookupPGONameFromMetadata(MDNode *MD) {

6
llvm/tools/llvm-profdata/llvm-profdata.cpp
View File

@ -3158,7 +3158,11 @@ static int order_main(int argc, const char *argv[]) {
BalancedPartitioning BP(Config);
BP.run(Nodes);
WithColor::note() << "# Ordered " << Nodes.size() << " functions\n";
OS << "# Ordered " << Nodes.size() << " functions\n";
OS << "# Warning: Mach-O may prefix symbols with \"_\" depending on the "
"linkage and this output does not take that into account. Some "
"post-processing may be required before passing to the linker via "
"-order_file.\n";
for (auto &N : Nodes) {
auto [Filename, ParsedFuncName] =
getParsedIRPGOFuncName(Reader->getSymtab().getFuncOrVarName(N.Id));

18
llvm/unittests/ProfileData/InstrProfTest.cpp
View File

@ -542,22 +542,14 @@ TEST_F(InstrProfTest, test_memprof_merge) {
TEST_F(InstrProfTest, test_irpgo_function_name) {
LLVMContext Ctx;
auto M = std::make_unique<Module>("MyModule.cpp", Ctx);
// Use Mach-O mangling so that non-private symbols get a `_` prefix.
M->setDataLayout(DataLayout("m:o"));
auto *FTy = FunctionType::get(Type::getVoidTy(Ctx), /*isVarArg=*/false);
std::vector<std::tuple<StringRef, Function::LinkageTypes, StringRef>> Data;
Data.emplace_back("ExternalFoo", Function::ExternalLinkage, "_ExternalFoo");
Data.emplace_back("ExternalFoo", Function::ExternalLinkage, "ExternalFoo");
Data.emplace_back("InternalFoo", Function::InternalLinkage,
"MyModule.cpp;_InternalFoo");
Data.emplace_back("PrivateFoo", Function::PrivateLinkage,
"MyModule.cpp;l_PrivateFoo");
Data.emplace_back("WeakODRFoo", Function::WeakODRLinkage, "_WeakODRFoo");
// Test Objective-C symbols
"MyModule.cpp;InternalFoo");
Data.emplace_back("\01-[C dynamicFoo:]", Function::ExternalLinkage,
"-[C dynamicFoo:]");
Data.emplace_back("-<C directFoo:>", Function::ExternalLinkage,
"_-<C directFoo:>");
Data.emplace_back("\01-[C internalFoo:]", Function::InternalLinkage,
"MyModule.cpp;-[C internalFoo:]");
@ -590,10 +582,6 @@ TEST_F(InstrProfTest, test_pgo_function_name) {
Data.emplace_back("ExternalFoo", Function::ExternalLinkage, "ExternalFoo");
Data.emplace_back("InternalFoo", Function::InternalLinkage,
"MyModule.cpp:InternalFoo");
Data.emplace_back("PrivateFoo", Function::PrivateLinkage,
"MyModule.cpp:PrivateFoo");
Data.emplace_back("WeakODRFoo", Function::WeakODRLinkage, "WeakODRFoo");
// Test Objective-C symbols
Data.emplace_back("\01-[C externalFoo:]", Function::ExternalLinkage,
"-[C externalFoo:]");
Data.emplace_back("\01-[C internalFoo:]", Function::InternalLinkage,
@ -611,8 +599,6 @@ TEST_F(InstrProfTest, test_pgo_function_name) {
TEST_F(InstrProfTest, test_irpgo_read_deprecated_names) {
LLVMContext Ctx;
auto M = std::make_unique<Module>("MyModule.cpp", Ctx);
// Use Mach-O mangling so that non-private symbols get a `_` prefix.
M->setDataLayout(DataLayout("m:o"));
auto *FTy = FunctionType::get(Type::getVoidTy(Ctx), /*isVarArg=*/false);
auto *InternalFooF =
Function::Create(FTy, Function::InternalLinkage, "InternalFoo", M.get());