Skip to content

Conversation

@preames
Copy link
Collaborator

@preames preames commented Aug 26, 2025

When choosing to materialize a select of two constants using zicond, we have a choice of which direction to compute the delta. The prior cost was looking only at the cost of the values without accounting for the fact it's actually the delta which is the highest cost and that sometimes the addend can fold into an addi.

…ith zicond

When choosing to materialize a select of two constants using zicond,
we have a choice of which direction to compute the delta.  The prior
cost was looking only at the cost of the values without accounting
for the fact it's actually the delta which is the highest cost and
that sometimes the addend can fold into an addi.

Note that the addi is also preferrable since it might be folded into
a dependent memory instruction if e.g. we're doing a load from a
select of two constant addresses.
@llvmbot
Copy link
Member

llvmbot commented Aug 26, 2025

@llvm/pr-subscribers-backend-risc-v

Author: Philip Reames (preames)

Changes

When choosing to materialize a select of two constants using zicond, we have a choice of which direction to compute the delta. The prior cost was looking only at the cost of the values without accounting for the fact it's actually the delta which is the highest cost and that sometimes the addend can fold into an addi.

Note that the addi is also preferable since it might be folded into a dependent memory instruction if e.g. we're doing a load from a select of two constant addresses.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+15-5)
  • (modified) llvm/test/CodeGen/RISCV/select-const.ll (+6-8)
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index b84bd1ce0ac50..beee74f4d9215 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -9280,11 +9280,21 @@ SDValue RISCVTargetLowering::lowerSELECT(SDValue Op, SelectionDAG &DAG) const {
         }
       }
 
-      const int TrueValCost = RISCVMatInt::getIntMatCost(
-          TrueVal, Subtarget.getXLen(), Subtarget, /*CompressionCost=*/true);
-      const int FalseValCost = RISCVMatInt::getIntMatCost(
-          FalseVal, Subtarget.getXLen(), Subtarget, /*CompressionCost=*/true);
-      bool IsCZERO_NEZ = TrueValCost <= FalseValCost;
+      auto getCost = [&](APInt Delta, APInt Addend) {
+        const int DeltaCost = RISCVMatInt::getIntMatCost(
+            Delta, Subtarget.getXLen(), Subtarget, /*CompressionCost=*/true);
+        // Dos the addend folds into an ADDI
+        if (Addend.isSignedIntN(12))
+          return DeltaCost;
+        const int AddendCost = RISCVMatInt::getIntMatCost(
+            Addend, Subtarget.getXLen(), Subtarget, /*CompressionCost=*/true);
+        // Panalize the ADD slightly so that we prefer to end with an ADDI
+        // if costs are otherwise equal.  This helps to expose the immediate
+        // for possible folding into a dependent memory instruction.
+        return AddendCost + DeltaCost + 1;
+      };
+      bool IsCZERO_NEZ = getCost(FalseVal - TrueVal, TrueVal) <=
+                         getCost(TrueVal - FalseVal, FalseVal);
       SDValue LHSVal = DAG.getConstant(
           IsCZERO_NEZ ? FalseVal - TrueVal : TrueVal - FalseVal, DL, VT);
       SDValue RHSVal =
diff --git a/llvm/test/CodeGen/RISCV/select-const.ll b/llvm/test/CodeGen/RISCV/select-const.ll
index c11b5d9b4a71d..e8019e99a6da9 100644
--- a/llvm/test/CodeGen/RISCV/select-const.ll
+++ b/llvm/test/CodeGen/RISCV/select-const.ll
@@ -506,11 +506,10 @@ define i32 @select_nonnegative_lui_addi(i32 signext %x) {
 ; RV32ZICOND-LABEL: select_nonnegative_lui_addi:
 ; RV32ZICOND:       # %bb.0:
 ; RV32ZICOND-NEXT:    srli a0, a0, 31
-; RV32ZICOND-NEXT:    lui a1, 1048572
-; RV32ZICOND-NEXT:    addi a1, a1, 25
-; RV32ZICOND-NEXT:    czero.eqz a0, a1, a0
 ; RV32ZICOND-NEXT:    lui a1, 4
-; RV32ZICOND-NEXT:    add a0, a0, a1
+; RV32ZICOND-NEXT:    addi a1, a1, -25
+; RV32ZICOND-NEXT:    czero.nez a0, a1, a0
+; RV32ZICOND-NEXT:    addi a0, a0, 25
 ; RV32ZICOND-NEXT:    ret
 ;
 ; RV64I-LABEL: select_nonnegative_lui_addi:
@@ -536,11 +535,10 @@ define i32 @select_nonnegative_lui_addi(i32 signext %x) {
 ; RV64ZICOND-LABEL: select_nonnegative_lui_addi:
 ; RV64ZICOND:       # %bb.0:
 ; RV64ZICOND-NEXT:    srli a0, a0, 63
-; RV64ZICOND-NEXT:    lui a1, 1048572
-; RV64ZICOND-NEXT:    addi a1, a1, 25
-; RV64ZICOND-NEXT:    czero.eqz a0, a1, a0
 ; RV64ZICOND-NEXT:    lui a1, 4
-; RV64ZICOND-NEXT:    add a0, a0, a1
+; RV64ZICOND-NEXT:    addi a1, a1, -25
+; RV64ZICOND-NEXT:    czero.nez a0, a1, a0
+; RV64ZICOND-NEXT:    addi a0, a0, 25
 ; RV64ZICOND-NEXT:    ret
   %cmp = icmp sgt i32 %x, -1
   %cond = select i1 %cmp, i32 16384, i32 25

return DeltaCost;
const int AddendCost = RISCVMatInt::getIntMatCost(
Addend, Subtarget.getXLen(), Subtarget, /*CompressionCost=*/true);
// Panalize the ADD slightly so that we prefer to end with an ADDI
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Panalize the ADD slightly so that we prefer to end with an ADDI
// Penalize the ADD slightly so that we prefer to end with an ADDI

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I moved this bit to a separate PR to reduce the conceptual moving pieces.

@wangpc-pp wangpc-pp requested review from wangpc-pp and removed request for pcwang-thead August 27, 2025 08:35
Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

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

LGTM.

@preames preames merged commit d6e3ade into llvm:main Aug 27, 2025
9 checks passed
@preames preames deleted the pr-zicond-select-of-constants-costing branch August 27, 2025 13:58
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