[llvm] Extend the mismatch attr check to all MayBeDerefined function (PR #88035)

Lei Wang via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 8 13:19:13 PDT 2024


https://github.com/wlei-llvm created https://github.com/llvm/llvm-project/pull/88035

Follow-up fix for #87279 which is not enough, we should apply the mismatch check to all cases that the definition can be replaced by another variant during link time. There is an existing API [isDefinitionExact](https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/IR/GlobalValue.h#L490) exactly used for this purpose, refer to the comments(https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/IR/GlobalValue.h#L134) from `mayBeDerefined`.
So we use this to extend the mismatch attr check.   

>From b1b45a7a2f1f545c0d7112c29702c9bc6153969f Mon Sep 17 00:00:00 2001
From: wlei <wlei at fb.com>
Date: Mon, 8 Apr 2024 11:33:36 -0700
Subject: [PATCH] Extend the mismatch attr check to all MayBeDerefinf function

---
 .../Utils/SampleProfileLoaderBaseImpl.h       | 33 +++++++++----------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h b/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
index 581d354fc4766c..afc91170f9b9f7 100644
--- a/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
+++ b/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
@@ -129,25 +129,24 @@ class PseudoProbeManager {
 
   bool profileIsValid(const Function &F, const FunctionSamples &Samples) const {
     const auto *Desc = getDesc(F);
-    bool IsAvailableExternallyLinkage =
-        GlobalValue::isAvailableExternallyLinkage(F.getLinkage());
+    bool MayBeDerefined = !F.isDefinitionExact();
     // 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 the "
-           "internal function's 'profile-checksum-mismatch' attribute.");
+    // functions whose definition is not exact(MayBeDerefined) even if their
+    // desc are available. This is because the desc is computed based on the
+    // original internal definition. When a derefined case occurs, the
+    // definition of function may be replaced by a differently optimized variant
+    // of the same source level function at link time and this could result in
+    // different checksums. So we should use the state from the new definition,
+    // which is saved in its attribute.
+    assert(
+        (LTOPhase != ThinOrFullLTOPhase::ThinLTOPostLink || MayBeDerefined ||
+         !Desc ||
+         profileIsHashMismatched(*Desc, Samples) ==
+             F.hasFnAttribute("profile-checksum-mismatch")) &&
+        "In post-link, profile checksum matching state doesn't match the exact "
+        "definition function's 'profile-checksum-mismatch' attribute.");
     (void)LTOPhase;
-    if (IsAvailableExternallyLinkage || !Desc)
+    if (MayBeDerefined || !Desc)
       return !F.hasFnAttribute("profile-checksum-mismatch");
 
     return Desc && !profileIsHashMismatched(*Desc, Samples);



More information about the llvm-commits mailing list