[llvm] [AArch64](NFC) Introduce unified `isLegalArithImmed()` and `isLegalCmpImmed()` (PR #203020)
Nathan Corbyn via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 12 03:14:04 PDT 2026
https://github.com/cofibrant updated https://github.com/llvm/llvm-project/pull/203020
>From ea97b4a1eb5e1d4bea0892d1c5194f21b096b526 Mon Sep 17 00:00:00 2001
From: Nathan Corbyn <n_corbyn at apple.com>
Date: Wed, 10 Jun 2026 16:30:39 +0100
Subject: [PATCH 1/4] [AArch64](NFC) Introduce unified `isLegalArithImmed()`
and `isLegalCmpImmed()`
---
.../Target/AArch64/AArch64ISelDAGToDAG.cpp | 11 +++---
.../Target/AArch64/AArch64ISelLowering.cpp | 36 +++++++------------
.../AArch64/GISel/AArch64GlobalISelUtils.h | 6 ----
.../GISel/AArch64PostLegalizerLowering.cpp | 12 ++-----
.../MCTargetDesc/AArch64AddressingModes.h | 22 ++++++++++++
5 files changed, 41 insertions(+), 46 deletions(-)
diff --git a/llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp b/llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
index 499bb2325186d..9c454349dc12d 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
@@ -738,16 +738,13 @@ bool AArch64DAGToDAGISel::SelectArithImmed(SDValue N, SDValue &Val,
return false;
uint64_t Immed = N.getNode()->getAsZExtVal();
- unsigned ShiftAmt;
- if (Immed >> 12 == 0) {
- ShiftAmt = 0;
- } else if ((Immed & 0xfff) == 0 && Immed >> 24 == 0) {
- ShiftAmt = 12;
- Immed = Immed >> 12;
- } else
+ if (!AArch64_AM::isLegalArithImmed(Immed))
return false;
+ unsigned ShiftAmt = AArch64_AM::getArithImmedShift(Immed);
+ Immed >>= ShiftAmt;
+
unsigned ShVal = AArch64_AM::getShifterImm(AArch64_AM::LSL, ShiftAmt);
SDLoc dl(N);
Val = CurDAG->getTargetConstant(Immed, dl, MVT::i32);
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 57a2d73e00f57..c8f16f1503aa9 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -3750,20 +3750,6 @@ static SDValue getCondCode(SelectionDAG &DAG, AArch64CC::CondCode CC) {
return DAG.getConstant(CC, SDLoc(), CondCodeVT);
}
-static bool isLegalArithImmed(uint64_t C) {
- // Matches AArch64DAGToDAGISel::SelectArithImmed().
- bool IsLegal = (C >> 12 == 0) || ((C & 0xFFFULL) == 0 && C >> 24 == 0);
- LLVM_DEBUG(dbgs() << "Is imm " << C
- << " legal: " << (IsLegal ? "yes\n" : "no\n"));
- return IsLegal;
-}
-
-bool isLegalCmpImmed(const APInt &C) {
- // Works for negative immediates too, as it can be written as an ADDS
- // instruction with a negated immediate.
- return isLegalArithImmed(C.abs().getZExtValue());
-}
-
unsigned numberOfInstrToLoadImm(const APInt &C) {
uint64_t Imm = C.getZExtValue();
SmallVector<AArch64_IMM::ImmInsnModel> Insn;
@@ -4260,7 +4246,7 @@ static SDValue getAArch64Cmp(SDValue LHS, SDValue RHS, ISD::CondCode CC,
// Adjust the constant to zero.
// CC has already been adjusted.
RHS = DAG.getConstant(0, DL, VT);
- } else if (!isLegalCmpImmed(C)) {
+ } else if (!AArch64_AM::isLegalCmpImmed(C)) {
unsigned NumImmForC = numberOfInstrToLoadImm(C);
// Constant does not fit, try adjusting it by one?
switch (CC) {
@@ -4270,7 +4256,7 @@ static SDValue getAArch64Cmp(SDValue LHS, SDValue RHS, ISD::CondCode CC,
case ISD::SETGE:
if (!C.isMinSignedValue()) {
APInt CMinusOne = C - 1;
- if (isLegalCmpImmed(CMinusOne) ||
+ if (AArch64_AM::isLegalCmpImmed(CMinusOne) ||
(NumImmForC > numberOfInstrToLoadImm(CMinusOne))) {
CC = (CC == ISD::SETLT) ? ISD::SETLE : ISD::SETGT;
RHS = DAG.getConstant(CMinusOne, DL, VT);
@@ -4282,7 +4268,7 @@ static SDValue getAArch64Cmp(SDValue LHS, SDValue RHS, ISD::CondCode CC,
// C is not 0 because it is a legal immediate.
assert(!C.isZero() && "C should not be zero here");
APInt CMinusOne = C - 1;
- if (isLegalCmpImmed(CMinusOne) ||
+ if (AArch64_AM::isLegalCmpImmed(CMinusOne) ||
(NumImmForC > numberOfInstrToLoadImm(CMinusOne))) {
CC = (CC == ISD::SETULT) ? ISD::SETULE : ISD::SETUGT;
RHS = DAG.getConstant(CMinusOne, DL, VT);
@@ -4293,7 +4279,7 @@ static SDValue getAArch64Cmp(SDValue LHS, SDValue RHS, ISD::CondCode CC,
case ISD::SETGT:
if (!C.isMaxSignedValue()) {
APInt CPlusOne = C + 1;
- if (isLegalCmpImmed(CPlusOne) ||
+ if (AArch64_AM::isLegalCmpImmed(CPlusOne) ||
(NumImmForC > numberOfInstrToLoadImm(CPlusOne))) {
CC = (CC == ISD::SETLE) ? ISD::SETLT : ISD::SETGE;
RHS = DAG.getConstant(CPlusOne, DL, VT);
@@ -4304,7 +4290,7 @@ static SDValue getAArch64Cmp(SDValue LHS, SDValue RHS, ISD::CondCode CC,
case ISD::SETUGT:
if (!C.isAllOnes()) {
APInt CPlusOne = C + 1;
- if (isLegalCmpImmed(CPlusOne) ||
+ if (AArch64_AM::isLegalCmpImmed(CPlusOne) ||
(NumImmForC > numberOfInstrToLoadImm(CPlusOne))) {
CC = (CC == ISD::SETULE) ? ISD::SETULT : ISD::SETUGE;
RHS = DAG.getConstant(CPlusOne, DL, VT);
@@ -4329,7 +4315,8 @@ static SDValue getAArch64Cmp(SDValue LHS, SDValue RHS, ISD::CondCode CC,
// cmp w13, w12
// can be turned into:
// cmp w12, w11, lsl #1
- if (!isa<ConstantSDNode>(RHS) || !isLegalCmpImmed(RHS->getAsAPIntVal())) {
+ if (!isa<ConstantSDNode>(RHS) ||
+ !AArch64_AM::isLegalCmpImmed(RHS->getAsAPIntVal())) {
bool LHSIsCMN = isCMN(LHS, CC, DAG);
bool RHSIsCMN = isCMN(RHS, CC, DAG);
SDValue TheLHS = LHSIsCMN ? LHS.getOperand(1) : LHS;
@@ -4368,7 +4355,7 @@ static SDValue getAArch64Cmp(SDValue LHS, SDValue RHS, ISD::CondCode CC,
cast<LoadSDNode>(LHS)->getMemoryVT() == MVT::i16 &&
LHS.getNode()->hasNUsesOfValue(1, 0)) {
int16_t ValueofRHS = RHS->getAsZExtVal();
- if (ValueofRHS < 0 && isLegalArithImmed(-ValueofRHS)) {
+ if (ValueofRHS < 0 && AArch64_AM::isLegalArithImmed(-ValueofRHS)) {
SDValue SExt =
DAG.getNode(ISD::SIGN_EXTEND_INREG, DL, LHS.getValueType(), LHS,
DAG.getValueType(MVT::i16));
@@ -19312,7 +19299,7 @@ bool AArch64TargetLowering::isLegalAddImmediate(int64_t Immed) const {
return false;
}
// Same encoding for add/sub, just flip the sign.
- return isLegalArithImmed((uint64_t)std::abs(Immed));
+ return AArch64_AM::isLegalArithImmed((uint64_t)std::abs(Immed));
}
bool AArch64TargetLowering::isLegalAddScalableImmediate(int64_t Imm) const {
@@ -23505,7 +23492,7 @@ static SDValue performSubWithBorrowCombine(SDNode *N, SelectionDAG &DAG) {
// Skip when the inner SUB can't be folded and the swap would cost a mov.
auto *RHSC = dyn_cast<ConstantSDNode>(Flags.getOperand(1));
if ((!CanFoldSub || !N0.hasOneUse()) && RHSC &&
- isLegalCmpImmed(RHSC->getAPIntValue()))
+ AArch64_AM::isLegalCmpImmed(RHSC->getAPIntValue()))
return SDValue();
Flags = DAG.getNode(AArch64ISD::SUBS, SDLoc(Flags), Flags->getVTList(),
Flags.getOperand(1), Flags.getOperand(0))
@@ -27272,7 +27259,8 @@ SDValue performCONDCombine(SDNode *N,
APInt M = AndNode->getConstantOperandAPInt(1);
APInt C = SubsNode->getConstantOperandAPInt(1);
- if (M.isMask() && C.isSubsetOf(M) && !isLegalArithImmed(C.getZExtValue())) {
+ if (M.isMask() && C.isSubsetOf(M) &&
+ !AArch64_AM::isLegalArithImmed(C.getZExtValue())) {
SDLoc DL(SubsNode);
EVT VT = SubsNode->getValueType(0);
unsigned ShiftAmt = M.countl_zero();
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64GlobalISelUtils.h b/llvm/lib/Target/AArch64/GISel/AArch64GlobalISelUtils.h
index 1d28f9f0d47e7..fdb7524fb0ed6 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64GlobalISelUtils.h
+++ b/llvm/lib/Target/AArch64/GISel/AArch64GlobalISelUtils.h
@@ -23,12 +23,6 @@ namespace llvm {
namespace AArch64GISelUtils {
-/// \returns true if \p C is a legal immediate operand for an arithmetic
-/// instruction.
-constexpr bool isLegalArithImmed(const uint64_t C) {
- return (C >> 12 == 0) || ((C & 0xFFFULL) == 0 && C >> 24 == 0);
-}
-
/// \returns A value when \p MI is a vector splat of a Register or constant.
/// Checks for generic opcodes and AArch64-specific generic opcodes.
std::optional<RegOrConstant>
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerLowering.cpp b/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerLowering.cpp
index c129266f016d4..38ca5cad5a795 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerLowering.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerLowering.cpp
@@ -578,12 +578,6 @@ void applyVAshrLshrImm(MachineInstr &MI, MachineRegisterInfo &MRI,
MI.eraseFromParent();
}
-bool isLegalCmpImmed(const APInt &C) {
- // Works for negative immediates too, as it can be written as an ADDS
- // instruction with a negated immediate.
- return isLegalArithImmed(C.abs().getZExtValue());
-}
-
/// Determine whether an integer G_ICMP against 1 or -1 can compare
/// against 0 instead.
///
@@ -656,7 +650,7 @@ tryAdjustICmpImmAndPred(Register LHS, Register RHS, CmpInst::Predicate P,
if (shouldBeAdjustedToZero(LHS, C, P, MRI))
return {{0, P}};
- if (isLegalCmpImmed(C))
+ if (AArch64_AM::isLegalCmpImmed(C))
return std::nullopt;
uint64_t OriginalC = C.getZExtValue();
@@ -722,7 +716,7 @@ tryAdjustICmpImmAndPred(Register LHS, Register RHS, CmpInst::Predicate P,
// Check if the new constant is valid, and return the updated constant and
// predicate if it is.
uint64_t NewC = C.getZExtValue();
- if (isLegalCmpImmed(C))
+ if (AArch64_AM::isLegalCmpImmed(C))
return {{NewC, P}};
auto NumberOfInstrToLoadImm = [=](uint64_t Imm) {
@@ -992,7 +986,7 @@ bool trySwapICmpOperands(MachineInstr &MI, MachineRegisterInfo &MRI) {
// immediate, because we know we can fold that.
Register RHS = MI.getOperand(3).getReg();
auto RHSCst = getIConstantVRegValWithLookThrough(RHS, MRI);
- if (RHSCst && isLegalCmpImmed(RHSCst->Value))
+ if (RHSCst && AArch64_AM::isLegalCmpImmed(RHSCst->Value))
return false;
Register LHS = MI.getOperand(2).getReg();
diff --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h
index 0492fdf3442c1..41216056ca2fb 100644
--- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h
+++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h
@@ -350,6 +350,28 @@ static inline bool isValidDecodeLogicalImmediate(uint64_t val,
return true;
}
+// isLegalArithImmed - \returns true if \p C is a legal immediate operand for an
+// arithmetic instruction.
+static inline bool isLegalArithImmed(const uint64_t C) {
+ return (C >> 12 == 0) || ((C & 0xFFFULL) == 0 && C >> 24 == 0);
+}
+
+// getArithImmedShift - assumes \p C is a legal immediate for arithmetic
+// instructions and \returns the required shift for this immediate.
+static inline unsigned getArithImmedShift(const uint64_t C) {
+ assert(isLegalArithImmed(C) &&
+ "Try to get shift amount for illegal immediate");
+ return C >> 12 == 0 ? 0 : 12;
+}
+
+// isLegalCmpImmed - \returns true if \p C is a legal immediate operand for a
+// comparison instruction.
+static inline bool isLegalCmpImmed(const APInt &C) {
+ // Works for negative immediates too, as it can be written as an ADDS
+ // instruction with a negated immediate.
+ return isLegalArithImmed(C.abs().getZExtValue());
+}
+
//===----------------------------------------------------------------------===//
// Floating-point Immediates
//
>From 0936ba2507703ecf30f0583913c33491a32fd64f Mon Sep 17 00:00:00 2001
From: Nathan Corbyn <n_corbyn at apple.com>
Date: Wed, 10 Jun 2026 16:38:37 +0100
Subject: [PATCH 2/4] Update assert message
---
llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h
index 41216056ca2fb..6d4e071ece5b8 100644
--- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h
+++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h
@@ -360,7 +360,7 @@ static inline bool isLegalArithImmed(const uint64_t C) {
// instructions and \returns the required shift for this immediate.
static inline unsigned getArithImmedShift(const uint64_t C) {
assert(isLegalArithImmed(C) &&
- "Try to get shift amount for illegal immediate");
+ "Tried to get the shift amount for an illegal immediate");
return C >> 12 == 0 ? 0 : 12;
}
>From 604f9c83d80b5c1f7b4debb5ec63e6a8e074d5d9 Mon Sep 17 00:00:00 2001
From: Nathan Corbyn <n_corbyn at apple.com>
Date: Wed, 10 Jun 2026 16:42:07 +0100
Subject: [PATCH 3/4] Use `constexpr` where possible
---
llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h
index 6d4e071ece5b8..9184f5d7b3e9a 100644
--- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h
+++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h
@@ -352,13 +352,13 @@ static inline bool isValidDecodeLogicalImmediate(uint64_t val,
// isLegalArithImmed - \returns true if \p C is a legal immediate operand for an
// arithmetic instruction.
-static inline bool isLegalArithImmed(const uint64_t C) {
+constexpr bool isLegalArithImmed(const uint64_t C) {
return (C >> 12 == 0) || ((C & 0xFFFULL) == 0 && C >> 24 == 0);
}
// getArithImmedShift - assumes \p C is a legal immediate for arithmetic
// instructions and \returns the required shift for this immediate.
-static inline unsigned getArithImmedShift(const uint64_t C) {
+constexpr unsigned getArithImmedShift(const uint64_t C) {
assert(isLegalArithImmed(C) &&
"Tried to get the shift amount for an illegal immediate");
return C >> 12 == 0 ? 0 : 12;
>From 40f49aeaf52a972828426e87957c61c36b84dc83 Mon Sep 17 00:00:00 2001
From: Nathan Corbyn <n_corbyn at apple.com>
Date: Fri, 12 Jun 2026 11:13:47 +0100
Subject: [PATCH 4/4] `//` to `///`
---
.../AArch64/MCTargetDesc/AArch64AddressingModes.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h
index 9184f5d7b3e9a..30ef2b9f25fcd 100644
--- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h
+++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h
@@ -350,22 +350,22 @@ static inline bool isValidDecodeLogicalImmediate(uint64_t val,
return true;
}
-// isLegalArithImmed - \returns true if \p C is a legal immediate operand for an
-// arithmetic instruction.
+/// isLegalArithImmed - \returns true if \p C is a legal immediate operand for
+/// an arithmetic instruction.
constexpr bool isLegalArithImmed(const uint64_t C) {
return (C >> 12 == 0) || ((C & 0xFFFULL) == 0 && C >> 24 == 0);
}
-// getArithImmedShift - assumes \p C is a legal immediate for arithmetic
-// instructions and \returns the required shift for this immediate.
+/// getArithImmedShift - assumes \p C is a legal immediate for arithmetic
+/// instructions and \returns the required shift for this immediate.
constexpr unsigned getArithImmedShift(const uint64_t C) {
assert(isLegalArithImmed(C) &&
"Tried to get the shift amount for an illegal immediate");
return C >> 12 == 0 ? 0 : 12;
}
-// isLegalCmpImmed - \returns true if \p C is a legal immediate operand for a
-// comparison instruction.
+/// isLegalCmpImmed - \returns true if \p C is a legal immediate operand for a
+/// comparison instruction.
static inline bool isLegalCmpImmed(const APInt &C) {
// Works for negative immediates too, as it can be written as an ADDS
// instruction with a negated immediate.
More information about the llvm-commits
mailing list