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

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Lei Wang (wlei-llvm)

<details>
<summary>Changes</summary>

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.   

---
Full diff: https://github.com/llvm/llvm-project/pull/88035.diff


1 Files Affected:

- (modified) llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h (+16-17) 


``````````diff
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);

``````````

</details>


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