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

[IndirectCallPromotion] Proper canonicalize coroutine function (#175606)

Browse Source 
Fix an issue where coroutine function and its await suspend wrappers are
all canonicalized to the same name. This creates duplicate entries in
`MD5FuncMap` (a sorted vector) and may return an incorrect GUID that
mismatches the one from prof metadata and miss ICP. For example, coro
function `foo` and its wrappers `foo.__await_suspend_wrapper__init`,
`foo.__await_suspend_wrapper__final` are all canonicalized to `foo`.
During GUID lookup, any of them can be returned due to unstable sort.
This is more of the reliability issue (the indeterminism) than a
performance issue because hot indirect calls should've been promoted in
sample loader pass.

This also fixes the same naming issue in `CGProfile` where symtab is
created. By the time the pass is run, wrapper functions should already
be inlined but naming collision can happen to the coro function and its
post-split clones (`foo.resume`, `foo.cleanup`).

This change
* Unifies `InstrProfSymtab::getCanonicalName` with
`FunctionSamples::getCanonicalFnName` by providing a customized list of
suffixes to remove instead of removing everything after the first dot
(.) except ".__uniq.". This also addresses the "FIXME" comment.
* Uses stable sort when storing `MD5FuncMap` which avoids indeterminism
during GUID lookup. `MD5FuncMap` can still contain duplicate entries
even with this change, because no check is done before insertion. Making
entries unique needs some additional work.
* Checks for same PGO names before calling the second `addFuncWithName`,
which does nothing in case of name match.
This commit is contained in:
Wei Wang 2026-01-13 23:18:45 -08:00 committed by GitHub
parent 6ddab42952
commit f51eca20cf
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 49 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
llvm/include/llvm/ProfileData/InstrProf.h
View File

@ -506,9 +506,9 @@ class InstrProfSymtab {
public:
using AddrHashMap = std::vector<std::pair<uint64_t, uint64_t>>;
// Returns the canonical name of the given PGOName. In a canonical name, all
// suffixes that begins with "." except ".__uniq." are stripped.
// FIXME: Unify this with `FunctionSamples::getCanonicalFnName`.
// Returns the canonical name of the given PGOName. This shares the same
// logic of FunctionSamples::getCanonicalFnName() but only strips ".llvm."
// and ".part", and leaves out ".__uniq.".
LLVM_ABI static StringRef getCanonicalName(StringRef PGOName);
private:
@ -761,7 +761,7 @@ void InstrProfSymtab::finalizeSymtab() const {
if (Sorted)
return;
llvm::sort(MD5NameMap, less_first());
llvm::sort(MD5FuncMap, less_first());
llvm::stable_sort(MD5FuncMap, less_first());
llvm::sort(AddrToMD5Map, less_first());
AddrToMD5Map.erase(llvm::unique(AddrToMD5Map), AddrToMD5Map.end());
Sorted = true;

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

@ -30,6 +30,7 @@
#include "llvm/IR/ProfDataUtils.h"
#include "llvm/IR/Type.h"
#include "llvm/ProfileData/InstrProfReader.h"
#include "llvm/ProfileData/SampleProf.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Compiler.h"
@ -531,11 +532,14 @@ Error InstrProfSymtab::create(Module &M, bool InLTO, bool AddCanonical) {
// Ignore in this case.
if (!F.hasName())
continue;
if (Error E = addFuncWithName(F, getIRPGOFuncName(F, InLTO), AddCanonical))
auto IRPGOFuncName = getIRPGOFuncName(F, InLTO);
if (Error E = addFuncWithName(F, IRPGOFuncName, AddCanonical))
return E;
// Also use getPGOFuncName() so that we can find records from older profiles
if (Error E = addFuncWithName(F, getPGOFuncName(F, InLTO), AddCanonical))
return E;
auto PGOFuncName = getPGOFuncName(F, InLTO);
if (PGOFuncName != IRPGOFuncName)
if (Error E = addFuncWithName(F, PGOFuncName, AddCanonical))
return E;
}
for (GlobalVariable &G : M.globals()) {
@ -645,22 +649,20 @@ StringRef InstrProfSymtab::getCanonicalName(StringRef PGOName) {
//
// ".__uniq." suffix is used to differentiate internal linkage functions in
// 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);
if (Pos != StringRef::npos)
Pos += UniqSuffix.length();
else
Pos = 0;
// Search '.' after ".__uniq." if ".__uniq." exists, otherwise search '.' from
// the beginning.
Pos = PGOName.find('.', Pos);
if (Pos != StringRef::npos && Pos != 0)
return PGOName.substr(0, Pos);
return PGOName;
// pattern ".xxx" which is kept before matching, other suffixes ".llvm." and
// ".part" will be stripped.
//
// Leverage the common canonicalization logic from FunctionSamples. Instead of
// removing all suffixes except ".__uniq.", explicitly specify the ones to be
// removed. This avoids the issue of colliding the canonical names of
// coroutine function with its await suspend wrappers or with its post-split
// clones. i.e. coro function foo, its wrappers
// (foo.__await_suspend_wrapper__init, and foo.__await_suspend_wrapper__final)
// and its post-split clones (foo.resume, foo.cleanup) are all canonicalized
// to "foo" otherwise, which can make the symtab lookup return unexpected
// result.
const SmallVector<StringRef> SuffixesToRemove{".llvm.", ".part."};
return FunctionSamples::getCanonicalFnName(PGOName, SuffixesToRemove);
}
Error InstrProfSymtab::addFuncWithName(Function &F, StringRef PGOFuncName,

24
llvm/test/Transforms/PGOProfile/indirect_call_promotion_unique.ll
View File

@ -89,13 +89,37 @@ entry:
ret i32 3
}
; IR has coro function "func7" and await suspend wrapper "func7.__await_suspend_wrapper__init",
; Profile only has the coro function name func7.
; func7.__await_suspend_wrapper__init is defined before func7 to be matched first.
; Note: ignore the incomplete function body, we only care about the names for test purpose.
define i32 @func7.__await_suspend_wrapper__init() {
entry:
ret i32 3
}
define i32 @func7() {
entry:
ret i32 3
}
define i32 @bar7() {
entry:
%tmp5 = load ptr, ptr @foo, align 8
; CHECK: icmp eq ptr %tmp5, @func7{{$}}
%call = call i32 %tmp5(), !prof !6
ret i32 %call
}
; GUID of "func1" is -2545542355363006406.
; GUID of "func2" is -4377547752858689819.
; GUID of "func3.__uniq.258901567653530696343884446915951489119" is 8271224222042874235.
; GUID of "func4.__uniq.140291095734751150107370763113257199296" is 1491826207425861106.
; GUID of "func5.__uniq.127882361580787111523790444488985774976" is -4238550483433487304.
; GUID of "func7" is 4143431755475214830.
!1 = !{!"VP", i32 0, i64 1600, i64 -2545542355363006406, i64 1600}
!2 = !{!"VP", i32 0, i64 1600, i64 -4377547752858689819, i64 1600}
!3 = !{!"VP", i32 0, i64 1600, i64 8271224222042874235, i64 1600}
!4 = !{!"VP", i32 0, i64 1600, i64 1491826207425861106, i64 1600}
!5 = !{!"VP", i32 0, i64 1600, i64 -4238550483433487304, i64 1600}
!6 = !{!"VP", i32 0, i64 1600, i64 4143431755475214830, i64 1600}