Skip to content

Commit a94a3cd

Browse files
authored
Always check the function attribute to determine checksum mismatch for available_externally functions (#87279)
This is to fix an assertion error. Apparently, `pseudo_probe_desc` could still be available for import functions, and its checksum mismatch state can be different from import function's `profile-checksum-mismatch` attr. This happens 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. During link time deduplication, the internal function definition (the checksum in desc is computed based on) is substituted by the `available_externally` definition, which cause the inconsistency. Hence, we fix it to by always checking the state for the new `available_externally` definition, which is saved in the function attribute.
1 parent d5ec49f commit a94a3cd

File tree

2 files changed

+25
-10
lines changed

2 files changed

+25
-10
lines changed

llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,16 +129,28 @@ class PseudoProbeManager {
129129

130130
bool profileIsValid(const Function &F, const FunctionSamples &Samples) const {
131131
const auto *Desc = getDesc(F);
132-
assert((LTOPhase != ThinOrFullLTOPhase::ThinLTOPostLink || !Desc ||
132+
bool IsAvailableExternallyLinkage =
133+
GlobalValue::isAvailableExternallyLinkage(F.getLinkage());
134+
// Always check the function attribute to determine checksum mismatch for
135+
// `available_externally` functions even if their desc are available. This
136+
// is because the desc is computed based on the original internal function
137+
// and it's substituted by the `available_externally` function during link
138+
// time. However, when unstable IR or ODR violation issue occurs, the
139+
// definitions of the same function across different translation units could
140+
// be different and result in different checksums. So we should use the
141+
// state from the new (available_externally) function, which is saved in its
142+
// attribute.
143+
assert((LTOPhase != ThinOrFullLTOPhase::ThinLTOPostLink ||
144+
IsAvailableExternallyLinkage || !Desc ||
133145
profileIsHashMismatched(*Desc, Samples) ==
134146
F.hasFnAttribute("profile-checksum-mismatch")) &&
135-
"In post-link, profile checksum matching state doesn't match "
136-
"function 'profile-checksum-mismatch' attribute.");
147+
"In post-link, profile checksum matching state doesn't match the "
148+
"internal function's 'profile-checksum-mismatch' attribute.");
137149
(void)LTOPhase;
138-
// The desc for import function is unavailable. Check the function attribute
139-
// for mismatch.
140-
return (!Desc && !F.hasFnAttribute("profile-checksum-mismatch")) ||
141-
(Desc && !profileIsHashMismatched(*Desc, Samples));
150+
if (IsAvailableExternallyLinkage || !Desc)
151+
return !F.hasFnAttribute("profile-checksum-mismatch");
152+
153+
return Desc && !profileIsHashMismatched(*Desc, Samples);
142154
}
143155
};
144156

llvm/test/Transforms/SampleProfile/pseudo-probe-callee-profile-mismatch.ll

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
; REQUIRES: x86_64-linux
22
; REQUIRES: asserts
3-
; 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
3+
; 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
44

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

68
; CHECK: Run stale profile matching for bar
79
; CHECK: Callsite with callee:baz is matched from 4 to 2
@@ -14,7 +16,7 @@
1416
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"
1517
target triple = "x86_64-unknown-linux-gnu"
1618

17-
define i32 @main() #0 {
19+
define available_externally i32 @main() #0 {
1820
%1 = call i32 @bar(), !dbg !13
1921
ret i32 0
2022
}
@@ -47,7 +49,8 @@ attributes #1 = { "profile-checksum-mismatch" "use-sample-profile" }
4749
!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)
4850
!10 = !DIFile(filename: "test2.c", directory: "/home/test", checksumkind: CSK_MD5, checksum: "553093afc026f9c73562eb3b0c5b7532")
4951
!11 = !{i32 2, !"Debug Info Version", i32 3}
50-
!12 = !{i64 -2624081020897602054, i64 281582081721716, !"main"}
52+
; Make a checksum mismatch in the pseudo_probe_desc
53+
!12 = !{i64 -2624081020897602054, i64 123456, !"main"}
5154
!13 = !DILocation(line: 8, column: 10, scope: !14)
5255
!14 = !DILexicalBlockFile(scope: !15, file: !1, discriminator: 186646591)
5356
!15 = distinct !DILexicalBlock(scope: !16, file: !1, line: 7, column: 40)

0 commit comments

Comments
 (0)