[RISCV] Prefer (select (x < 0), y, z) -> x >> (XLEN - 1) & (y - z) + z even with Zicond. #125772
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-backend-risc-v Author: Craig Topper (topperc) ChangesThe Zicond version of this requires an li instruction and an Without Zicond we match this in a DAGCombine on RISCVISD::SELECT_CC. Full diff: https://github.com/llvm/llvm-project/pull/125772.diff 2 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 2d2213b420f5a4..1120524d77902d 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -8378,6 +8378,33 @@ SDValue RISCVTargetLowering::lowerSELECT(SDValue Op, SelectionDAG &DAG) const {
if (isa<ConstantSDNode>(TrueV) && isa<ConstantSDNode>(FalseV)) {
const APInt &TrueVal = TrueV->getAsAPIntVal();
const APInt &FalseVal = FalseV->getAsAPIntVal();
+
+ // Prefer these over Zicond to avoid materializing an immediate:
+ // (select (x < 0), y, z) -> x >> (XLEN - 1) & (y - z) + z
+ // (select (x > -1), z, y) -> x >> (XLEN - 1) & (y - z) + z
+ if (CondV.getOpcode() == ISD::SETCC &&
+ CondV.getOperand(0).getValueType() == VT && CondV.hasOneUse()) {
+ ISD::CondCode CCVal = cast<CondCodeSDNode>(CondV.getOperand(2))->get();
+ if ((CCVal == ISD::SETLT && isNullConstant(CondV.getOperand(1))) ||
+ (CCVal == ISD::SETGT && isAllOnesConstant(CondV.getOperand(1)))) {
+ int64_t TrueImm = TrueVal.getSExtValue();
+ int64_t FalseImm = FalseVal.getSExtValue();
+ if (CCVal == ISD::SETGT)
+ std::swap(TrueImm, FalseImm);
+ if (isInt<12>(TrueImm) && isInt<12>(FalseImm) &&
+ isInt<12>(TrueImm - FalseImm)) {
+ SDValue SRA =
+ DAG.getNode(ISD::SRA, DL, VT, CondV.getOperand(0),
+ DAG.getConstant(Subtarget.getXLen() - 1, DL, VT));
+ SDValue AND =
+ DAG.getNode(ISD::AND, DL, VT, SRA,
+ DAG.getSignedConstant(TrueImm - FalseImm, DL, VT));
+ return DAG.getNode(ISD::ADD, DL, VT, AND,
+ DAG.getSignedConstant(FalseImm, DL, VT));
+ }
+ }
+ }
+
const int TrueValCost = RISCVMatInt::getIntMatCost(
TrueVal, Subtarget.getXLen(), Subtarget, /*CompressionCost=*/true);
const int FalseValCost = RISCVMatInt::getIntMatCost(
diff --git a/llvm/test/CodeGen/RISCV/select-const.ll b/llvm/test/CodeGen/RISCV/select-const.ll
index 6a24d03de87497..90a81c535cef29 100644
--- a/llvm/test/CodeGen/RISCV/select-const.ll
+++ b/llvm/test/CodeGen/RISCV/select-const.ll
@@ -3,10 +3,14 @@
; RUN: | FileCheck -check-prefixes=RV32,RV32I %s
; RUN: llc -mtriple=riscv32 -mattr=+f -target-abi=ilp32 -verify-machineinstrs < %s \
; RUN: | FileCheck -check-prefixes=RV32,RV32IF %s
+; RUN: llc -mtriple=riscv32 -mattr=+zicond -target-abi=ilp32 -verify-machineinstrs < %s \
+; RUN: | FileCheck -check-prefixes=RV32,RV32ZICOND %s
; RUN: llc -mtriple=riscv64 -target-abi=lp64 -verify-machineinstrs < %s \
; RUN: | FileCheck -check-prefixes=RV64,RV64I %s
; RUN: llc -mtriple=riscv64 -mattr=+f,+d -target-abi=lp64 -verify-machineinstrs < %s \
; RUN: | FileCheck -check-prefixes=RV64,RV64IFD %s
+; RUN: llc -mtriple=riscv64 -mattr=+zicond -target-abi=lp64 -verify-machineinstrs < %s \
+; RUN: | FileCheck -check-prefixes=RV64,RV64ZICOND %s
;; This tests how good we are at materialising constants using `select`. The aim
;; is that we do so without a branch if possible (at the moment our lowering of
@@ -59,25 +63,59 @@ define signext i32 @select_const_int_pow2_zero(i1 zeroext %a) nounwind {
}
define signext i32 @select_const_int_harder(i1 zeroext %a) nounwind {
-; RV32-LABEL: select_const_int_harder:
-; RV32: # %bb.0:
-; RV32-NEXT: bnez a0, .LBB3_2
-; RV32-NEXT: # %bb.1:
-; RV32-NEXT: li a0, 38
-; RV32-NEXT: ret
-; RV32-NEXT: .LBB3_2:
-; RV32-NEXT: li a0, 6
-; RV32-NEXT: ret
+; RV32I-LABEL: select_const_int_harder:
+; RV32I: # %bb.0:
+; RV32I-NEXT: bnez a0, .LBB3_2
+; RV32I-NEXT: # %bb.1:
+; RV32I-NEXT: li a0, 38
+; RV32I-NEXT: ret
+; RV32I-NEXT: .LBB3_2:
+; RV32I-NEXT: li a0, 6
+; RV32I-NEXT: ret
;
-; RV64-LABEL: select_const_int_harder:
-; RV64: # %bb.0:
-; RV64-NEXT: bnez a0, .LBB3_2
-; RV64-NEXT: # %bb.1:
-; RV64-NEXT: li a0, 38
-; RV64-NEXT: ret
-; RV64-NEXT: .LBB3_2:
-; RV64-NEXT: li a0, 6
-; RV64-NEXT: ret
+; RV32IF-LABEL: select_const_int_harder:
+; RV32IF: # %bb.0:
+; RV32IF-NEXT: bnez a0, .LBB3_2
+; RV32IF-NEXT: # %bb.1:
+; RV32IF-NEXT: li a0, 38
+; RV32IF-NEXT: ret
+; RV32IF-NEXT: .LBB3_2:
+; RV32IF-NEXT: li a0, 6
+; RV32IF-NEXT: ret
+;
+; RV32ZICOND-LABEL: select_const_int_harder:
+; RV32ZICOND: # %bb.0:
+; RV32ZICOND-NEXT: li a1, 32
+; RV32ZICOND-NEXT: czero.nez a0, a1, a0
+; RV32ZICOND-NEXT: addi a0, a0, 6
+; RV32ZICOND-NEXT: ret
+;
+; RV64I-LABEL: select_const_int_harder:
+; RV64I: # %bb.0:
+; RV64I-NEXT: bnez a0, .LBB3_2
+; RV64I-NEXT: # %bb.1:
+; RV64I-NEXT: li a0, 38
+; RV64I-NEXT: ret
+; RV64I-NEXT: .LBB3_2:
+; RV64I-NEXT: li a0, 6
+; RV64I-NEXT: ret
+;
+; RV64IFD-LABEL: select_const_int_harder:
+; RV64IFD: # %bb.0:
+; RV64IFD-NEXT: bnez a0, .LBB3_2
+; RV64IFD-NEXT: # %bb.1:
+; RV64IFD-NEXT: li a0, 38
+; RV64IFD-NEXT: ret
+; RV64IFD-NEXT: .LBB3_2:
+; RV64IFD-NEXT: li a0, 6
+; RV64IFD-NEXT: ret
+;
+; RV64ZICOND-LABEL: select_const_int_harder:
+; RV64ZICOND: # %bb.0:
+; RV64ZICOND-NEXT: li a1, 32
+; RV64ZICOND-NEXT: czero.nez a0, a1, a0
+; RV64ZICOND-NEXT: addi a0, a0, 6
+; RV64ZICOND-NEXT: ret
%1 = select i1 %a, i32 6, i32 38
ret i32 %1
}
@@ -106,6 +144,14 @@ define float @select_const_fp(i1 zeroext %a) nounwind {
; RV32IF-NEXT: fmv.x.w a0, fa5
; RV32IF-NEXT: ret
;
+; RV32ZICOND-LABEL: select_const_fp:
+; RV32ZICOND: # %bb.0:
+; RV32ZICOND-NEXT: lui a1, 1024
+; RV32ZICOND-NEXT: czero.nez a0, a1, a0
+; RV32ZICOND-NEXT: lui a1, 263168
+; RV32ZICOND-NEXT: add a0, a0, a1
+; RV32ZICOND-NEXT: ret
+;
; RV64I-LABEL: select_const_fp:
; RV64I: # %bb.0:
; RV64I-NEXT: mv a1, a0
@@ -128,6 +174,14 @@ define float @select_const_fp(i1 zeroext %a) nounwind {
; RV64IFD-NEXT: fmv.w.x fa5, a0
; RV64IFD-NEXT: fmv.x.w a0, fa5
; RV64IFD-NEXT: ret
+;
+; RV64ZICOND-LABEL: select_const_fp:
+; RV64ZICOND: # %bb.0:
+; RV64ZICOND-NEXT: lui a1, 1024
+; RV64ZICOND-NEXT: czero.nez a0, a1, a0
+; RV64ZICOND-NEXT: lui a1, 263168
+; RV64ZICOND-NEXT: add a0, a0, a1
+; RV64ZICOND-NEXT: ret
%1 = select i1 %a, float 3.0, float 4.0
ret float %1
}
|
dtcxzyw
added a commit
to dtcxzyw/llvm-codegen-benchmark
that referenced
this pull request
Feb 4, 2025
dtcxzyw
approved these changes
Feb 4, 2025
| int64_t FalseImm = FalseVal.getSExtValue(); | ||
| if (CCVal == ISD::SETGT) | ||
| std::swap(TrueImm, FalseImm); | ||
| if (isInt<12>(TrueImm) && isInt<12>(FalseImm) && |
There was a problem hiding this comment.
Do we really need isInt<12>(FalseImm)?
There was a problem hiding this comment.
It's needed to make guarantee the ISD::ADD becomes an ADDI. The one we probably don't need is isInt<12>(TrueImm). It currently protects the TrueImm - FalseImm from signed overflow, but we could change it to an unsigned subtract.
I copied the checks from how it was written in the RISCVISD::SELECT_CC combine.
There should be test changes in the second commit. The first commit contains a new RUN lines with Zicond to an existing test. |
…z even with Zicond. The Zicond version of this requires an li instruction and an additional register. Without Zicond we match this in a DAGCombine on RISCVISD::SELECT_CC.
topperc
force-pushed
the
pr/zicond-constants
branch
from
962c545 to
142f975
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/137/builds/12959 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/185/builds/12775 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/175/builds/12816 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/17534 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/12202 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/56/builds/18085 Here is the relevant piece of the build log for the reference |
topperc
added a commit
that referenced
this pull request
Feb 6, 2025
…(y - z) + z even with Zicond. (#125772)" With the test changes. Original message: The Zicond version of this requires an li instruction and an additional register. Without Zicond we match this in a DAGCombine on RISCVISD::SELECT_CC. This PR has 2 commits. I'll pre-commit the test change if this looks good.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/60/builds/18992 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/9663 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/33/builds/10963 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/22142 Here is the relevant piece of the build log for the reference |
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
…z even with Zicond. (llvm#125772) The Zicond version of this requires an li instruction and an additional register. Without Zicond we match this in a DAGCombine on RISCVISD::SELECT_CC. This PR has 2 commits. I'll pre-commit the test change if this looks good.
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
… - z) + z even with Zicond. (llvm#125772)" This reverts commit ead88c7. I seem to have lost the test updates when rebasing.
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
…(y - z) + z even with Zicond. (llvm#125772)" With the test changes. Original message: The Zicond version of this requires an li instruction and an additional register. Without Zicond we match this in a DAGCombine on RISCVISD::SELECT_CC. This PR has 2 commits. I'll pre-commit the test change if this looks good.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Zicond version of this requires an li instruction and an
additional register.
Without Zicond we match this in a DAGCombine on RISCVISD::SELECT_CC.
This PR has 2 commits. I'll pre-commit the test change if this looks good.