[RISCV] Use PromoteSetCCOperands to promote operands for UMAX/UMIN during type legalization. #82716
Conversation
This file contains 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
…ring type legalization. For RISC-V, we were always choosing to sign extend when promoting i32->i64. If the promoted inputs happen to be zero extended already, we should use zero extend instead. This is what we do for SETCC.
topperc
requested review from
preames,
RKSimon,
davemgreen,
efriedma-quic and
david-arm
|
@llvm/pr-subscribers-llvm-selectiondag Author: Craig Topper (topperc) ChangesFor RISC-V, we were always choosing to sign extend when promoting i32->i64. If the promoted inputs happen to be zero extended already, we should use zero extend instead. This is what we do for SETCC. Full diff: https://github.com/llvm/llvm-project/pull/82716.diff 5 Files Affected:
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
index a4ba261686c688..575f46f9341edb 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
@@ -1343,10 +1343,14 @@ SDValue DAGTypeLegalizer::PromoteIntRes_ZExtIntBinOp(SDNode *N) {
}
SDValue DAGTypeLegalizer::PromoteIntRes_UMINUMAX(SDNode *N) {
+ SDValue LHS = N->getOperand(0);
+ SDValue RHS = N->getOperand(1);
+
// It doesn't matter if we sign extend or zero extend in the inputs. So do
- // whatever is best for the target.
- SDValue LHS = SExtOrZExtPromotedInteger(N->getOperand(0));
- SDValue RHS = SExtOrZExtPromotedInteger(N->getOperand(1));
+ // whatever is best for the target and the promoted operands. We can reuse
+ // PromoteSetCCOperands by passing it an unsigned predicate.
+ PromoteSetCCOperands(LHS, RHS, ISD::SETUGT);
+
return DAG.getNode(N->getOpcode(), SDLoc(N),
LHS.getValueType(), LHS, RHS);
}
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
index 91149871628574..05c918b9c78524 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
@@ -274,20 +274,6 @@ class LLVM_LIBRARY_VISIBILITY DAGTypeLegalizer {
return DAG.getZeroExtendInReg(Op, dl, OldVT);
}
- // Get a promoted operand and sign or zero extend it to the final size
- // (depending on TargetLoweringInfo::isSExtCheaperThanZExt). For a given
- // subtarget and type, the choice of sign or zero-extension will be
- // consistent.
- SDValue SExtOrZExtPromotedInteger(SDValue Op) {
- EVT OldVT = Op.getValueType();
- SDLoc DL(Op);
- Op = GetPromotedInteger(Op);
- if (TLI.isSExtCheaperThanZExt(OldVT, Op.getValueType()))
- return DAG.getNode(ISD::SIGN_EXTEND_INREG, DL, Op.getValueType(), Op,
- DAG.getValueType(OldVT));
- return DAG.getZeroExtendInReg(Op, DL, OldVT);
- }
-
// Promote the given operand V (vector or scalar) according to N's specific
// reduction kind. N must be an integer VECREDUCE_* or VP_REDUCE_*. Returns
// the nominal extension opcode (ISD::(ANY|ZERO|SIGN)_EXTEND) and the
diff --git a/llvm/test/CodeGen/RISCV/fpclamptosat.ll b/llvm/test/CodeGen/RISCV/fpclamptosat.ll
index 3880ac9f4ec623..9e93ad0043a7e0 100644
--- a/llvm/test/CodeGen/RISCV/fpclamptosat.ll
+++ b/llvm/test/CodeGen/RISCV/fpclamptosat.ll
@@ -2812,7 +2812,6 @@ define i16 @utesth_f16i16_mm(half %x) {
; RV64-NEXT: .cfi_offset ra, -8
; RV64-NEXT: call __extendhfsf2
; RV64-NEXT: fcvt.lu.s a0, fa0, rtz
-; RV64-NEXT: sext.w a0, a0
; RV64-NEXT: lui a1, 16
; RV64-NEXT: addiw a1, a1, -1
; RV64-NEXT: bltu a0, a1, .LBB43_2
diff --git a/llvm/test/CodeGen/RISCV/rvv/fpclamptosat_vec.ll b/llvm/test/CodeGen/RISCV/rvv/fpclamptosat_vec.ll
index 208c881294eed6..48ce7d623475cb 100644
--- a/llvm/test/CodeGen/RISCV/rvv/fpclamptosat_vec.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/fpclamptosat_vec.ll
@@ -4827,10 +4827,10 @@ define <8 x i16> @utesth_f16i16_mm(<8 x half> %x) {
; CHECK-NOV-NEXT: mv s0, a0
; CHECK-NOV-NEXT: fmv.w.x fa0, a1
; CHECK-NOV-NEXT: call __extendhfsf2
-; CHECK-NOV-NEXT: fmv.s fs5, fa0
+; CHECK-NOV-NEXT: fmv.s fs6, fa0
; CHECK-NOV-NEXT: fmv.w.x fa0, s7
; CHECK-NOV-NEXT: call __extendhfsf2
-; CHECK-NOV-NEXT: fmv.s fs6, fa0
+; CHECK-NOV-NEXT: fmv.s fs5, fa0
; CHECK-NOV-NEXT: fmv.w.x fa0, s6
; CHECK-NOV-NEXT: call __extendhfsf2
; CHECK-NOV-NEXT: fmv.s fs4, fa0
@@ -4846,54 +4846,36 @@ define <8 x i16> @utesth_f16i16_mm(<8 x half> %x) {
; CHECK-NOV-NEXT: fmv.w.x fa0, s2
; CHECK-NOV-NEXT: call __extendhfsf2
; CHECK-NOV-NEXT: fmv.s fs0, fa0
-; CHECK-NOV-NEXT: fcvt.lu.s s2, fs6, rtz
-; CHECK-NOV-NEXT: fcvt.lu.s a0, fs5, rtz
; CHECK-NOV-NEXT: fmv.w.x fa0, s1
-; CHECK-NOV-NEXT: sext.w s1, a0
+; CHECK-NOV-NEXT: fcvt.lu.s s1, fs6, rtz
; CHECK-NOV-NEXT: call __extendhfsf2
; CHECK-NOV-NEXT: fcvt.lu.s a0, fa0, rtz
-; CHECK-NOV-NEXT: sext.w a0, a0
; CHECK-NOV-NEXT: lui a1, 16
; CHECK-NOV-NEXT: addiw a1, a1, -1
-; CHECK-NOV-NEXT: bltu a0, a1, .LBB43_2
+; CHECK-NOV-NEXT: bgeu a0, a1, .LBB43_10
; CHECK-NOV-NEXT: # %bb.1: # %entry
-; CHECK-NOV-NEXT: mv a0, a1
+; CHECK-NOV-NEXT: fcvt.lu.s a2, fs5, rtz
+; CHECK-NOV-NEXT: bgeu s1, a1, .LBB43_11
; CHECK-NOV-NEXT: .LBB43_2: # %entry
; CHECK-NOV-NEXT: fcvt.lu.s a3, fs4, rtz
-; CHECK-NOV-NEXT: sext.w a2, s2
-; CHECK-NOV-NEXT: bltu s1, a1, .LBB43_4
-; CHECK-NOV-NEXT: # %bb.3: # %entry
-; CHECK-NOV-NEXT: mv s1, a1
-; CHECK-NOV-NEXT: .LBB43_4: # %entry
+; CHECK-NOV-NEXT: bgeu a2, a1, .LBB43_12
+; CHECK-NOV-NEXT: .LBB43_3: # %entry
; CHECK-NOV-NEXT: fcvt.lu.s a4, fs3, rtz
-; CHECK-NOV-NEXT: sext.w a3, a3
-; CHECK-NOV-NEXT: bltu a2, a1, .LBB43_6
-; CHECK-NOV-NEXT: # %bb.5: # %entry
-; CHECK-NOV-NEXT: mv a2, a1
-; CHECK-NOV-NEXT: .LBB43_6: # %entry
+; CHECK-NOV-NEXT: bgeu a3, a1, .LBB43_13
+; CHECK-NOV-NEXT: .LBB43_4: # %entry
; CHECK-NOV-NEXT: fcvt.lu.s a5, fs2, rtz
-; CHECK-NOV-NEXT: sext.w a4, a4
-; CHECK-NOV-NEXT: bltu a3, a1, .LBB43_8
-; CHECK-NOV-NEXT: # %bb.7: # %entry
-; CHECK-NOV-NEXT: mv a3, a1
-; CHECK-NOV-NEXT: .LBB43_8: # %entry
+; CHECK-NOV-NEXT: bgeu a4, a1, .LBB43_14
+; CHECK-NOV-NEXT: .LBB43_5: # %entry
; CHECK-NOV-NEXT: fcvt.lu.s a6, fs1, rtz
-; CHECK-NOV-NEXT: sext.w a5, a5
-; CHECK-NOV-NEXT: bltu a4, a1, .LBB43_10
-; CHECK-NOV-NEXT: # %bb.9: # %entry
-; CHECK-NOV-NEXT: mv a4, a1
-; CHECK-NOV-NEXT: .LBB43_10: # %entry
-; CHECK-NOV-NEXT: fcvt.lu.s a7, fs0, rtz
-; CHECK-NOV-NEXT: sext.w a6, a6
; CHECK-NOV-NEXT: bgeu a5, a1, .LBB43_15
-; CHECK-NOV-NEXT: # %bb.11: # %entry
-; CHECK-NOV-NEXT: sext.w a7, a7
+; CHECK-NOV-NEXT: .LBB43_6: # %entry
+; CHECK-NOV-NEXT: fcvt.lu.s a7, fs0, rtz
; CHECK-NOV-NEXT: bgeu a6, a1, .LBB43_16
-; CHECK-NOV-NEXT: .LBB43_12: # %entry
-; CHECK-NOV-NEXT: bltu a7, a1, .LBB43_14
-; CHECK-NOV-NEXT: .LBB43_13: # %entry
+; CHECK-NOV-NEXT: .LBB43_7: # %entry
+; CHECK-NOV-NEXT: bltu a7, a1, .LBB43_9
+; CHECK-NOV-NEXT: .LBB43_8: # %entry
; CHECK-NOV-NEXT: mv a7, a1
-; CHECK-NOV-NEXT: .LBB43_14: # %entry
+; CHECK-NOV-NEXT: .LBB43_9: # %entry
; CHECK-NOV-NEXT: sh a7, 14(s0)
; CHECK-NOV-NEXT: sh a6, 12(s0)
; CHECK-NOV-NEXT: sh a5, 10(s0)
@@ -4920,14 +4902,34 @@ define <8 x i16> @utesth_f16i16_mm(<8 x half> %x) {
; CHECK-NOV-NEXT: fld fs6, 0(sp) # 8-byte Folded Reload
; CHECK-NOV-NEXT: addi sp, sp, 128
; CHECK-NOV-NEXT: ret
+; CHECK-NOV-NEXT: .LBB43_10: # %entry
+; CHECK-NOV-NEXT: mv a0, a1
+; CHECK-NOV-NEXT: fcvt.lu.s a2, fs5, rtz
+; CHECK-NOV-NEXT: bltu s1, a1, .LBB43_2
+; CHECK-NOV-NEXT: .LBB43_11: # %entry
+; CHECK-NOV-NEXT: mv s1, a1
+; CHECK-NOV-NEXT: fcvt.lu.s a3, fs4, rtz
+; CHECK-NOV-NEXT: bltu a2, a1, .LBB43_3
+; CHECK-NOV-NEXT: .LBB43_12: # %entry
+; CHECK-NOV-NEXT: mv a2, a1
+; CHECK-NOV-NEXT: fcvt.lu.s a4, fs3, rtz
+; CHECK-NOV-NEXT: bltu a3, a1, .LBB43_4
+; CHECK-NOV-NEXT: .LBB43_13: # %entry
+; CHECK-NOV-NEXT: mv a3, a1
+; CHECK-NOV-NEXT: fcvt.lu.s a5, fs2, rtz
+; CHECK-NOV-NEXT: bltu a4, a1, .LBB43_5
+; CHECK-NOV-NEXT: .LBB43_14: # %entry
+; CHECK-NOV-NEXT: mv a4, a1
+; CHECK-NOV-NEXT: fcvt.lu.s a6, fs1, rtz
+; CHECK-NOV-NEXT: bltu a5, a1, .LBB43_6
; CHECK-NOV-NEXT: .LBB43_15: # %entry
; CHECK-NOV-NEXT: mv a5, a1
-; CHECK-NOV-NEXT: sext.w a7, a7
-; CHECK-NOV-NEXT: bltu a6, a1, .LBB43_12
+; CHECK-NOV-NEXT: fcvt.lu.s a7, fs0, rtz
+; CHECK-NOV-NEXT: bltu a6, a1, .LBB43_7
; CHECK-NOV-NEXT: .LBB43_16: # %entry
; CHECK-NOV-NEXT: mv a6, a1
-; CHECK-NOV-NEXT: bgeu a7, a1, .LBB43_13
-; CHECK-NOV-NEXT: j .LBB43_14
+; CHECK-NOV-NEXT: bgeu a7, a1, .LBB43_8
+; CHECK-NOV-NEXT: j .LBB43_9
;
; CHECK-V-LABEL: utesth_f16i16_mm:
; CHECK-V: # %bb.0: # %entry
diff --git a/llvm/test/CodeGen/RISCV/rvv/get_vector_length.ll b/llvm/test/CodeGen/RISCV/rvv/get_vector_length.ll
index 1d42b6e3937c76..bd0fecd285515f 100644
--- a/llvm/test/CodeGen/RISCV/rvv/get_vector_length.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/get_vector_length.ll
@@ -52,47 +52,27 @@ define i32 @vector_length_i16_fixed(i16 zeroext %tc) {
}
define i32 @vector_length_i32_fixed(i32 zeroext %tc) {
-; RV32-LABEL: vector_length_i32_fixed:
-; RV32: # %bb.0:
-; RV32-NEXT: li a1, 2
-; RV32-NEXT: bltu a0, a1, .LBB4_2
-; RV32-NEXT: # %bb.1:
-; RV32-NEXT: li a0, 2
-; RV32-NEXT: .LBB4_2:
-; RV32-NEXT: ret
-;
-; RV64-LABEL: vector_length_i32_fixed:
-; RV64: # %bb.0:
-; RV64-NEXT: sext.w a0, a0
-; RV64-NEXT: li a1, 2
-; RV64-NEXT: bltu a0, a1, .LBB4_2
-; RV64-NEXT: # %bb.1:
-; RV64-NEXT: li a0, 2
-; RV64-NEXT: .LBB4_2:
-; RV64-NEXT: ret
+; CHECK-LABEL: vector_length_i32_fixed:
+; CHECK: # %bb.0:
+; CHECK-NEXT: li a1, 2
+; CHECK-NEXT: bltu a0, a1, .LBB4_2
+; CHECK-NEXT: # %bb.1:
+; CHECK-NEXT: li a0, 2
+; CHECK-NEXT: .LBB4_2:
+; CHECK-NEXT: ret
%a = call i32 @llvm.experimental.get.vector.length.i32(i32 %tc, i32 2, i1 false)
ret i32 %a
}
define i32 @vector_length_XLen_fixed(iXLen zeroext %tc) {
-; RV32-LABEL: vector_length_XLen_fixed:
-; RV32: # %bb.0:
-; RV32-NEXT: li a1, 2
-; RV32-NEXT: bltu a0, a1, .LBB5_2
-; RV32-NEXT: # %bb.1:
-; RV32-NEXT: li a0, 2
-; RV32-NEXT: .LBB5_2:
-; RV32-NEXT: ret
-;
-; RV64-LABEL: vector_length_XLen_fixed:
-; RV64: # %bb.0:
-; RV64-NEXT: sext.w a0, a0
-; RV64-NEXT: li a1, 2
-; RV64-NEXT: bltu a0, a1, .LBB5_2
-; RV64-NEXT: # %bb.1:
-; RV64-NEXT: li a0, 2
-; RV64-NEXT: .LBB5_2:
-; RV64-NEXT: ret
+; CHECK-LABEL: vector_length_XLen_fixed:
+; CHECK: # %bb.0:
+; CHECK-NEXT: li a1, 2
+; CHECK-NEXT: bltu a0, a1, .LBB5_2
+; CHECK-NEXT: # %bb.1:
+; CHECK-NEXT: li a0, 2
+; CHECK-NEXT: .LBB5_2:
+; CHECK-NEXT: ret
%a = call i32 @llvm.experimental.get.vector.length.iXLen(iXLen %tc, i32 2, i1 false)
ret i32 %a
}
|
david-arm
reviewed
Feb 23, 2024
| SDValue RHS = SExtOrZExtPromotedInteger(N->getOperand(1)); | ||
| // whatever is best for the target and the promoted operands. We can reuse | ||
| // PromoteSetCCOperands by passing it an unsigned predicate. | ||
| PromoteSetCCOperands(LHS, RHS, ISD::SETUGT); |
There was a problem hiding this comment.
I guess here you're making the assumption that whatever operation is used for umin/umax that the promotion operation will be the same. In theory I can imagine umin/umax using any one of ISD::SETUGT, ISD::SETUGE, ISD::SETULT, ISD::SETULE. In practice, do you think the promotion behaviour will be different between those 4?
There was a problem hiding this comment.
The current implementation of PromoteSetCCOperands only uses the condition code in calls to ISD::isSignedIntSetCC/isUnsignedIntSetCC/isIntEqualitySetCC so any of the 4 unsigned predicates should be the same.
There was a problem hiding this comment.
I've separated the core part of PromoteSetCCOperands into a helper that is independent of the condition code.
I think this could also be used for USUBSAT and maybe other places so distancing from SETCC seemed like a good idea.
… we can call directly.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
arsenm
approved these changes
Feb 26, 2024
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.
For RISC-V, we were always choosing to sign extend when promoting i32->i64. If the promoted inputs happen to be zero extended already, we should use zero extend instead. This is what we do for SETCC.