Skip to content

AArch64: Move outline atomic libcalls configuration #144374

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Jun 16, 2025

This de-conditionalizes the setting of the libcall names
on outlineAtomics() && !hasLSE(). The existence of the
libcall is a module level property, which cannot depend on the
subtarget so this is fine. It's better if the initial list of
calls has more entries than will be used than to have missing
ones. There aren't any alternative names set, so this is also
fine.

Currently RuntimeLibcallsInfo conflates the existence of the calls
with the lowering usage decision, so this suboptimally will report
the libcall name on subtargets that should not use the calls. This
doesn't matter in this case though, as the atomic lowering actions
are already separately controlled and aren't based on decisions on
libcall availability. We could be paranoid and clear the names in
TargetLowering.

Also fixes not catching all aarch64 triples in the RuntimeLibcallsInfo
construction; the previous check missed aarch64_be.

Copy link
Contributor Author

arsenm commented Jun 16, 2025

@llvmbot
Copy link
Member

llvmbot commented Jun 16, 2025

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-backend-aarch64

Author: Matt Arsenault (arsenm)

Changes

This de-conditionalizes the setting of the libcall names
on outlineAtomics() && !hasLSE(). The existence of the
libcall is a module level property, which cannot depend on the
subtarget so this is fine. It's better if the initial list of
calls has more entries than will be used than to have missing
ones. There aren't any alternative names set, so this is also
fine.

Currently RuntimeLibcallsInfo conflates the existence of the calls
with the lowering usage decision, so this suboptimally will report
the libcall name on subtargets that should not use the calls. This
doesn't matter in this case though, as the atomic lowering actions
are already separately controlled and aren't based on decisions on
libcall availability. We could be paranoid and clear the names in
TargetLowering.

Also fixes not catching all aarch64 triples in the RuntimeLibcallsInfo
construction; the previous check missed aarch64_be.


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

2 Files Affected:

  • (modified) llvm/lib/IR/RuntimeLibcalls.cpp (+23-1)
  • (modified) llvm/lib/Target/AArch64/AArch64ISelLowering.cpp (-21)
diff --git a/llvm/lib/IR/RuntimeLibcalls.cpp b/llvm/lib/IR/RuntimeLibcalls.cpp
index d63d398e243f9..09200f380305a 100644
--- a/llvm/lib/IR/RuntimeLibcalls.cpp
+++ b/llvm/lib/IR/RuntimeLibcalls.cpp
@@ -18,6 +18,28 @@ static cl::opt<bool>
 
 static void setAArch64LibcallNames(RuntimeLibcallsInfo &Info,
                                    const Triple &TT) {
+#define LCALLNAMES(A, B, N)                                                    \
+  Info.setLibcallName(A##N##_RELAX, #B #N "_relax");                           \
+  Info.setLibcallName(A##N##_ACQ, #B #N "_acq");                               \
+  Info.setLibcallName(A##N##_REL, #B #N "_rel");                               \
+  Info.setLibcallName(A##N##_ACQ_REL, #B #N "_acq_rel");
+#define LCALLNAME4(A, B)                                                       \
+  LCALLNAMES(A, B, 1)                                                          \
+  LCALLNAMES(A, B, 2) LCALLNAMES(A, B, 4) LCALLNAMES(A, B, 8)
+#define LCALLNAME5(A, B)                                                       \
+  LCALLNAMES(A, B, 1)                                                          \
+  LCALLNAMES(A, B, 2)                                                          \
+  LCALLNAMES(A, B, 4) LCALLNAMES(A, B, 8) LCALLNAMES(A, B, 16)
+  LCALLNAME5(RTLIB::OUTLINE_ATOMIC_CAS, __aarch64_cas)
+  LCALLNAME4(RTLIB::OUTLINE_ATOMIC_SWP, __aarch64_swp)
+  LCALLNAME4(RTLIB::OUTLINE_ATOMIC_LDADD, __aarch64_ldadd)
+  LCALLNAME4(RTLIB::OUTLINE_ATOMIC_LDSET, __aarch64_ldset)
+  LCALLNAME4(RTLIB::OUTLINE_ATOMIC_LDCLR, __aarch64_ldclr)
+  LCALLNAME4(RTLIB::OUTLINE_ATOMIC_LDEOR, __aarch64_ldeor)
+#undef LCALLNAMES
+#undef LCALLNAME4
+#undef LCALLNAME5
+
   if (TT.isWindowsArm64EC()) {
     // FIXME: are there calls we need to exclude from this?
 #define HANDLE_LIBCALL(code, name)                                             \
@@ -488,7 +510,7 @@ void RuntimeLibcallsInfo::initLibcalls(const Triple &TT) {
     }
   }
 
-  if (TT.getArch() == Triple::ArchType::aarch64)
+  if (TT.isAArch64())
     setAArch64LibcallNames(*this, TT);
   else if (TT.isARM() || TT.isThumb())
     setARMLibcallNames(*this, TT);
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 7519ac5260a64..335e2ccadb084 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -966,27 +966,6 @@ AArch64TargetLowering::AArch64TargetLowering(const TargetMachine &TM,
     setOperationAction(ISD::ATOMIC_LOAD_XOR, MVT::i16, LibCall);
     setOperationAction(ISD::ATOMIC_LOAD_XOR, MVT::i32, LibCall);
     setOperationAction(ISD::ATOMIC_LOAD_XOR, MVT::i64, LibCall);
-#define LCALLNAMES(A, B, N)                                                    \
-  setLibcallName(A##N##_RELAX, #B #N "_relax");                                \
-  setLibcallName(A##N##_ACQ, #B #N "_acq");                                    \
-  setLibcallName(A##N##_REL, #B #N "_rel");                                    \
-  setLibcallName(A##N##_ACQ_REL, #B #N "_acq_rel");
-#define LCALLNAME4(A, B)                                                       \
-  LCALLNAMES(A, B, 1)                                                          \
-  LCALLNAMES(A, B, 2) LCALLNAMES(A, B, 4) LCALLNAMES(A, B, 8)
-#define LCALLNAME5(A, B)                                                       \
-  LCALLNAMES(A, B, 1)                                                          \
-  LCALLNAMES(A, B, 2)                                                          \
-  LCALLNAMES(A, B, 4) LCALLNAMES(A, B, 8) LCALLNAMES(A, B, 16)
-    LCALLNAME5(RTLIB::OUTLINE_ATOMIC_CAS, __aarch64_cas)
-    LCALLNAME4(RTLIB::OUTLINE_ATOMIC_SWP, __aarch64_swp)
-    LCALLNAME4(RTLIB::OUTLINE_ATOMIC_LDADD, __aarch64_ldadd)
-    LCALLNAME4(RTLIB::OUTLINE_ATOMIC_LDSET, __aarch64_ldset)
-    LCALLNAME4(RTLIB::OUTLINE_ATOMIC_LDCLR, __aarch64_ldclr)
-    LCALLNAME4(RTLIB::OUTLINE_ATOMIC_LDEOR, __aarch64_ldeor)
-#undef LCALLNAMES
-#undef LCALLNAME4
-#undef LCALLNAME5
   }
 
   if (Subtarget->outlineAtomics() && !Subtarget->hasLSFE()) {

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might eventually want some mechanism that scans the module to see if, for example, there are any functions that might call the outlined atomic routines. A linker could use this to minimize the number of objects we pull into the link. But this is fine for now.

Copy link
Contributor Author

arsenm commented Jun 17, 2025

Merge activity

  • Jun 17, 12:42 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jun 17, 12:44 AM UTC: Graphite rebased this pull request as part of a merge.
  • Jun 17, 12:47 AM UTC: Graphite rebased this pull request as part of a merge.
  • Jun 17, 12:49 AM UTC: @arsenm merged this pull request with Graphite.

@arsenm arsenm force-pushed the users/arsenm/aarch64/move-outline-atomic-libcall-setting-runtime-libcalls branch from 6f7d763 to 40c284a Compare June 17, 2025 00:43
This de-conditionalizes the setting of the libcall names
on outlineAtomics() && !hasLSE(). The existence of the
libcall is a module level property, which cannot depend on the
subtarget so this is fine. It's better if the initial list of
calls has more entries than will be used than to have missing
ones. There aren't any alternative names set, so this is also
fine.

Currently RuntimeLibcallsInfo conflates the existence of the calls
with the lowering usage decision, so this suboptimally will report
the libcall name on subtargets that should not use the calls. This
doesn't matter in this case though, as the atomic lowering actions
are already separately controlled and aren't based on decisions on
libcall availability. We could be paranoid and clear the names in
TargetLowering.

Also fixes not catching all aarch64 triples in the RuntimeLibcallsInfo
construction; the previous check missed aarch64_be.
@arsenm arsenm force-pushed the users/arsenm/aarch64/move-outline-atomic-libcall-setting-runtime-libcalls branch from 40c284a to 8629db3 Compare June 17, 2025 00:47
@arsenm arsenm merged commit 9bd234a into main Jun 17, 2025
5 of 7 checks passed
@arsenm arsenm deleted the users/arsenm/aarch64/move-outline-atomic-libcall-setting-runtime-libcalls branch June 17, 2025 00:49
ajaden-codes pushed a commit to Jaddyen/llvm-project that referenced this pull request Jun 17, 2025
This de-conditionalizes the setting of the libcall names
on outlineAtomics() && !hasLSE(). The existence of the
libcall is a module level property, which cannot depend on the
subtarget so this is fine. It's better if the initial list of
calls has more entries than will be used than to have missing
ones. There aren't any alternative names set, so this is also
fine.

Currently RuntimeLibcallsInfo conflates the existence of the calls
with the lowering usage decision, so this suboptimally will report
the libcall name on subtargets that should not use the calls. This
doesn't matter in this case though, as the atomic lowering actions
are already separately controlled and aren't based on decisions on
libcall availability. We could be paranoid and clear the names in
TargetLowering.

Also fixes not catching all aarch64 triples in the RuntimeLibcallsInfo
construction; the previous check missed aarch64_be.
fschlimb pushed a commit to fschlimb/llvm-project that referenced this pull request Jun 18, 2025
This de-conditionalizes the setting of the libcall names
on outlineAtomics() && !hasLSE(). The existence of the
libcall is a module level property, which cannot depend on the
subtarget so this is fine. It's better if the initial list of
calls has more entries than will be used than to have missing
ones. There aren't any alternative names set, so this is also
fine.

Currently RuntimeLibcallsInfo conflates the existence of the calls
with the lowering usage decision, so this suboptimally will report
the libcall name on subtargets that should not use the calls. This
doesn't matter in this case though, as the atomic lowering actions
are already separately controlled and aren't based on decisions on
libcall availability. We could be paranoid and clear the names in
TargetLowering.

Also fixes not catching all aarch64 triples in the RuntimeLibcallsInfo
construction; the previous check missed aarch64_be.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants