[RISCV] Use BuildPairF64 and SplitF64 for bitcast i64<->f64 on rv32 regardless of Zfa. #85982
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
…egardless of Zfa. Previously we used BuildPairF64 and SplitF64 only if Zfa was supported since they will select register file moves that are only available with Zfa. We recently changed the handling of BuildPairF64/SplitF64 for Zdinx to not go through memory so we should use that for bitcast. That leaves the D without Zfa case that does need to go through memory. Previously we let type legalization expand to loads and stores using a new stack temporary created for each bitcast. After this patch we will create the loads ands stores in the custom inserter and share the same stack slot for all. This also allows DAGCombiner to optimize when bitcast is mixed with BuildPairF64/SplitF64.
|
@llvm/pr-subscribers-backend-risc-v Author: Craig Topper (topperc) ChangesPreviously we used BuildPairF64 and SplitF64 only if Zfa was supported since they will select register file moves that are only available with Zfa. We recently changed the handling of BuildPairF64/SplitF64 for Zdinx to not go through memory so we should use that for bitcast. That leaves the D without Zfa case that does need to go through memory. Previously we let type legalization expand to loads and stores using a new stack temporary created for each bitcast. After this patch we will create the loads ands stores in the custom inserter and share the same stack slot for all. This also allows DAGCombiner to optimize when bitcast is mixed with BuildPairF64/SplitF64. Full diff: https://github.com/llvm/llvm-project/pull/85982.diff 5 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 3aa28215efc23c..a3ebfb34ad7aa8 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -559,11 +559,12 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
if (Subtarget.hasStdExtDOrZdinx()) {
setOperationAction(FPLegalNodeTypes, MVT::f64, Legal);
+ if (!Subtarget.is64Bit())
+ setOperationAction(ISD::BITCAST, MVT::i64, Custom);
+
if (Subtarget.hasStdExtZfa()) {
setOperationAction(FPRndMode, MVT::f64, Legal);
setOperationAction(ISD::FNEARBYINT, MVT::f64, Legal);
- if (!Subtarget.is64Bit())
- setOperationAction(ISD::BITCAST, MVT::i64, Custom);
} else {
if (Subtarget.is64Bit())
setOperationAction(FPRndMode, MVT::f64, Custom);
@@ -6071,8 +6072,7 @@ SDValue RISCVTargetLowering::LowerOperation(SDValue Op,
DAG.getNode(RISCVISD::FMV_W_X_RV64, DL, MVT::f32, NewOp0);
return FPConv;
}
- if (VT == MVT::f64 && Op0VT == MVT::i64 && XLenVT == MVT::i32 &&
- Subtarget.hasStdExtZfa()) {
+ if (VT == MVT::f64 && Op0VT == MVT::i64 && XLenVT == MVT::i32) {
SDValue Lo, Hi;
std::tie(Lo, Hi) = DAG.SplitScalar(Op0, DL, MVT::i32, MVT::i32);
SDValue RetReg =
@@ -12157,8 +12157,7 @@ void RISCVTargetLowering::ReplaceNodeResults(SDNode *N,
SDValue FPConv =
DAG.getNode(RISCVISD::FMV_X_ANYEXTW_RV64, DL, MVT::i64, Op0);
Results.push_back(DAG.getNode(ISD::TRUNCATE, DL, MVT::i32, FPConv));
- } else if (VT == MVT::i64 && Op0VT == MVT::f64 && XLenVT == MVT::i32 &&
- Subtarget.hasStdExtZfa()) {
+ } else if (VT == MVT::i64 && Op0VT == MVT::f64 && XLenVT == MVT::i32) {
SDValue NewReg = DAG.getNode(RISCVISD::SplitF64, DL,
DAG.getVTList(MVT::i32, MVT::i32), Op0);
SDValue RetReg = DAG.getNode(ISD::BUILD_PAIR, DL, MVT::i64,
diff --git a/llvm/test/CodeGen/RISCV/double-convert.ll b/llvm/test/CodeGen/RISCV/double-convert.ll
index 7a9439e5b322c6..c1429642962e89 100644
--- a/llvm/test/CodeGen/RISCV/double-convert.ll
+++ b/llvm/test/CodeGen/RISCV/double-convert.ll
@@ -1116,13 +1116,7 @@ define i64 @fmv_x_d(double %a, double %b) nounwind {
;
; RV32IZFINXZDINX-LABEL: fmv_x_d:
; RV32IZFINXZDINX: # %bb.0:
-; RV32IZFINXZDINX-NEXT: addi sp, sp, -16
; RV32IZFINXZDINX-NEXT: fadd.d a0, a0, a2
-; RV32IZFINXZDINX-NEXT: sw a0, 8(sp)
-; RV32IZFINXZDINX-NEXT: sw a1, 12(sp)
-; RV32IZFINXZDINX-NEXT: lw a0, 8(sp)
-; RV32IZFINXZDINX-NEXT: lw a1, 12(sp)
-; RV32IZFINXZDINX-NEXT: addi sp, sp, 16
; RV32IZFINXZDINX-NEXT: ret
;
; RV64IZFINXZDINX-LABEL: fmv_x_d:
@@ -1257,13 +1251,13 @@ define double @fmv_d_x(i64 %a, i64 %b) nounwind {
; RV32IFD-LABEL: fmv_d_x:
; RV32IFD: # %bb.0:
; RV32IFD-NEXT: addi sp, sp, -16
-; RV32IFD-NEXT: sw a3, 4(sp)
-; RV32IFD-NEXT: sw a2, 0(sp)
-; RV32IFD-NEXT: sw a1, 12(sp)
; RV32IFD-NEXT: sw a0, 8(sp)
-; RV32IFD-NEXT: fld fa5, 0(sp)
+; RV32IFD-NEXT: sw a1, 12(sp)
+; RV32IFD-NEXT: fld fa5, 8(sp)
+; RV32IFD-NEXT: sw a2, 8(sp)
+; RV32IFD-NEXT: sw a3, 12(sp)
; RV32IFD-NEXT: fld fa4, 8(sp)
-; RV32IFD-NEXT: fadd.d fa0, fa4, fa5
+; RV32IFD-NEXT: fadd.d fa0, fa5, fa4
; RV32IFD-NEXT: addi sp, sp, 16
; RV32IFD-NEXT: ret
;
@@ -1276,17 +1270,7 @@ define double @fmv_d_x(i64 %a, i64 %b) nounwind {
;
; RV32IZFINXZDINX-LABEL: fmv_d_x:
; RV32IZFINXZDINX: # %bb.0:
-; RV32IZFINXZDINX-NEXT: addi sp, sp, -16
-; RV32IZFINXZDINX-NEXT: sw a3, 4(sp)
-; RV32IZFINXZDINX-NEXT: sw a2, 0(sp)
-; RV32IZFINXZDINX-NEXT: sw a1, 12(sp)
-; RV32IZFINXZDINX-NEXT: sw a0, 8(sp)
-; RV32IZFINXZDINX-NEXT: lw a0, 0(sp)
-; RV32IZFINXZDINX-NEXT: lw a1, 4(sp)
-; RV32IZFINXZDINX-NEXT: lw a2, 8(sp)
-; RV32IZFINXZDINX-NEXT: lw a3, 12(sp)
-; RV32IZFINXZDINX-NEXT: fadd.d a0, a2, a0
-; RV32IZFINXZDINX-NEXT: addi sp, sp, 16
+; RV32IZFINXZDINX-NEXT: fadd.d a0, a0, a2
; RV32IZFINXZDINX-NEXT: ret
;
; RV64IZFINXZDINX-LABEL: fmv_d_x:
diff --git a/llvm/test/CodeGen/RISCV/inline-asm-d-constraint-f.ll b/llvm/test/CodeGen/RISCV/inline-asm-d-constraint-f.ll
index 71769a800c06bf..c480ba800c6904 100644
--- a/llvm/test/CodeGen/RISCV/inline-asm-d-constraint-f.ll
+++ b/llvm/test/CodeGen/RISCV/inline-asm-d-constraint-f.ll
@@ -75,24 +75,10 @@ define double @constraint_f_double_abi_name(double %a) nounwind {
define double @constraint_gpr(double %x) {
; RV32F-LABEL: constraint_gpr:
; RV32F: # %bb.0:
-; RV32F-NEXT: addi sp, sp, -32
-; RV32F-NEXT: .cfi_def_cfa_offset 32
-; RV32F-NEXT: sw a0, 8(sp)
-; RV32F-NEXT: sw a1, 12(sp)
-; RV32F-NEXT: fld fa5, 8(sp)
-; RV32F-NEXT: fsd fa5, 24(sp)
-; RV32F-NEXT: lw a0, 24(sp)
-; RV32F-NEXT: lw a1, 28(sp)
+; RV32F-NEXT: .cfi_def_cfa_offset 0
; RV32F-NEXT: #APP
; RV32F-NEXT: mv a0, a0
; RV32F-NEXT: #NO_APP
-; RV32F-NEXT: sw a1, 20(sp)
-; RV32F-NEXT: sw a0, 16(sp)
-; RV32F-NEXT: fld fa5, 16(sp)
-; RV32F-NEXT: fsd fa5, 8(sp)
-; RV32F-NEXT: lw a0, 8(sp)
-; RV32F-NEXT: lw a1, 12(sp)
-; RV32F-NEXT: addi sp, sp, 32
; RV32F-NEXT: ret
;
; RV64F-LABEL: constraint_gpr:
diff --git a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-bitcast.ll b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-bitcast.ll
index b7afee754f68b3..5252eb71c383db 100644
--- a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-bitcast.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-bitcast.ll
@@ -416,8 +416,8 @@ define double @bitcast_v1i64_f64(<1 x i64> %a) {
; RV32ELEN32: # %bb.0:
; RV32ELEN32-NEXT: addi sp, sp, -16
; RV32ELEN32-NEXT: .cfi_def_cfa_offset 16
-; RV32ELEN32-NEXT: sw a1, 12(sp)
; RV32ELEN32-NEXT: sw a0, 8(sp)
+; RV32ELEN32-NEXT: sw a1, 12(sp)
; RV32ELEN32-NEXT: fld fa0, 8(sp)
; RV32ELEN32-NEXT: addi sp, sp, 16
; RV32ELEN32-NEXT: ret
diff --git a/llvm/test/CodeGen/RISCV/spill-fill-fold.ll b/llvm/test/CodeGen/RISCV/spill-fill-fold.ll
index a9a0cc5cf94d85..8cf5f55ad5c51d 100644
--- a/llvm/test/CodeGen/RISCV/spill-fill-fold.ll
+++ b/llvm/test/CodeGen/RISCV/spill-fill-fold.ll
@@ -290,8 +290,8 @@ define double @spill_i64_to_double(i64 %a) nounwind {
; RV32ID-NEXT: fsd fs9, 40(sp) # 8-byte Folded Spill
; RV32ID-NEXT: fsd fs10, 32(sp) # 8-byte Folded Spill
; RV32ID-NEXT: fsd fs11, 24(sp) # 8-byte Folded Spill
-; RV32ID-NEXT: sw a1, 20(sp)
; RV32ID-NEXT: sw a0, 16(sp)
+; RV32ID-NEXT: sw a1, 20(sp)
; RV32ID-NEXT: fld fa5, 16(sp)
; RV32ID-NEXT: fsd fa5, 8(sp) # 8-byte Folded Spill
; RV32ID-NEXT: #APP
@@ -804,13 +804,15 @@ define double @fill_i64_to_double(i64 %a) nounwind {
; RV32ID-NEXT: fsd fs9, 40(sp) # 8-byte Folded Spill
; RV32ID-NEXT: fsd fs10, 32(sp) # 8-byte Folded Spill
; RV32ID-NEXT: fsd fs11, 24(sp) # 8-byte Folded Spill
-; RV32ID-NEXT: sw a1, 20(sp)
-; RV32ID-NEXT: sw a0, 16(sp)
-; RV32ID-NEXT: fld fa5, 16(sp)
-; RV32ID-NEXT: fsd fa5, 8(sp) # 8-byte Folded Spill
+; RV32ID-NEXT: sw a1, 12(sp) # 4-byte Folded Spill
+; RV32ID-NEXT: sw a0, 8(sp) # 4-byte Folded Spill
; RV32ID-NEXT: #APP
; RV32ID-NEXT: #NO_APP
-; RV32ID-NEXT: fld fa0, 8(sp) # 8-byte Folded Reload
+; RV32ID-NEXT: lw a0, 8(sp) # 4-byte Folded Reload
+; RV32ID-NEXT: sw a0, 16(sp)
+; RV32ID-NEXT: lw a0, 12(sp) # 4-byte Folded Reload
+; RV32ID-NEXT: sw a0, 20(sp)
+; RV32ID-NEXT: fld fa0, 16(sp)
; RV32ID-NEXT: lw ra, 172(sp) # 4-byte Folded Reload
; RV32ID-NEXT: lw s0, 168(sp) # 4-byte Folded Reload
; RV32ID-NEXT: lw s1, 164(sp) # 4-byte Folded Reload
|
|
This trips the subregister assertions in |
topperc
added a commit
that referenced
this pull request
Mar 21, 2024
…nts (#85998) With GPR pairs from Zdinx, we can't guarantee there are no subregisters on integer instruction operands. I've been able to get these assertions to fire after some other recent PRs. I've added a FIXME to support this properly. I just wanted to prevent the assertion failure for now. No test case because my other patch #85982 that allowed me to fail the assert hasn't been approved yet, and I don't know for that that patch is required to hit this assert. It's just what exposed it for me. So I think this patch is a good precaution regardless.
wangpc-pp
reviewed
Mar 21, 2024
| ; RV32ID-NEXT: #APP | ||
| ; RV32ID-NEXT: #NO_APP | ||
| ; RV32ID-NEXT: fld fa0, 8(sp) # 8-byte Folded Reload | ||
| ; RV32ID-NEXT: lw a0, 8(sp) # 4-byte Folded Reload |
There was a problem hiding this comment.
I don't know what to do about it. We spilled the GPRs that were holding the double due to the inline assembly. Then we reloaded the GPRs, then did the copy to FP. I think we must have gotten lucky with instruction scheduling before so that the copy to FPR happened before the inline assembly so we only had to spill and reload an FPR.
chencha3
pushed a commit
to chencha3/llvm-project
that referenced
this pull request
Mar 23, 2024
…nts (llvm#85998) With GPR pairs from Zdinx, we can't guarantee there are no subregisters on integer instruction operands. I've been able to get these assertions to fire after some other recent PRs. I've added a FIXME to support this properly. I just wanted to prevent the assertion failure for now. No test case because my other patch llvm#85982 that allowed me to fail the assert hasn't been approved yet, and I don't know for that that patch is required to hit this assert. It's just what exposed it for me. So I think this patch is a good precaution regardless.
chencha3
pushed a commit
to chencha3/llvm-project
that referenced
this pull request
Mar 23, 2024
…egardless of Zfa. (llvm#85982) Previously we used BuildPairF64 and SplitF64 only if Zfa was supported since they will select register file moves that are only available with Zfa. We recently changed the handling of BuildPairF64/SplitF64 for Zdinx to not go through memory so we should use that for bitcast. That leaves the D without Zfa case that does need to go through memory. Previously we let type legalization expand to loads and stores using a new stack temporary created for each bitcast. After this patch we will create the loads ands stores in the custom inserter and share the same stack slot for all. This also allows DAGCombiner to optimize when bitcast is mixed with BuildPairF64/SplitF64.
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.
Previously we used BuildPairF64 and SplitF64 only if Zfa was supported since they will select register file moves that are only available with Zfa.
We recently changed the handling of BuildPairF64/SplitF64 for Zdinx to not go through memory so we should use that for bitcast.
That leaves the D without Zfa case that does need to go through memory. Previously we let type legalization expand to loads and stores using a new stack temporary created for each bitcast. After this patch we will create the loads ands stores in the custom inserter and share the same stack slot for all. This also allows DAGCombiner to optimize when bitcast is mixed with BuildPairF64/SplitF64.