ARM: Start moving runtime libcall configuration out of TargetLowering #142617
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
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-backend-arm Author: Matt Arsenault (arsenm) ChangesThese Module level triple checks implemented in the Subtarget are kind of Full diff: https://github.com/llvm/llvm-project/pull/142617.diff 4 Files Affected:
diff --git a/llvm/include/llvm/TargetParser/Triple.h b/llvm/include/llvm/TargetParser/Triple.h
index 7fd5278f1ed53..351da0d6598c2 100644
--- a/llvm/include/llvm/TargetParser/Triple.h
+++ b/llvm/include/llvm/TargetParser/Triple.h
@@ -917,6 +917,34 @@ class Triple {
isOSBinFormatELF();
}
+ // ARM EABI is the bare-metal EABI described in ARM ABI documents and
+ // can be accessed via -target arm-none-eabi. This is NOT GNUEABI.
+ // FIXME: Add a flag for bare-metal for that target and set Triple::EABI
+ // even for GNUEABI, so we can make a distinction here and still conform to
+ // the EABI on GNU (and Android) mode. This requires change in Clang, too.
+ // FIXME: The Darwin exception is temporary, while we move users to
+ // "*-*-*-macho" triples as quickly as possible.
+ bool isTargetAEABI() const {
+ return (getEnvironment() == Triple::EABI ||
+ getEnvironment() == Triple::EABIHF) &&
+ !isOSDarwin() && !isOSWindows();
+ }
+
+ bool isTargetGNUAEABI() const {
+ return (getEnvironment() == Triple::GNUEABI ||
+ getEnvironment() == Triple::GNUEABIT64 ||
+ getEnvironment() == Triple::GNUEABIHF ||
+ getEnvironment() == Triple::GNUEABIHFT64) &&
+ !isOSDarwin() && !isOSWindows();
+ }
+
+ bool isTargetMuslAEABI() const {
+ return (getEnvironment() == Triple::MuslEABI ||
+ getEnvironment() == Triple::MuslEABIHF ||
+ getEnvironment() == Triple::OpenHOS) &&
+ !isOSDarwin() && !isOSWindows();
+ }
+
/// Tests whether the target is T32.
bool isArmT32() const {
switch (getSubArch()) {
diff --git a/llvm/lib/IR/RuntimeLibcalls.cpp b/llvm/lib/IR/RuntimeLibcalls.cpp
index db0373055160c..30290e97f64d3 100644
--- a/llvm/lib/IR/RuntimeLibcalls.cpp
+++ b/llvm/lib/IR/RuntimeLibcalls.cpp
@@ -11,6 +11,56 @@
using namespace llvm;
using namespace RTLIB;
+static void setARMLibcallNames(RuntimeLibcallsInfo &Info, const Triple &TT) {
+ // Register based DivRem for AEABI (RTABI 4.2)
+ if (TT.isTargetAEABI() || TT.isAndroid() || TT.isTargetGNUAEABI() ||
+ TT.isTargetMuslAEABI() || TT.isOSWindows()) {
+ if (TT.isOSWindows()) {
+ const struct {
+ const RTLIB::Libcall Op;
+ const char *const Name;
+ const CallingConv::ID CC;
+ } LibraryCalls[] = {
+ {RTLIB::SDIVREM_I8, "__rt_sdiv", CallingConv::ARM_AAPCS},
+ {RTLIB::SDIVREM_I16, "__rt_sdiv", CallingConv::ARM_AAPCS},
+ {RTLIB::SDIVREM_I32, "__rt_sdiv", CallingConv::ARM_AAPCS},
+ {RTLIB::SDIVREM_I64, "__rt_sdiv64", CallingConv::ARM_AAPCS},
+
+ {RTLIB::UDIVREM_I8, "__rt_udiv", CallingConv::ARM_AAPCS},
+ {RTLIB::UDIVREM_I16, "__rt_udiv", CallingConv::ARM_AAPCS},
+ {RTLIB::UDIVREM_I32, "__rt_udiv", CallingConv::ARM_AAPCS},
+ {RTLIB::UDIVREM_I64, "__rt_udiv64", CallingConv::ARM_AAPCS},
+ };
+
+ for (const auto &LC : LibraryCalls) {
+ Info.setLibcallName(LC.Op, LC.Name);
+ Info.setLibcallCallingConv(LC.Op, LC.CC);
+ }
+ } else {
+ const struct {
+ const RTLIB::Libcall Op;
+ const char *const Name;
+ const CallingConv::ID CC;
+ } LibraryCalls[] = {
+ {RTLIB::SDIVREM_I8, "__aeabi_idivmod", CallingConv::ARM_AAPCS},
+ {RTLIB::SDIVREM_I16, "__aeabi_idivmod", CallingConv::ARM_AAPCS},
+ {RTLIB::SDIVREM_I32, "__aeabi_idivmod", CallingConv::ARM_AAPCS},
+ {RTLIB::SDIVREM_I64, "__aeabi_ldivmod", CallingConv::ARM_AAPCS},
+
+ {RTLIB::UDIVREM_I8, "__aeabi_uidivmod", CallingConv::ARM_AAPCS},
+ {RTLIB::UDIVREM_I16, "__aeabi_uidivmod", CallingConv::ARM_AAPCS},
+ {RTLIB::UDIVREM_I32, "__aeabi_uidivmod", CallingConv::ARM_AAPCS},
+ {RTLIB::UDIVREM_I64, "__aeabi_uldivmod", CallingConv::ARM_AAPCS},
+ };
+
+ for (const auto &LC : LibraryCalls) {
+ Info.setLibcallName(LC.Op, LC.Name);
+ Info.setLibcallCallingConv(LC.Op, LC.CC);
+ }
+ }
+ }
+}
+
/// Set default libcall names. If a target wants to opt-out of a libcall it
/// should be placed here.
void RuntimeLibcallsInfo::initLibcalls(const Triple &TT) {
@@ -249,6 +299,9 @@ void RuntimeLibcallsInfo::initLibcalls(const Triple &TT) {
}
}
+ if (TT.getArch() == Triple::ArchType::arm)
+ setARMLibcallNames(*this, TT);
+
if (TT.getArch() == Triple::ArchType::avr) {
// Division rtlib functions (not supported), use divmod functions instead
setLibcallName(RTLIB::SDIV_I8, nullptr);
diff --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp
index d156851d7e214..4b01d6fe6edb7 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -1274,50 +1274,6 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
setOperationAction(ISD::UREM, MVT::i64, Custom);
HasStandaloneRem = false;
- if (Subtarget->isTargetWindows()) {
- const struct {
- const RTLIB::Libcall Op;
- const char * const Name;
- const CallingConv::ID CC;
- } LibraryCalls[] = {
- { RTLIB::SDIVREM_I8, "__rt_sdiv", CallingConv::ARM_AAPCS },
- { RTLIB::SDIVREM_I16, "__rt_sdiv", CallingConv::ARM_AAPCS },
- { RTLIB::SDIVREM_I32, "__rt_sdiv", CallingConv::ARM_AAPCS },
- { RTLIB::SDIVREM_I64, "__rt_sdiv64", CallingConv::ARM_AAPCS },
-
- { RTLIB::UDIVREM_I8, "__rt_udiv", CallingConv::ARM_AAPCS },
- { RTLIB::UDIVREM_I16, "__rt_udiv", CallingConv::ARM_AAPCS },
- { RTLIB::UDIVREM_I32, "__rt_udiv", CallingConv::ARM_AAPCS },
- { RTLIB::UDIVREM_I64, "__rt_udiv64", CallingConv::ARM_AAPCS },
- };
-
- for (const auto &LC : LibraryCalls) {
- setLibcallName(LC.Op, LC.Name);
- setLibcallCallingConv(LC.Op, LC.CC);
- }
- } else {
- const struct {
- const RTLIB::Libcall Op;
- const char * const Name;
- const CallingConv::ID CC;
- } LibraryCalls[] = {
- { RTLIB::SDIVREM_I8, "__aeabi_idivmod", CallingConv::ARM_AAPCS },
- { RTLIB::SDIVREM_I16, "__aeabi_idivmod", CallingConv::ARM_AAPCS },
- { RTLIB::SDIVREM_I32, "__aeabi_idivmod", CallingConv::ARM_AAPCS },
- { RTLIB::SDIVREM_I64, "__aeabi_ldivmod", CallingConv::ARM_AAPCS },
-
- { RTLIB::UDIVREM_I8, "__aeabi_uidivmod", CallingConv::ARM_AAPCS },
- { RTLIB::UDIVREM_I16, "__aeabi_uidivmod", CallingConv::ARM_AAPCS },
- { RTLIB::UDIVREM_I32, "__aeabi_uidivmod", CallingConv::ARM_AAPCS },
- { RTLIB::UDIVREM_I64, "__aeabi_uldivmod", CallingConv::ARM_AAPCS },
- };
-
- for (const auto &LC : LibraryCalls) {
- setLibcallName(LC.Op, LC.Name);
- setLibcallCallingConv(LC.Op, LC.CC);
- }
- }
-
setOperationAction(ISD::SDIVREM, MVT::i32, Custom);
setOperationAction(ISD::UDIVREM, MVT::i32, Custom);
setOperationAction(ISD::SDIVREM, MVT::i64, Custom);
diff --git a/llvm/lib/Target/ARM/ARMSubtarget.h b/llvm/lib/Target/ARM/ARMSubtarget.h
index 7329d3f2055f0..890a22f574a6b 100644
--- a/llvm/lib/Target/ARM/ARMSubtarget.h
+++ b/llvm/lib/Target/ARM/ARMSubtarget.h
@@ -348,31 +348,11 @@ class ARMSubtarget : public ARMGenSubtargetInfo {
bool isTargetELF() const { return TargetTriple.isOSBinFormatELF(); }
bool isTargetMachO() const { return TargetTriple.isOSBinFormatMachO(); }
- // ARM EABI is the bare-metal EABI described in ARM ABI documents and
- // can be accessed via -target arm-none-eabi. This is NOT GNUEABI.
- // FIXME: Add a flag for bare-metal for that target and set Triple::EABI
- // even for GNUEABI, so we can make a distinction here and still conform to
- // the EABI on GNU (and Android) mode. This requires change in Clang, too.
- // FIXME: The Darwin exception is temporary, while we move users to
- // "*-*-*-macho" triples as quickly as possible.
- bool isTargetAEABI() const {
- return (TargetTriple.getEnvironment() == Triple::EABI ||
- TargetTriple.getEnvironment() == Triple::EABIHF) &&
- !isTargetDarwin() && !isTargetWindows();
- }
- bool isTargetGNUAEABI() const {
- return (TargetTriple.getEnvironment() == Triple::GNUEABI ||
- TargetTriple.getEnvironment() == Triple::GNUEABIT64 ||
- TargetTriple.getEnvironment() == Triple::GNUEABIHF ||
- TargetTriple.getEnvironment() == Triple::GNUEABIHFT64) &&
- !isTargetDarwin() && !isTargetWindows();
- }
- bool isTargetMuslAEABI() const {
- return (TargetTriple.getEnvironment() == Triple::MuslEABI ||
- TargetTriple.getEnvironment() == Triple::MuslEABIHF ||
- TargetTriple.getEnvironment() == Triple::OpenHOS) &&
- !isTargetDarwin() && !isTargetWindows();
- }
+ bool isTargetAEABI() const { return TargetTriple.isTargetAEABI(); }
+
+ bool isTargetGNUAEABI() const { return TargetTriple.isTargetGNUAEABI(); }
+
+ bool isTargetMuslAEABI() const { return TargetTriple.isTargetMuslAEABI(); }
// ARM Targets that support EHABI exception handling standard
// Darwin uses SjLj. Other targets might need more checks.
|
arsenm
requested review from
aemerson,
davemgreen,
efriedma-quic,
nasherm,
sdesmalen-arm,
sjoerdmeijer,
smithp35 and
stuij
Base automatically changed from
users/arsenm/arm/remove-unused-condcode-table-field
to
main
June 3, 2025 21:02
arsenm
force-pushed
the
users/arsenm/arm/start-moving-runtime-libcall-names-out-of-targetlowering
branch
2 times, most recently
from
2087b4d to
8335786
Compare
These Module level triple checks implemented in the Subtarget are kind of a pain and we should probably get rid of all of them in across all targets, and stick to a consistent set of names.
arsenm
force-pushed
the
users/arsenm/arm/start-moving-runtime-libcall-names-out-of-targetlowering
branch
from
8335786 to
b036500
Compare
| // Register based DivRem for AEABI (RTABI 4.2) | ||
| if (TT.isTargetAEABI() || TT.isAndroid() || TT.isTargetGNUAEABI() || | ||
| TT.isTargetMuslAEABI() || TT.isOSWindows()) { | ||
| if (TT.isOSWindows()) { |
There was a problem hiding this comment.
Can we reduce the nesting here? if (TT.isOSWindows()) {} else if (TT.isTargetAEABI() || TT.isAndroid() || ...
There was a problem hiding this comment.
I have most of a patch to replace this with tablegenerated code so I don't want to put too much effort into making it nice
There was a problem hiding this comment.
I guess this is fine, then.
efriedma-quic
approved these changes
Jun 10, 2025
| // Register based DivRem for AEABI (RTABI 4.2) | ||
| if (TT.isTargetAEABI() || TT.isAndroid() || TT.isTargetGNUAEABI() || | ||
| TT.isTargetMuslAEABI() || TT.isOSWindows()) { | ||
| if (TT.isOSWindows()) { |
There was a problem hiding this comment.
I guess this is fine, then.
arsenm
deleted the
users/arsenm/arm/start-moving-runtime-libcall-names-out-of-targetlowering
branch
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/33/builds/17969 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/94/builds/7904 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/153/builds/34298 Here is the relevant piece of the build log for the reference |
|
@arsenm This broke a couple buildbots and premerge CI it appears. Are you able to revert/fix forward? Premerge CI for this PR: https://github.com/llvm/llvm-project/actions/runs/15535232854 Postmerge buildbot: https://lab.llvm.org/buildbot/#/builders/153/builds/34298 |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/20492 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/29814 Here is the relevant piece of the build log for the reference |
|
I'm thinking that the test might be invalid given the new behavior. It relies on @mstorsjo @arsenm Please let me know if I'm mistaken here/need to adjust something. |
|
The whole point of doing this is the RuntimeLibcallInfo uses outside of TargetLowering are wrong; they will incorrectly see the default function names which won't actually be used with the target. The test probably needs updating to the correct libcall name |
|
The problem will still occur for other subtarget dependent libcall choices, which is the next problem I'm working on |
I don't think that matters here.
The libcall name in the test is correct, at least as far as I can tell. The I am unsure if the LTO infrastructure that figures out what needs to be imported is doing the correct thing here though. I can readd the test/mark it as XFAIL if this is expected to return to the same behavior at some point. |
Yeah, this is what's happening now - llvm-project/lld/COFF/Driver.cpp Lines 2551 to 2561 in a082f66 __rt_sdiv as a potential libcall, so it gets imported properly before doing LTO.
So the issue that the test exercised no longer exists, at least for these libcalls (perhaps there are other libcalls that can trigger similar issues later?), so I think the right thing to do is to convert this from a negative test to a positive one that tests that this now works as expected. (We do lose test coverage for the error situation though.) I can make a PR for that. |
rorth
pushed a commit
to rorth/llvm-project
that referenced
this pull request
Jun 11, 2025
…llvm#142617) These Module level triple checks implemented in the Subtarget are kind of a pain and we should probably get rid of all of them in across all targets, and stick to a consistent set of names.
DhruvSrivastavaX
pushed a commit
to DhruvSrivastavaX/lldb-for-aix
that referenced
this pull request
Jun 12, 2025
…llvm#142617) These Module level triple checks implemented in the Subtarget are kind of a pain and we should probably get rid of all of them in across all targets, and stick to a consistent set of names.
tomtor
pushed a commit
to tomtor/llvm-project
that referenced
this pull request
Jun 14, 2025
…llvm#142617) These Module level triple checks implemented in the Subtarget are kind of a pain and we should probably get rid of all of them in across all targets, and stick to a consistent set of names.
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.
These Module level triple checks implemented in the Subtarget are kind of
a pain and we should probably get rid of all of them in across all targets,
and stick to a consistent set of names.