[FMV][GlobalOpt] Do not statically resolve non-FMV callers. #123383
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes a runtime regression in the llvm testsuite: https://lab.llvm.org/buildbot/#/builders/198/builds/1237 On clang-aarch64-sve2-vla: predres FAIL A 'predres' version is unexpectedly trapping on GravitonG4. My explanation is that when the caller in not a versioned function, the compiler exclusively relies on the command line option, or target attribute to deduce whether a feature is available. However, there is no guarantee that in reality the host supports those implied features.
labrinea
changed the title
|
@llvm/pr-subscribers-llvm-transforms Author: Alexandros Lamprineas (labrinea) ChangesThis fixes a runtime regression in the llvm testsuite: https://lab.llvm.org/buildbot/#/builders/198/builds/1237 On clang-aarch64-sve2-vla: predres A 'predres' version is unexpectedly trapping on GravitonG4. My explanation is that when the caller in not a versioned function, the compiler exclusively relies on the command line option, or target attribute to deduce whether a feature is available. However, there is no guarantee that in reality the host supports those implied features. Full diff: /~https://github.com/llvm/llvm-project/pull/123383.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index bf0cacc6224be8..4e9cef97b670da 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -2785,7 +2785,15 @@ static bool OptimizeNonTrivialIFuncs(
} else {
// We can't reason much about non-FMV callers. Just pick the highest
// priority callee if it matches, otherwise bail.
- if (I > 0 || !implies(CallerBits, CalleeBits))
+ //if (I > 0 || !implies(CallerBits, CalleeBits))
+ //
+ // FIXME: This is causing a regression in the llvm test suite,
+ // specifically a 'predres' version is unexpectedly trapping on
+ // GravitonG4. My explanation is that when the caller in not a
+ // versioned function, the compiler exclusively relies on the
+ // command line option, or target attribute to deduce whether a
+ // feature is available. However, there is no guarantee that in
+ // reality the host supports those implied features.
continue;
}
auto &Calls = CallSites[Caller];
diff --git a/llvm/test/Transforms/GlobalOpt/resolve-fmv-ifunc.ll b/llvm/test/Transforms/GlobalOpt/resolve-fmv-ifunc.ll
index 4b6a19d3f05cf5..fa817a8cbf417f 100644
--- a/llvm/test/Transforms/GlobalOpt/resolve-fmv-ifunc.ll
+++ b/llvm/test/Transforms/GlobalOpt/resolve-fmv-ifunc.ll
@@ -221,7 +221,7 @@ resolver_entry:
define i32 @caller4() #8 {
; CHECK-LABEL: define i32 @caller4(
; CHECK-SAME: ) local_unnamed_addr #[[ATTR7:[0-9]+]] {
-; CHECK: [[CALL:%.*]] = tail call i32 @test_non_fmv_caller._Maes()
+; CHECK: [[CALL:%.*]] = tail call i32 @test_non_fmv_caller()
;
entry:
%call = tail call i32 @test_non_fmv_caller()
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
clang format
jroelofs
approved these changes
Jan 17, 2025
labrinea
added a commit
to labrinea/llvm-project
that referenced
this pull request
Jan 21, 2025
Adds `optimize-non-fmv-callers` (disabled by default) as a short term workaround to keep the llvm testsuite bots green, until we decide what is the right solution for the problem which was previously addressed with llvm#123383.
github-actions bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jan 21, 2025
…n-FMV callers. (#123757) Adds `optimize-non-fmv-callers` (disabled by default) as a short term workaround to keep the llvm testsuite bots green, until we decide what is the right solution for the problem which was previously addressed with llvm/llvm-project#123383.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes a runtime regression in the llvm testsuite:
https://lab.llvm.org/buildbot/#/builders/198/builds/1237
On clang-aarch64-sve2-vla:
predres
FAIL
A 'predres' version is unexpectedly trapping on GravitonG4. My explanation is that when the caller in not a versioned function, the compiler exclusively relies on the command line option, or target attribute to deduce whether a feature is available. However, there is no guarantee that in reality the host supports those implied features.
This is a quickfix. We may rather change the mcpu option in the llvm testsuite build instead.