[RISCV] Optimize (and (i1) f, (setcc a, b, eq)) to use the zicond extension - #217946
[RISCV] Optimize (and (i1) f, (setcc a, b, eq)) to use the zicond extension#217946afonso360 wants to merge 2 commits into
(and (i1) f, (setcc a, b, eq)) to use the zicond extension#217946Conversation
2974205 to
12b2ae0
Compare
|
Based on previous reviews it looks like @topperc might be the correct person to review this? |
|
Please put new tests in a separate commit within the PR showing the current codegen. Then apply the codegen and test update in a second commit. |
b52fe63 to
73012dd
Compare
|
Updated! Thanks for looking at this |
|
Hello @afonso360 👋 Thank you for submitting a Pull Request (PR) to the LLVM Project. Since this is your first PR, here are a few useful links covering our main contribution policies and review practices.
Please reply to this message to confirm that you have read these policies, especially the LLVM AI Tool Use Policy, and that any AI tool usage has been noted in the PR description. Frequently asked questionsHow do I add reviewers? This PR will be automatically labeled, and the relevant teams will be notified. For some parts of the project, reviewers may also be added automatically. You can also add reviewers manually using the Reviewers section on this page. If you cannot use that section, it is probably because you do not have write permissions for the repository. In that case, you can request a review by tagging reviewers in a comment using What if there are no comments? If you have not received any comments on your PR after a week, you can request a review by pinging the PR with a comment such as “Ping”. The common courtesy ping rate is once a week. Please remember that you are asking for volunteer time from other developers. Are any special GitHub settings required to contribute to LLVM? We only require contributors to have a public email address associated with their GitHub commits, see this section of LLVM Developer Policy for details. If you have questions, feel free to leave a comment on this PR, or ask on LLVM Discord or LLVM Discourse. Thank you, |
|
@llvm/pr-subscribers-backend-risc-v Author: Afonso Bordado (afonso360) ChangesThis is a generalization of the previously existing rule, that required one of the sides of the With this commit we now support comparisons between any two variables. This requires us to insert an additional instruction to compare the two. But it is still beneficial vs the unoptimized lowering as it reduces 1 instruction in the output. This fixes one of the issues identified in #179584. Namely Full diff: https://github.com/llvm/llvm-project/pull/217946.diff 2 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 5bd12f635ac76..309010d2c0424 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -18272,6 +18272,11 @@ static SDValue reverseZExtICmpCombine(SDNode *N, SelectionDAG &DAG,
return DAG.getNode(ISD::ZERO_EXTEND, DL, VT, Res);
}
+// (and (i1) f, (setcc a, b, eq)) -> (czero.nez f, (xor a, b))
+// (and (i1) f, (setcc a, b, ne)) -> (czero.eqz f, (xor a, b))
+// (and (setcc a, b, eq), (i1) g) -> (czero.nez g, (xor a, b))
+// (and (setcc a, b, ne), (i1) g) -> (czero.eqz g, (xor a, b))
+//
// (and (i1) f, (setcc c, 0, ne)) -> (czero.nez f, c)
// (and (i1) f, (setcc c, 0, eq)) -> (czero.eqz f, c)
// (and (setcc c, 0, ne), (i1) g) -> (czero.nez g, c)
@@ -18284,8 +18289,8 @@ static SDValue combineANDOfSETCCToCZERO(SDNode *N, SelectionDAG &DAG,
SDValue N0 = N->getOperand(0);
SDValue N1 = N->getOperand(1);
- auto IsEqualCompZero = [](SDValue &V) -> bool {
- if (V.getOpcode() == ISD::SETCC && isNullConstant(V.getOperand(1))) {
+ auto IsSetCCEquality = [](SDValue &V) -> bool {
+ if (V.getOpcode() == ISD::SETCC) {
ISD::CondCode CC = cast<CondCodeSDNode>(V.getOperand(2))->get();
if (ISD::isIntEqualitySetCC(CC))
return true;
@@ -18293,23 +18298,34 @@ static SDValue combineANDOfSETCCToCZERO(SDNode *N, SelectionDAG &DAG,
return false;
};
- if (!IsEqualCompZero(N0) || !N0.hasOneUse())
+ if (!IsSetCCEquality(N0) || !N0.hasOneUse())
std::swap(N0, N1);
- if (!IsEqualCompZero(N0) || !N0.hasOneUse())
+ if (!IsSetCCEquality(N0) || !N0.hasOneUse())
return SDValue();
KnownBits Known = DAG.computeKnownBits(N1);
if (Known.getMaxValue().ugt(1))
return SDValue();
+ SDLoc DL(N);
+ EVT VT = N->getValueType(0);
unsigned CzeroOpcode =
(cast<CondCodeSDNode>(N0.getOperand(2))->get() == ISD::SETNE)
? RISCVISD::CZERO_EQZ
: RISCVISD::CZERO_NEZ;
- EVT VT = N->getValueType(0);
- SDLoc DL(N);
- return DAG.getNode(CzeroOpcode, DL, VT, N1, N0.getOperand(0));
+ // From here we have two cases:
+ // Either setcc is a comparision with zero, and we can lower directly to a
+ // czero instruction; Or it's not a constant zero, in which case we can still
+ // use a czero, but we first need to get a zero or one value comparing the two
+ // sides of setcc.
+ SDValue Rhs = N0.getOperand(0);
+ if (!isNullConstant(N0->getOperand(1))) {
+ Rhs = DAG.getNode(ISD::XOR, DL, N0.getValueType(), N0.getOperand(0),
+ N0.getOperand(1));
+ }
+
+ return DAG.getNode(CzeroOpcode, DL, VT, N1, Rhs);
}
static SDValue reduceANDOfAtomicLoad(SDNode *N,
diff --git a/llvm/test/CodeGen/RISCV/zicond-opts.ll b/llvm/test/CodeGen/RISCV/zicond-opts.ll
index e6b36537b2b90..34e4da410ed79 100644
--- a/llvm/test/CodeGen/RISCV/zicond-opts.ll
+++ b/llvm/test/CodeGen/RISCV/zicond-opts.ll
@@ -163,6 +163,122 @@ define i32 @icmp_and_and(i64 %x, i64 %y, i64 %z) {
ret i32 %9
}
+; (and (i1) f, (setcc a, b, eq)) -> (czero.nez f, (xor a, b))
+define i1 @and_icmp_eq_non_constant(i64 %x0, i64 %x1, i64 %y0, i64 %y1) {
+; RV32ZICOND-LABEL: and_icmp_eq_non_constant:
+; RV32ZICOND: # %bb.0:
+; RV32ZICOND-NEXT: xor t0, a1, a5
+; RV32ZICOND-NEXT: sltu a0, a0, a4
+; RV32ZICOND-NEXT: czero.nez a0, a0, t0
+; RV32ZICOND-NEXT: sltu a1, a1, a5
+; RV32ZICOND-NEXT: or a0, a0, a1
+; RV32ZICOND-NEXT: xor a1, a3, a7
+; RV32ZICOND-NEXT: xor a4, a2, a6
+; RV32ZICOND-NEXT: or a4, a4, a1
+; RV32ZICOND-NEXT: sltu a2, a2, a6
+; RV32ZICOND-NEXT: czero.nez a1, a2, a1
+; RV32ZICOND-NEXT: sltu a2, a3, a7
+; RV32ZICOND-NEXT: czero.nez a0, a0, a4
+; RV32ZICOND-NEXT: or a1, a1, a2
+; RV32ZICOND-NEXT: or a0, a1, a0
+; RV32ZICOND-NEXT: ret
+;
+; RV64ZICOND-LABEL: and_icmp_eq_non_constant:
+; RV64ZICOND: # %bb.0:
+; RV64ZICOND-NEXT: sltu a0, a0, a2
+; RV64ZICOND-NEXT: xor a2, a1, a3
+; RV64ZICOND-NEXT: czero.nez a0, a0, a2
+; RV64ZICOND-NEXT: sltu a1, a1, a3
+; RV64ZICOND-NEXT: or a0, a1, a0
+; RV64ZICOND-NEXT: ret
+
+ %5 = icmp ult i64 %x0, %y0
+ %6 = icmp eq i64 %x1, %y1
+ %7 = and i1 %5, %6
+ %8 = icmp ult i64 %x1, %y1
+ %9 = or i1 %8, %7
+ ret i1 %9
+}
+
+; (and (i1) f, (setcc a, b, ne)) -> (czero.eqz f, (xor a, b))
+define i1 @and_icmp_ne_non_constant(i64 %x0, i64 %x1, i64 %y0, i64 %y1) {
+; RV32ZICOND-LABEL: and_icmp_ne_non_constant:
+; RV32ZICOND: # %bb.0:
+; RV32ZICOND-NEXT: xor t0, a1, a5
+; RV32ZICOND-NEXT: sltu a0, a0, a4
+; RV32ZICOND-NEXT: czero.nez a0, a0, t0
+; RV32ZICOND-NEXT: sltu a1, a1, a5
+; RV32ZICOND-NEXT: or a0, a0, a1
+; RV32ZICOND-NEXT: xor a1, a3, a7
+; RV32ZICOND-NEXT: xor a4, a2, a6
+; RV32ZICOND-NEXT: or a4, a4, a1
+; RV32ZICOND-NEXT: sltu a2, a2, a6
+; RV32ZICOND-NEXT: czero.nez a1, a2, a1
+; RV32ZICOND-NEXT: sltu a2, a3, a7
+; RV32ZICOND-NEXT: czero.eqz a0, a0, a4
+; RV32ZICOND-NEXT: or a1, a1, a2
+; RV32ZICOND-NEXT: or a0, a1, a0
+; RV32ZICOND-NEXT: ret
+;
+; RV64ZICOND-LABEL: and_icmp_ne_non_constant:
+; RV64ZICOND: # %bb.0:
+; RV64ZICOND-NEXT: sltu a0, a0, a2
+; RV64ZICOND-NEXT: xor a2, a1, a3
+; RV64ZICOND-NEXT: czero.eqz a0, a0, a2
+; RV64ZICOND-NEXT: sltu a1, a1, a3
+; RV64ZICOND-NEXT: or a0, a1, a0
+; RV64ZICOND-NEXT: ret
+
+ %5 = icmp ult i64 %x0, %y0
+ %6 = icmp ne i64 %x1, %y1
+ %7 = and i1 %5, %6
+ %8 = icmp ult i64 %x1, %y1
+ %9 = or i1 %8, %7
+ ret i1 %9
+}
+
+; (and (i1) f, (setcc a, b, ne)) -> (czero.eqz f, (xor a, b))
+; Test the above transform, but setcc has multiple uses, so we can't transform.
+define i32 @and_icmp_eq_non_constant_multiple_uses(i64 %x0, i64 %x1, i64 %y0, i64 %y1) {
+; RV32ZICOND-LABEL: and_icmp_eq_non_constant_multiple_uses:
+; RV32ZICOND: # %bb.0:
+; RV32ZICOND-NEXT: xor t0, a1, a5
+; RV32ZICOND-NEXT: sltu a0, a0, a4
+; RV32ZICOND-NEXT: czero.nez a0, a0, t0
+; RV32ZICOND-NEXT: xor a3, a3, a7
+; RV32ZICOND-NEXT: xor a2, a2, a6
+; RV32ZICOND-NEXT: sltu a1, a1, a5
+; RV32ZICOND-NEXT: or a2, a2, a3
+; RV32ZICOND-NEXT: or a0, a0, a1
+; RV32ZICOND-NEXT: seqz a1, a2
+; RV32ZICOND-NEXT: and a0, a0, a1
+; RV32ZICOND-NEXT: add a0, a0, a1
+; RV32ZICOND-NEXT: ret
+;
+; RV64ZICOND-LABEL: and_icmp_eq_non_constant_multiple_uses:
+; RV64ZICOND: # %bb.0:
+; RV64ZICOND-NEXT: xor a1, a1, a3
+; RV64ZICOND-NEXT: sltu a0, a0, a2
+; RV64ZICOND-NEXT: seqz a1, a1
+; RV64ZICOND-NEXT: and a0, a0, a1
+; RV64ZICOND-NEXT: add a0, a0, a1
+; RV64ZICOND-NEXT: ret
+
+ %lt = icmp ult i64 %x0, %y0
+ %eq = icmp eq i64 %x1, %y1
+
+ ; First use of %eq.
+ %both = and i1 %lt, %eq
+
+ %both.ext = zext i1 %both to i32
+
+ ; Second use of %eq.
+ %eq.ext = zext i1 %eq to i32
+
+ %result = add i32 %both.ext, %eq.ext
+ ret i32 %result
+}
+
; (select cond, x, rotl(x, rot.amt)) -> (rotl x, (czero_nez rot.amt, cond))
define i64 @rotate_l_nez(i64 %x, i64 %rot.amt, i1 %cond) {
; RV32ZICOND-LABEL: rotate_l_nez:
@@ -426,19 +542,19 @@ define i64 @select_wo_optsize_minsize(i64 %true, i64 %false, i1 zeroext %c) {
define i64 @select_w_optsize(i64 %true, i64 %false, i1 zeroext %c) optsize {
; RV32ZICOND-LABEL: select_w_optsize:
; RV32ZICOND: # %bb.0:
-; RV32ZICOND-NEXT: bnez a4, .LBB16_2
+; RV32ZICOND-NEXT: bnez a4, .LBB19_2
; RV32ZICOND-NEXT: # %bb.1:
; RV32ZICOND-NEXT: mv a0, a2
; RV32ZICOND-NEXT: mv a1, a3
-; RV32ZICOND-NEXT: .LBB16_2:
+; RV32ZICOND-NEXT: .LBB19_2:
; RV32ZICOND-NEXT: ret
;
; RV64ZICOND-LABEL: select_w_optsize:
; RV64ZICOND: # %bb.0:
-; RV64ZICOND-NEXT: bnez a2, .LBB16_2
+; RV64ZICOND-NEXT: bnez a2, .LBB19_2
; RV64ZICOND-NEXT: # %bb.1:
; RV64ZICOND-NEXT: mv a0, a1
-; RV64ZICOND-NEXT: .LBB16_2:
+; RV64ZICOND-NEXT: .LBB19_2:
; RV64ZICOND-NEXT: ret
%r = select i1 %c, i64 %true, i64 %false
ret i64 %r
@@ -447,19 +563,19 @@ define i64 @select_w_optsize(i64 %true, i64 %false, i1 zeroext %c) optsize {
define i64 @select_w_minsize(i64 %true, i64 %false, i1 zeroext %c) minsize {
; RV32ZICOND-LABEL: select_w_minsize:
; RV32ZICOND: # %bb.0:
-; RV32ZICOND-NEXT: bnez a4, .LBB17_2
+; RV32ZICOND-NEXT: bnez a4, .LBB20_2
; RV32ZICOND-NEXT: # %bb.1:
; RV32ZICOND-NEXT: mv a0, a2
; RV32ZICOND-NEXT: mv a1, a3
-; RV32ZICOND-NEXT: .LBB17_2:
+; RV32ZICOND-NEXT: .LBB20_2:
; RV32ZICOND-NEXT: ret
;
; RV64ZICOND-LABEL: select_w_minsize:
; RV64ZICOND: # %bb.0:
-; RV64ZICOND-NEXT: bnez a2, .LBB17_2
+; RV64ZICOND-NEXT: bnez a2, .LBB20_2
; RV64ZICOND-NEXT: # %bb.1:
; RV64ZICOND-NEXT: mv a0, a1
-; RV64ZICOND-NEXT: .LBB17_2:
+; RV64ZICOND-NEXT: .LBB20_2:
; RV64ZICOND-NEXT: ret
%r = select i1 %c, i64 %true, i64 %false
ret i64 %r
|
I have read the various policies, and believe this PR complies with all relevant parts |
…ension This is a generalization of the previously existing rule, that required one of the sides of the `setcc` to be zero. With this commit we now support comparisions betwen any two variables. This requires us to insert an additional instruction to compare the two. But it is still beneficial vs the unoptimized lowering as it reduces 1 instruction in the output. This fixes one of the issues identified in llvm#179584. Namely `test_lt3`, now uses czero.{nez,eqz} for those cases.
73012dd to
06aa8ad
Compare
This is a generalization of the previously existing rule, that required one of the sides of the
setccto be zero.With this commit we now support comparisons between any two variables. This requires us to insert an additional instruction to compare the two. But it is still beneficial vs the unoptimized lowering as it reduces 1 instruction in the output.
This fixes one of the issues identified in #179584. Namely
test_lt3, now usesczero.{nez,eqz}for those cases.AI Disclosure: I have used AI to help me understand the LLVM build and test system as well as to understand the existing codebase. The new code add in this PR is authored by me.