diff options
author | Lei Wang <wlei@fb.com> | 2024-04-03 10:58:17 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-04-03 10:58:17 -0700 |
commit | a94a3cd3d6d4ca6cadaafc29c8097bd2fe078b9d (patch) | |
tree | 86271baa18bb972b7b4117208a094cc162fe8026 | |
parent | d5ec49ff3dc26cdbe350e9cafc6b8e331fff7911 (diff) | |
download | llvm-a94a3cd3d6d4ca6cadaafc29c8097bd2fe078b9d.zip llvm-a94a3cd3d6d4ca6cadaafc29c8097bd2fe078b9d.tar.gz llvm-a94a3cd3d6d4ca6cadaafc29c8097bd2fe078b9d.tar.bz2 |
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.
-rw-r--r-- | llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h | 26 | ||||
-rw-r--r-- | llvm/test/Transforms/SampleProfile/pseudo-probe-callee-profile-mismatch.ll | 9 |
2 files changed, 25 insertions, 10 deletions
diff --git a/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h b/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h index d898ee5..581d354 100644 --- a/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h +++ b/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h @@ -129,16 +129,28 @@ public: 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); } }; diff --git a/llvm/test/Transforms/SampleProfile/pseudo-probe-callee-profile-mismatch.ll b/llvm/test/Transforms/SampleProfile/pseudo-probe-callee-profile-mismatch.ll index 4881937..43be142 100644 --- a/llvm/test/Transforms/SampleProfile/pseudo-probe-callee-profile-mismatch.ll +++ b/llvm/test/Transforms/SampleProfile/pseudo-probe-callee-profile-mismatch.ll @@ -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 @@ -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 } @@ -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) |