[llvm] [RISCV][MC] Recognise that fcvt.d.s with frm != 0b000 is valid (PR #67555)
Alex Bradbury via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 27 06:34:52 PDT 2023
https://github.com/asb created https://github.com/llvm/llvm-project/pull/67555
This seems to be an issue common to both GCC and LLVM. There are various RISC-V FCVT instructions where the frm field makes no difference to the output as the result is always exact (e.g. fcvt.d.s, fcvt.s.h, fcvt.d.h). As with GCC, we always generate a form of these fcvt instructions where frm=0b000. However, the ISA manual _doesn't_ state that frm values are invalid, and we should ensure we can accept them. This patch does so by adding the frm field to fcvt.d.s and adding an InstAlias so that if no frm is specified, it defaults to rne (0b000).
This patch just corrects fcvt.d.s in order to allow the approach to be reviewed, before applying it to the other affected instructions.
I haven't added tests to llvm/test/MC/Disassembler/RISCV, because it doesn't seem necessary to test there in addition to our usual round-trip tests in llvm/test/MC/RISCV. But feedback is welcome.
>From 085f9fa7b0ec5774d67cfd26412c78b2c37dd866 Mon Sep 17 00:00:00 2001
From: Alex Bradbury <asb at igalia.com>
Date: Wed, 27 Sep 2023 14:27:10 +0100
Subject: [PATCH] [RISCV][MC] Recognise that fcvt.d.s with frm != 0b000 is
valid
This seems to be an issue common to both GCC and LLVM. There are various
RISC-V FCVT instructions where the frm field makes no difference to the
output as the result is always exact (e.g. fcvt.d.s, fcvt.s.h,
fcvt.d.h). As with GCC, we always generate a form of these fcvt
instructions where frm=0b000. However, the ISA manual _doesn't_ state
that frm values are invalid, and we should ensure we can accept them.
This patch does so by adding the frm field to fcvt.d.s and adding an
InstAlias so that if no frm is specified, it defaults to rne (0b000).
This patch just corrects fcvt.d.s in order to allow the approach to be
reviewed, before applying it to the other affected instructions.
I haven't added tests to llvm/test/MC/Disassembler/RISCV, because it
doesn't seem necessary to test there in addition to our usual round-trip
tests in llvm/test/MC/RISCV. But feedback is welcome.
---
llvm/lib/Target/RISCV/RISCVInstrInfoD.td | 20 +++++++++++++-------
llvm/test/MC/RISCV/rv32d-valid.s | 3 +++
llvm/test/MC/RISCV/rv32zdinx-valid.s | 3 +++
3 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoD.td b/llvm/lib/Target/RISCV/RISCVInstrInfoD.td
index 7a79e3ca6a2f14b..415f8663d6c5a7d 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoD.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoD.td
@@ -115,7 +115,7 @@ foreach Ext = DExts in {
Ext.PrimaryTy, "fcvt.s.d">,
Sched<[WriteFCvtF64ToF32, ReadFCvtF64ToF32]>;
- defm FCVT_D_S : FPUnaryOp_r_m<0b0100001, 0b00000, 0b000, Ext, Ext.PrimaryTy,
+ defm FCVT_D_S : FPUnaryOp_r_frm_m<0b0100001, 0b00000, Ext, Ext.PrimaryTy,
Ext.F32Ty, "fcvt.d.s">,
Sched<[WriteFCvtF32ToF64, ReadFCvtF32ToF64]>;
@@ -200,6 +200,8 @@ let usesCustomInserter = 1 in {
def PseudoQuietFLE_D : PseudoQuietFCMP<FPR64>;
def PseudoQuietFLT_D : PseudoQuietFCMP<FPR64>;
}
+
+def : InstAlias<"fcvt.d.s $rd, $rs", (FCVT_D_S FPR64:$rd, FPR32:$rs, FRM_RNE)>;
} // Predicates = [HasStdExtD]
let Predicates = [HasStdExtZdinx, IsRV64] in {
@@ -214,6 +216,8 @@ let usesCustomInserter = 1 in {
def PseudoQuietFLE_D_INX : PseudoQuietFCMP<FPR64INX>;
def PseudoQuietFLT_D_INX : PseudoQuietFCMP<FPR64INX>;
}
+
+def : InstAlias<"fcvt.d.s $rd, $rs", (FCVT_D_S_INX FPR64INX:$rd, FPR32INX:$rs, FRM_RNE)>;
} // Predicates = [HasStdExtZdinx, IsRV64]
let Predicates = [HasStdExtZdinx, IsRV32] in {
@@ -228,6 +232,8 @@ let usesCustomInserter = 1 in {
def PseudoQuietFLE_D_IN32X : PseudoQuietFCMP<FPR64IN32X>;
def PseudoQuietFLT_D_IN32X : PseudoQuietFCMP<FPR64IN32X>;
}
+
+def : InstAlias<"fcvt.d.s $rd, $rs", (FCVT_D_S_IN32X FPR64IN32X:$rd, FPR32INX:$rs, FRM_RNE)>;
} // Predicates = [HasStdExtZdinx, IsRV32]
//===----------------------------------------------------------------------===//
@@ -240,7 +246,7 @@ let Predicates = [HasStdExtD] in {
// f64 -> f32, f32 -> f64
def : Pat<(any_fpround FPR64:$rs1), (FCVT_S_D FPR64:$rs1, FRM_DYN)>;
-def : Pat<(any_fpextend FPR32:$rs1), (FCVT_D_S FPR32:$rs1)>;
+def : Pat<(any_fpextend FPR32:$rs1), (FCVT_D_S FPR32:$rs1, FRM_RNE)>;
} // Predicates = [HasStdExtD]
let Predicates = [HasStdExtZdinx, IsRV64] in {
@@ -248,7 +254,7 @@ let Predicates = [HasStdExtZdinx, IsRV64] in {
// f64 -> f32, f32 -> f64
def : Pat<(any_fpround FPR64INX:$rs1), (FCVT_S_D_INX FPR64INX:$rs1, FRM_DYN)>;
-def : Pat<(any_fpextend FPR32INX:$rs1), (FCVT_D_S_INX FPR32INX:$rs1)>;
+def : Pat<(any_fpextend FPR32INX:$rs1), (FCVT_D_S_INX FPR32INX:$rs1, FRM_RNE)>;
} // Predicates = [HasStdExtZdinx, IsRV64]
let Predicates = [HasStdExtZdinx, IsRV32] in {
@@ -256,7 +262,7 @@ let Predicates = [HasStdExtZdinx, IsRV32] in {
// f64 -> f32, f32 -> f64
def : Pat<(any_fpround FPR64IN32X:$rs1), (FCVT_S_D_IN32X FPR64IN32X:$rs1, FRM_DYN)>;
-def : Pat<(any_fpextend FPR32INX:$rs1), (FCVT_D_S_IN32X FPR32INX:$rs1)>;
+def : Pat<(any_fpextend FPR32INX:$rs1), (FCVT_D_S_IN32X FPR32INX:$rs1, FRM_RNE)>;
} // Predicates = [HasStdExtZdinx, IsRV32]
// [u]int<->double conversion patterns must be gated on IsRV32 or IsRV64, so
@@ -281,7 +287,7 @@ def : Pat<(riscv_fpclass FPR64:$rs1), (FCLASS_D $rs1)>;
def : PatFprFpr<fcopysign, FSGNJ_D, FPR64, f64>;
def : Pat<(fcopysign FPR64:$rs1, (fneg FPR64:$rs2)), (FSGNJN_D $rs1, $rs2)>;
-def : Pat<(fcopysign FPR64:$rs1, FPR32:$rs2), (FSGNJ_D $rs1, (FCVT_D_S $rs2))>;
+def : Pat<(fcopysign FPR64:$rs1, FPR32:$rs2), (FSGNJ_D $rs1, (FCVT_D_S $rs2, FRM_RNE))>;
def : Pat<(fcopysign FPR32:$rs1, FPR64:$rs2), (FSGNJ_S $rs1, (FCVT_S_D $rs2,
FRM_DYN))>;
@@ -318,7 +324,7 @@ def : PatFprFpr<fcopysign, FSGNJ_D_INX, FPR64INX, f64>;
def : Pat<(fcopysign FPR64INX:$rs1, (fneg FPR64INX:$rs2)),
(FSGNJN_D_INX $rs1, $rs2)>;
def : Pat<(fcopysign FPR64INX:$rs1, FPR32INX:$rs2),
- (FSGNJ_D_INX $rs1, (FCVT_D_S_INX $rs2))>;
+ (FSGNJ_D_INX $rs1, (FCVT_D_S_INX $rs2, FRM_RNE))>;
def : Pat<(fcopysign FPR32INX:$rs1, FPR64INX:$rs2),
(FSGNJ_S_INX $rs1, (FCVT_S_D_INX $rs2, FRM_DYN))>;
@@ -355,7 +361,7 @@ def : PatFprFpr<fcopysign, FSGNJ_D_IN32X, FPR64IN32X, f64>;
def : Pat<(fcopysign FPR64IN32X:$rs1, (fneg FPR64IN32X:$rs2)),
(FSGNJN_D_IN32X $rs1, $rs2)>;
def : Pat<(fcopysign FPR64IN32X:$rs1, FPR32INX:$rs2),
- (FSGNJ_D_IN32X $rs1, (FCVT_D_S_INX $rs2))>;
+ (FSGNJ_D_IN32X $rs1, (FCVT_D_S_INX $rs2, FRM_RNE))>;
def : Pat<(fcopysign FPR32INX:$rs1, FPR64IN32X:$rs2),
(FSGNJ_S_INX $rs1, (FCVT_S_D_IN32X $rs2, FRM_DYN))>;
diff --git a/llvm/test/MC/RISCV/rv32d-valid.s b/llvm/test/MC/RISCV/rv32d-valid.s
index f6254ebfff19865..eb8e9c394a20cd7 100644
--- a/llvm/test/MC/RISCV/rv32d-valid.s
+++ b/llvm/test/MC/RISCV/rv32d-valid.s
@@ -96,6 +96,9 @@ fcvt.s.d fs5, fs6, dyn
# CHECK-ASM-AND-OBJ: fcvt.d.s fs7, fs8
# CHECK-ASM: encoding: [0xd3,0x0b,0x0c,0x42]
fcvt.d.s fs7, fs8
+# CHECK-ASM-AND-OBJ: fcvt.d.s fs7, fs8, rup
+# CHECK-ASM: encoding: [0xd3,0x3b,0x0c,0x42]
+fcvt.d.s fs7, fs8, rup
# CHECK-ASM-AND-OBJ: feq.d a1, fs8, fs9
# CHECK-ASM: encoding: [0xd3,0x25,0x9c,0xa3]
feq.d a1, fs8, fs9
diff --git a/llvm/test/MC/RISCV/rv32zdinx-valid.s b/llvm/test/MC/RISCV/rv32zdinx-valid.s
index 660116bc9a55564..b668bab0caaec7d 100644
--- a/llvm/test/MC/RISCV/rv32zdinx-valid.s
+++ b/llvm/test/MC/RISCV/rv32zdinx-valid.s
@@ -59,6 +59,9 @@ fcvt.s.d x26, x28, dyn
# CHECK-ASM-AND-OBJ: fcvt.d.s s10, t3
# CHECK-ASM: encoding: [0x53,0x0d,0x0e,0x42]
fcvt.d.s x26, x28
+# CHECK-ASM-AND-OBJ: fcvt.d.s s10, t3, rup
+# CHECK-ASM: encoding: [0x53,0x3d,0x0e,0x42]
+fcvt.d.s x26, x28, rup
# CHECK-ASM-AND-OBJ: feq.d s10, t3, t5
# CHECK-ASM: encoding: [0x53,0x2d,0xee,0xa3]
feq.d x26, x28, x30
More information about the llvm-commits
mailing list