Skip to content

ARM: Stop using setCmpLibcallCC in MachO case #142710

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Jun 4, 2025

setCmpLibcallCC appears to be a hack to express a non 0/1 return
value difference in the eabi case that's only relevant when
expanding unordered compares. I see no test changes if I remove
this case.

setCmpLibcallCC appears to be a hack to express a non 0/1 return
value difference in the eabi case that's only relevant when
expanding unordered compares. I see no test changes if I remove
this case.
Copy link
Contributor Author

arsenm commented Jun 4, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@arsenm arsenm added the backend:ARM label Jun 4, 2025 — with Graphite App
@llvmbot
Copy link
Member

llvmbot commented Jun 4, 2025

@llvm/pr-subscribers-backend-arm

Author: Matt Arsenault (arsenm)

Changes

setCmpLibcallCC appears to be a hack to express a non 0/1 return
value difference in the eabi case that's only relevant when
expanding unordered compares. I see no test changes if I remove
this case.


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

1 Files Affected:

  • (modified) llvm/lib/Target/ARM/ARMISelLowering.cpp (+55-57)
diff --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp
index d156851d7e214..5ba4bf03270bc 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -523,67 +523,65 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
         Subtarget->hasARMOps() && !Subtarget->useSoftFloat()) {
       static const struct {
         const RTLIB::Libcall Op;
-        const char * const Name;
-        const ISD::CondCode Cond;
+        const char *const Name;
       } LibraryCalls[] = {
-        // Single-precision floating-point arithmetic.
-        { RTLIB::ADD_F32, "__addsf3vfp", ISD::SETCC_INVALID },
-        { RTLIB::SUB_F32, "__subsf3vfp", ISD::SETCC_INVALID },
-        { RTLIB::MUL_F32, "__mulsf3vfp", ISD::SETCC_INVALID },
-        { RTLIB::DIV_F32, "__divsf3vfp", ISD::SETCC_INVALID },
-
-        // Double-precision floating-point arithmetic.
-        { RTLIB::ADD_F64, "__adddf3vfp", ISD::SETCC_INVALID },
-        { RTLIB::SUB_F64, "__subdf3vfp", ISD::SETCC_INVALID },
-        { RTLIB::MUL_F64, "__muldf3vfp", ISD::SETCC_INVALID },
-        { RTLIB::DIV_F64, "__divdf3vfp", ISD::SETCC_INVALID },
-
-        // Single-precision comparisons.
-        { RTLIB::OEQ_F32, "__eqsf2vfp",    ISD::SETNE },
-        { RTLIB::UNE_F32, "__nesf2vfp",    ISD::SETNE },
-        { RTLIB::OLT_F32, "__ltsf2vfp",    ISD::SETNE },
-        { RTLIB::OLE_F32, "__lesf2vfp",    ISD::SETNE },
-        { RTLIB::OGE_F32, "__gesf2vfp",    ISD::SETNE },
-        { RTLIB::OGT_F32, "__gtsf2vfp",    ISD::SETNE },
-        { RTLIB::UO_F32,  "__unordsf2vfp", ISD::SETNE },
-
-        // Double-precision comparisons.
-        { RTLIB::OEQ_F64, "__eqdf2vfp",    ISD::SETNE },
-        { RTLIB::UNE_F64, "__nedf2vfp",    ISD::SETNE },
-        { RTLIB::OLT_F64, "__ltdf2vfp",    ISD::SETNE },
-        { RTLIB::OLE_F64, "__ledf2vfp",    ISD::SETNE },
-        { RTLIB::OGE_F64, "__gedf2vfp",    ISD::SETNE },
-        { RTLIB::OGT_F64, "__gtdf2vfp",    ISD::SETNE },
-        { RTLIB::UO_F64,  "__unorddf2vfp", ISD::SETNE },
-
-        // Floating-point to integer conversions.
-        // i64 conversions are done via library routines even when generating VFP
-        // instructions, so use the same ones.
-        { RTLIB::FPTOSINT_F64_I32, "__fixdfsivfp",    ISD::SETCC_INVALID },
-        { RTLIB::FPTOUINT_F64_I32, "__fixunsdfsivfp", ISD::SETCC_INVALID },
-        { RTLIB::FPTOSINT_F32_I32, "__fixsfsivfp",    ISD::SETCC_INVALID },
-        { RTLIB::FPTOUINT_F32_I32, "__fixunssfsivfp", ISD::SETCC_INVALID },
-
-        // Conversions between floating types.
-        { RTLIB::FPROUND_F64_F32, "__truncdfsf2vfp",  ISD::SETCC_INVALID },
-        { RTLIB::FPEXT_F32_F64,   "__extendsfdf2vfp", ISD::SETCC_INVALID },
-
-        // Integer to floating-point conversions.
-        // i64 conversions are done via library routines even when generating VFP
-        // instructions, so use the same ones.
-        // FIXME: There appears to be some naming inconsistency in ARM libgcc:
-        // e.g., __floatunsidf vs. __floatunssidfvfp.
-        { RTLIB::SINTTOFP_I32_F64, "__floatsidfvfp",    ISD::SETCC_INVALID },
-        { RTLIB::UINTTOFP_I32_F64, "__floatunssidfvfp", ISD::SETCC_INVALID },
-        { RTLIB::SINTTOFP_I32_F32, "__floatsisfvfp",    ISD::SETCC_INVALID },
-        { RTLIB::UINTTOFP_I32_F32, "__floatunssisfvfp", ISD::SETCC_INVALID },
+          // Single-precision floating-point arithmetic.
+          {RTLIB::ADD_F32, "__addsf3vfp"},
+          {RTLIB::SUB_F32, "__subsf3vfp"},
+          {RTLIB::MUL_F32, "__mulsf3vfp"},
+          {RTLIB::DIV_F32, "__divsf3vfp"},
+
+          // Double-precision floating-point arithmetic.
+          {RTLIB::ADD_F64, "__adddf3vfp"},
+          {RTLIB::SUB_F64, "__subdf3vfp"},
+          {RTLIB::MUL_F64, "__muldf3vfp"},
+          {RTLIB::DIV_F64, "__divdf3vfp"},
+
+          // Single-precision comparisons.
+          {RTLIB::OEQ_F32, "__eqsf2vfp"},
+          {RTLIB::UNE_F32, "__nesf2vfp"},
+          {RTLIB::OLT_F32, "__ltsf2vfp"},
+          {RTLIB::OLE_F32, "__lesf2vfp"},
+          {RTLIB::OGE_F32, "__gesf2vfp"},
+          {RTLIB::OGT_F32, "__gtsf2vfp"},
+          {RTLIB::UO_F32, "__unordsf2vfp"},
+
+          // Double-precision comparisons.
+          {RTLIB::OEQ_F64, "__eqdf2vfp"},
+          {RTLIB::UNE_F64, "__nedf2vfp"},
+          {RTLIB::OLT_F64, "__ltdf2vfp"},
+          {RTLIB::OLE_F64, "__ledf2vfp"},
+          {RTLIB::OGE_F64, "__gedf2vfp"},
+          {RTLIB::OGT_F64, "__gtdf2vfp"},
+          {RTLIB::UO_F64, "__unorddf2vfp"},
+
+          // Floating-point to integer conversions.
+          // i64 conversions are done via library routines even when generating
+          // VFP
+          // instructions, so use the same ones.
+          {RTLIB::FPTOSINT_F64_I32, "__fixdfsivfp"},
+          {RTLIB::FPTOUINT_F64_I32, "__fixunsdfsivfp"},
+          {RTLIB::FPTOSINT_F32_I32, "__fixsfsivfp"},
+          {RTLIB::FPTOUINT_F32_I32, "__fixunssfsivfp"},
+
+          // Conversions between floating types.
+          {RTLIB::FPROUND_F64_F32, "__truncdfsf2vfp"},
+          {RTLIB::FPEXT_F32_F64, "__extendsfdf2vfp"},
+
+          // Integer to floating-point conversions.
+          // i64 conversions are done via library routines even when generating
+          // VFP
+          // instructions, so use the same ones.
+          // FIXME: There appears to be some naming inconsistency in ARM libgcc:
+          // e.g., __floatunsidf vs. __floatunssidfvfp.
+          {RTLIB::SINTTOFP_I32_F64, "__floatsidfvfp"},
+          {RTLIB::UINTTOFP_I32_F64, "__floatunssidfvfp"},
+          {RTLIB::SINTTOFP_I32_F32, "__floatsisfvfp"},
+          {RTLIB::UINTTOFP_I32_F32, "__floatunssisfvfp"},
       };
 
-      for (const auto &LC : LibraryCalls) {
+      for (const auto &LC : LibraryCalls)
         setLibcallName(LC.Op, LC.Name);
-        if (LC.Cond != ISD::SETCC_INVALID)
-          setCmpLibcallCC(LC.Op, LC.Cond);
-      }
     }
   }
 

@arsenm arsenm marked this pull request as ready for review June 4, 2025 01:59
@arsenm
Copy link
Contributor Author

arsenm commented Jun 16, 2025

ping

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.

It looks like we don't actually have any test coverage for this codepath. And it's probably not a codepath anyone actually cares about: it's only relevant specifically if you're generating MachO on a target with VFP support and specify either Thumb1-only codegen, or some weird sp-only FPU. (For example, clang --target=armv6-apple-ios5 -mthumb.)

I'd be okay with just deleting this code outright, if nobody from Apple objects.

Dropping the setCmpLibcallCC calls is probably wrong: I think you end up accidentally inheriting the cond-codes from the "standard" gcc-compatible libcalls.

@efriedma-quic efriedma-quic requested a review from TNorthover June 16, 2025 21:59
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.

3 participants