Skip to content

Always check the function attribute to determine checksum mismatch for available_externally functions #87279

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 3, 2024
Merged

Always check the function attribute to determine checksum mismatch for available_externally functions #87279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e87aaf
Always check thhe unction attribute to determine checksum mismatch fo…
wlei-llvm Apr 1, 2024
ce3457f
update comment
wlei-llvm Apr 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,16 +129,28 @@ class PseudoProbeManager {

bool profileIsValid(const Function &F, const FunctionSamples &Samples) const {
const auto *Desc = getDesc(F);
assert((LTOPhase != ThinOrFullLTOPhase::ThinLTOPostLink || !Desc ||
bool IsAvailableExternallyLinkage =
GlobalValue::isAvailableExternallyLinkage(F.getLinkage());
// Always check the function attribute to determine checksum mismatch for
// `available_externally` functions even if their desc are available. This
// is because the desc is computed based on the original internal function
// and it's substituted by the `available_externally` function during link
// time. However, when unstable IR or ODR violation issue occurs, the
// definitions of the same function across different translation units could
// be different and result in different checksums. So we should use the
// state from the new (available_externally) function, which is saved in its
// attribute.
assert((LTOPhase != ThinOrFullLTOPhase::ThinLTOPostLink ||
IsAvailableExternallyLinkage || !Desc ||
profileIsHashMismatched(*Desc, Samples) ==
F.hasFnAttribute("profile-checksum-mismatch")) &&
"In post-link, profile checksum matching state doesn't match "
"function 'profile-checksum-mismatch' attribute.");
"In post-link, profile checksum matching state doesn't match the "
"internal function's 'profile-checksum-mismatch' attribute.");
(void)LTOPhase;
// The desc for import function is unavailable. Check the function attribute
// for mismatch.
return (!Desc && !F.hasFnAttribute("profile-checksum-mismatch")) ||
(Desc && !profileIsHashMismatched(*Desc, Samples));
if (IsAvailableExternallyLinkage || !Desc)
return !F.hasFnAttribute("profile-checksum-mismatch");

return Desc && !profileIsHashMismatched(*Desc, Samples);
}
};

Expand Down
9 changes: 6 additions & 3 deletions llvm/test/Transforms/SampleProfile/pseudo-probe-callee-profile-mismatch.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
; REQUIRES: x86_64-linux
; REQUIRES: asserts
; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/pseudo-probe-callee-profile-mismatch.prof --salvage-stale-profile -S --debug-only=sample-profile,sample-profile-matcher,sample-profile-impl -pass-remarks=inline 2>&1 | FileCheck %s
; RUN: opt < %s -passes='thinlto<O2>' -pgo-kind=pgo-sample-use-pipeline -sample-profile-file=%S/Inputs/pseudo-probe-callee-profile-mismatch.prof --salvage-stale-profile -S --debug-only=sample-profile,sample-profile-matcher,sample-profile-impl -pass-remarks=inline 2>&1 | FileCheck %s

; There is no profile-checksum-mismatch attr, even the checksum is mismatched in the pseudo_probe_desc, it doesn't run the matching.
; CHECK-NOT: Run stale profile matching for main

; CHECK: Run stale profile matching for bar
; CHECK: Callsite with callee:baz is matched from 4 to 2
Expand All @@ -14,7 +16,7 @@
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

define i32 @main() #0 {
define available_externally i32 @main() #0 {
%1 = call i32 @bar(), !dbg !13
ret i32 0
}
Expand Down Expand Up @@ -47,7 +49,8 @@ attributes #1 = { "profile-checksum-mismatch" "use-sample-profile" }
!9 = distinct !DICompileUnit(language: DW_LANG_C11, file: !10, producer: "clang version 19.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
!10 = !DIFile(filename: "test2.c", directory: "/home/test", checksumkind: CSK_MD5, checksum: "553093afc026f9c73562eb3b0c5b7532")
!11 = !{i32 2, !"Debug Info Version", i32 3}
!12 = !{i64 -2624081020897602054, i64 281582081721716, !"main"}
; Make a checksum mismatch in the pseudo_probe_desc
!12 = !{i64 -2624081020897602054, i64 123456, !"main"}
!13 = !DILocation(line: 8, column: 10, scope: !14)
!14 = !DILexicalBlockFile(scope: !15, file: !1, discriminator: 186646591)
!15 = distinct !DILexicalBlock(scope: !16, file: !1, line: 7, column: 40)
Expand Down