ARM: Remove CPU from computeTargetABI #151983
Merged
Conversation
This file contains hidden or 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
The target CPU is a subtarget / function level concept, which should not influence the module level ABI decisions. No tests fail so it appears nothing is relying on this.
|
@llvm/pr-subscribers-backend-arm Author: Matt Arsenault (arsenm) ChangesThe target CPU is a subtarget / function level concept, which Full diff: https://github.com/llvm/llvm-project/pull/151983.diff 3 Files Affected:
diff --git a/llvm/include/llvm/TargetParser/ARMTargetParser.h b/llvm/include/llvm/TargetParser/ARMTargetParser.h
index 3ae6c4956656d..90eae9ef78da6 100644
--- a/llvm/include/llvm/TargetParser/ARMTargetParser.h
+++ b/llvm/include/llvm/TargetParser/ARMTargetParser.h
@@ -270,10 +270,9 @@ LLVM_ABI ProfileKind parseArchProfile(StringRef Arch);
LLVM_ABI unsigned parseArchVersion(StringRef Arch);
LLVM_ABI void fillValidCPUArchList(SmallVectorImpl<StringRef> &Values);
-LLVM_ABI StringRef computeDefaultTargetABI(const Triple &TT, StringRef CPU);
+LLVM_ABI StringRef computeDefaultTargetABI(const Triple &TT);
-LLVM_ABI ARMABI computeTargetABI(const Triple &TT, StringRef CPU,
- StringRef ABIName = "");
+LLVM_ABI ARMABI computeTargetABI(const Triple &TT, StringRef ABIName = "");
/// Get the (LLVM) name of the minimum ARM CPU for the arch we are targeting.
///
diff --git a/llvm/lib/Target/ARM/ARMTargetMachine.cpp b/llvm/lib/Target/ARM/ARMTargetMachine.cpp
index e8d0d35080775..fedf9e2cf34b1 100644
--- a/llvm/lib/Target/ARM/ARMTargetMachine.cpp
+++ b/llvm/lib/Target/ARM/ARMTargetMachine.cpp
@@ -121,10 +121,10 @@ static std::unique_ptr<TargetLoweringObjectFile> createTLOF(const Triple &TT) {
return std::make_unique<ARMElfTargetObjectFile>();
}
-static std::string computeDataLayout(const Triple &TT, StringRef CPU,
+static std::string computeDataLayout(const Triple &TT,
const TargetOptions &Options,
bool isLittle) {
- auto ABI = ARM::computeTargetABI(TT, CPU, Options.MCOptions.ABIName);
+ auto ABI = ARM::computeTargetABI(TT, Options.MCOptions.ABIName);
std::string Ret;
if (isLittle)
@@ -202,11 +202,10 @@ ARMBaseTargetMachine::ARMBaseTargetMachine(const Target &T, const Triple &TT,
std::optional<Reloc::Model> RM,
std::optional<CodeModel::Model> CM,
CodeGenOptLevel OL, bool isLittle)
- : CodeGenTargetMachineImpl(T, computeDataLayout(TT, CPU, Options, isLittle),
- TT, CPU, FS, Options,
- getEffectiveRelocModel(TT, RM),
+ : CodeGenTargetMachineImpl(T, computeDataLayout(TT, Options, isLittle), TT,
+ CPU, FS, Options, getEffectiveRelocModel(TT, RM),
getEffectiveCodeModel(CM, CodeModel::Small), OL),
- TargetABI(ARM::computeTargetABI(TT, CPU, Options.MCOptions.ABIName)),
+ TargetABI(ARM::computeTargetABI(TT, Options.MCOptions.ABIName)),
TLOF(createTLOF(getTargetTriple())), isLittle(isLittle) {
// Default to triple-appropriate float ABI
diff --git a/llvm/lib/TargetParser/ARMTargetParser.cpp b/llvm/lib/TargetParser/ARMTargetParser.cpp
index dcb30b7ba0410..08944e6148a00 100644
--- a/llvm/lib/TargetParser/ARMTargetParser.cpp
+++ b/llvm/lib/TargetParser/ARMTargetParser.cpp
@@ -535,9 +535,8 @@ void ARM::fillValidCPUArchList(SmallVectorImpl<StringRef> &Values) {
}
}
-StringRef ARM::computeDefaultTargetABI(const Triple &TT, StringRef CPU) {
- StringRef ArchName =
- CPU.empty() ? TT.getArchName() : getArchName(parseCPUArch(CPU));
+StringRef ARM::computeDefaultTargetABI(const Triple &TT) {
+ StringRef ArchName = TT.getArchName();
if (TT.isOSBinFormatMachO()) {
if (TT.getEnvironment() == Triple::EABI ||
@@ -575,10 +574,9 @@ StringRef ARM::computeDefaultTargetABI(const Triple &TT, StringRef CPU) {
}
}
-ARM::ARMABI ARM::computeTargetABI(const Triple &TT, StringRef CPU,
- StringRef ABIName) {
+ARM::ARMABI ARM::computeTargetABI(const Triple &TT, StringRef ABIName) {
if (ABIName.empty())
- ABIName = ARM::computeDefaultTargetABI(TT, CPU);
+ ABIName = ARM::computeDefaultTargetABI(TT);
if (ABIName == "aapcs16")
return ARM_ABI_AAPCS16;
|
arsenm
requested review from
aemerson,
davemgreen,
efriedma-quic,
nasherm,
ostannard,
sdesmalen-arm,
sjoerdmeijer,
smithp35 and
stuij
efriedma-quic
approved these changes
Aug 4, 2025
There was a problem hiding this comment.
Various targets use the module-wide "target CPU" for various decisions, including adding attributes to the object file. Saying it "shouldn't" affect module-level decisions doesn't match reality.
That said, in this particular case, the triple should be sufficient. clang canonicalizes the triple: if you try to pass in a triple with the wrong profile, it canonicalizes it to the right architecture. (For example, if you pass --target=arm-none-eabi -mcpu=cortex-m0 to clang, it canonicalizes the triple to thumbv6m-unknown-none-eabi.) I think the CPU check is just an artifact of a bunch of refactoring (starting with 1971c35), and shouldn't actually be necessary.
LGTM
|
There appears to be a spurious error in the CI checks. I'm not even sure where the error is, but all of the actual lit tests passed |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/28284 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/32065 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/207/builds/4771 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/2/builds/30565 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/159/builds/28035 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/181/builds/25280 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/203/builds/18949 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/205/builds/17738 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/134/builds/23861 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/200/builds/14726 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/163/builds/24114 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/24708 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/14130 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/3/builds/20083 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/10858 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/20766 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/140/builds/28299 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/22284 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/154/builds/19812 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/46/builds/21210 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/206/builds/4361 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/108/builds/16153 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/8/builds/19158 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/12809 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/22120 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/210/builds/995 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/95/builds/16683 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/14728 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/23/builds/12831 Here is the relevant piece of the build log for the reference |
|
This is breaking some build bots. Can you revert it? |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/197/builds/7736 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/14925 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/29/builds/15648 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/208/builds/3386 Here is the relevant piece of the build log for the reference |
|
This is causing a build failure in clang |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/107/builds/13368 Here is the relevant piece of the build log for the reference |
bogner
added a commit
to bogner/llvm-project
that referenced
this pull request
Aug 5, 2025
That change was missing the API update in clang.
bogner
added a commit
that referenced
this pull request
Aug 5, 2025
The error in the CI checks was that clang fails to build - from the pre-checkin logs: I've fixed this in #152066 for you. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/35894 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/56/builds/32566 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/60/builds/34741 Here is the relevant piece of the build log for the reference |
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.
The target CPU is a subtarget / function level concept, which
should not influence the module level ABI decisions. No tests fail
so it appears nothing is relying on this.