ARM: Avoid using isTarget wrappers around Triple predicates

[arsenm]

These are module level properties, and querying them through
a function-level subtarget context is confusing. Plus we don't
need an aliased name. This doesn't avoid all the uses, just the
ones in the TargetLowering constructor.

[arsenm]

• ARM: Avoid using isTarget wrappers around Triple predicates #144705 👈 (View in Graphite)
• RuntimeLibcalls: Pass in FloatABI and EABI type #144691 : 1 other dependent PR (#144692 )
• main

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

[llvmbot]

@llvm/pr-subscribers-backend-arm

Author: Matt Arsenault (arsenm)

Changes

These are module level properties, and querying them through
a function-level subtarget context is confusing. Plus we don't
need an aliased name. This doesn't avoid all the uses, just the
ones in the TargetLowering constructor.

---

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

1 Files Affected:

• (modified) llvm/lib/Target/ARM/ARMISelLowering.cpp (+14-14)

diff --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp
index 500ce9dc364ee..46697e364d2b6 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -508,7 +508,9 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
   setBooleanContents(ZeroOrOneBooleanContent);
   setBooleanVectorContents(ZeroOrNegativeOneBooleanContent);
 
-  if (Subtarget->isTargetMachO()) {
+  const Triple &TT = TM.getTargetTriple();
+
+  if (TT.isOSBinFormatMachO()) {
     // Uses VFP for Thumb libfuncs if available.
     if (Subtarget->isThumb() && Subtarget->hasVFP2Base() &&
         Subtarget->hasARMOps() && !Subtarget->useSoftFloat()) {
@@ -582,8 +584,8 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
 
   // RTLIB
   if (Subtarget->isAAPCS_ABI() &&
-      (Subtarget->isTargetAEABI() || Subtarget->isTargetGNUAEABI() ||
-       Subtarget->isTargetMuslAEABI() || Subtarget->isTargetAndroid())) {
+      (TT.isTargetAEABI() || TT.isTargetGNUAEABI() || TT.isTargetMuslAEABI() ||
+       TT.isAndroid())) {
     // clang-format off
     static const struct {
       const RTLIB::Libcall Op;
@@ -705,7 +707,7 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
   // The half <-> float conversion functions are always soft-float on
   // non-watchos platforms, but are needed for some targets which use a
   // hard-float calling convention by default.
-  if (!Subtarget->isTargetWatchABI()) {
+  if (!TT.isWatchABI()) {
     if (Subtarget->isAAPCS_ABI()) {
       setLibcallCallingConv(RTLIB::FPROUND_F32_F16, CallingConv::ARM_AAPCS);
       setLibcallCallingConv(RTLIB::FPROUND_F64_F16, CallingConv::ARM_AAPCS);
@@ -719,7 +721,7 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
 
   // In EABI, these functions have an __aeabi_ prefix, but in GNUEABI they have
   // a __gnu_ prefix (which is the default).
-  if (Subtarget->isTargetAEABI()) {
+  if (TT.isTargetAEABI()) {
     static const struct {
       const RTLIB::Libcall Op;
       const char * const Name;
@@ -734,7 +736,7 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
       setLibcallName(LC.Op, LC.Name);
       setLibcallCallingConv(LC.Op, LC.CC);
     }
-  } else if (!Subtarget->isTargetMachO()) {
+  } else if (!TT.isOSBinFormatMachO()) {
     setLibcallName(RTLIB::FPROUND_F32_F16, "__gnu_f2h_ieee");
     setLibcallName(RTLIB::FPEXT_F16_F32, "__gnu_h2f_ieee");
   }
@@ -1220,7 +1222,7 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
     setOperationAction(ISD::UDIV,  MVT::i32, LibCall);
   }
 
-  if (Subtarget->isTargetWindows() && !Subtarget->hasDivideInThumbMode()) {
+  if (TT.isOSWindows() && !Subtarget->hasDivideInThumbMode()) {
     setOperationAction(ISD::SDIV, MVT::i32, Custom);
     setOperationAction(ISD::UDIV, MVT::i32, Custom);
 
@@ -1232,9 +1234,8 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
   setOperationAction(ISD::UREM,  MVT::i32, Expand);
 
   // Register based DivRem for AEABI (RTABI 4.2)
-  if (Subtarget->isTargetAEABI() || Subtarget->isTargetAndroid() ||
-      Subtarget->isTargetGNUAEABI() || Subtarget->isTargetMuslAEABI() ||
-      Subtarget->isTargetWindows()) {
+  if (TT.isTargetAEABI() || TT.isAndroid() || TT.isTargetGNUAEABI() ||
+      TT.isTargetMuslAEABI() || TT.isOSWindows()) {
     setOperationAction(ISD::SREM, MVT::i64, Custom);
     setOperationAction(ISD::UREM, MVT::i64, Custom);
     HasStandaloneRem = false;
@@ -1264,7 +1265,7 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
   setOperationAction(ISD::STACKSAVE,          MVT::Other, Expand);
   setOperationAction(ISD::STACKRESTORE,       MVT::Other, Expand);
 
-  if (Subtarget->isTargetWindows())
+  if (TT.isOSWindows())
     setOperationAction(ISD::DYNAMIC_STACKALLOC, MVT::i32, Custom);
   else
     setOperationAction(ISD::DYNAMIC_STACKALLOC, MVT::i32, Expand);
@@ -1319,8 +1320,7 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
   }
 
   // Compute supported atomic widths.
-  if (Subtarget->isTargetLinux() ||
-      (!Subtarget->isMClass() && Subtarget->hasV6Ops())) {
+  if (TT.isOSLinux() || (!Subtarget->isMClass() && Subtarget->hasV6Ops())) {
     // For targets where __sync_* routines are reliably available, we use them
     // if necessary.
     //
@@ -1531,7 +1531,7 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
 
   // On MSVC, both 32-bit and 64-bit, ldexpf(f32) is not defined.  MinGW has
   // it, but it's just a wrapper around ldexp.
-  if (Subtarget->isTargetWindows()) {
+  if (TT.isOSWindows()) {
     for (ISD::NodeType Op : {ISD::FLDEXP, ISD::STRICT_FLDEXP, ISD::FFREXP})
       if (isOperationExpand(Op, MVT::f32))
         setOperationAction(Op, MVT::f32, Promote);

[smithp35]

This looks reasonable to me. I can't see a way of changing those properties on a subtarget level with attributes.

Added the ARM backend maintainers to see if they have anything to add.

Would it be worth adding a comment to the function definitions in Subtarget https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/ARM/ARMSubtarget.h#L336

Something similar to the comment about the isCortexA5() in https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/ARM/ARMSubtarget.h#L285

/// These properties are per-module, please use the TargetMachine TargetTriple.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[efriedma-quic]

LGTM