[RISCV][MC] Recognise that fcvt.d.s with frm != 0b000 is valid (#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.
Recently added tests ensure that the default `rne` rounding mode is
printed as desired.
GitOrigin-RevId: b28d83eec44831e1c1dfcb861564a0beb74c7e9b
diff --git a/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index 7d8d82e..eb861cee 100644
--- a/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -259,6 +259,7 @@
std::unique_ptr<RISCVOperand> defaultMaskRegOp() const;
std::unique_ptr<RISCVOperand> defaultFRMArgOp() const;
+ std::unique_ptr<RISCVOperand> defaultFRMArgLegacyOp() const;
public:
enum RISCVMatchResultTy {
@@ -563,6 +564,7 @@
/// Return true if the operand is a valid floating point rounding mode.
bool isFRMArg() const { return Kind == KindTy::FRM; }
+ bool isFRMArgLegacy() const { return Kind == KindTy::FRM; }
bool isRTZArg() const { return isFRMArg() && FRM.FRM == RISCVFPRndMode::RTZ; }
/// Return true if the operand is a valid fli.s floating-point immediate.
@@ -3257,6 +3259,11 @@
llvm::SMLoc());
}
+std::unique_ptr<RISCVOperand> RISCVAsmParser::defaultFRMArgLegacyOp() const {
+ return RISCVOperand::createFRMArg(RISCVFPRndMode::RoundingMode::RNE,
+ llvm::SMLoc());
+}
+
bool RISCVAsmParser::validateInstruction(MCInst &Inst,
OperandVector &Operands) {
unsigned Opcode = Inst.getOpcode();
diff --git a/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp b/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
index 666b65d..e31c53e 100644
--- a/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
+++ b/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
@@ -158,6 +158,19 @@
O << ", " << RISCVFPRndMode::roundingModeToString(FRMArg);
}
+void RISCVInstPrinter::printFRMArgLegacy(const MCInst *MI, unsigned OpNo,
+ const MCSubtargetInfo &STI,
+ raw_ostream &O) {
+ auto FRMArg =
+ static_cast<RISCVFPRndMode::RoundingMode>(MI->getOperand(OpNo).getImm());
+ // Never print rounding mode if it's the default 'rne'. This ensures the
+ // output can still be parsed by older tools that erroneously failed to
+ // accept a rounding mode.
+ if (FRMArg == RISCVFPRndMode::RoundingMode::RNE)
+ return;
+ O << ", " << RISCVFPRndMode::roundingModeToString(FRMArg);
+}
+
void RISCVInstPrinter::printFPImmOperand(const MCInst *MI, unsigned OpNo,
const MCSubtargetInfo &STI,
raw_ostream &O) {
diff --git a/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.h b/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.h
index 20f12af..852d281 100644
--- a/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.h
+++ b/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.h
@@ -40,6 +40,8 @@
const MCSubtargetInfo &STI, raw_ostream &O);
void printFRMArg(const MCInst *MI, unsigned OpNo, const MCSubtargetInfo &STI,
raw_ostream &O);
+ void printFRMArgLegacy(const MCInst *MI, unsigned OpNo,
+ const MCSubtargetInfo &STI, raw_ostream &O);
void printFPImmOperand(const MCInst *MI, unsigned OpNo,
const MCSubtargetInfo &STI, raw_ostream &O);
void printZeroOffsetMemOp(const MCInst *MI, unsigned OpNo,
diff --git a/lib/Target/RISCV/RISCVInstrInfoD.td b/lib/Target/RISCV/RISCVInstrInfoD.td
index 7a79e3c..8911e18 100644
--- a/lib/Target/RISCV/RISCVInstrInfoD.td
+++ b/lib/Target/RISCV/RISCVInstrInfoD.td
@@ -115,8 +115,8 @@
Ext.PrimaryTy, "fcvt.s.d">,
Sched<[WriteFCvtF64ToF32, ReadFCvtF64ToF32]>;
- defm FCVT_D_S : FPUnaryOp_r_m<0b0100001, 0b00000, 0b000, Ext, Ext.PrimaryTy,
- Ext.F32Ty, "fcvt.d.s">,
+ defm FCVT_D_S : FPUnaryOp_r_frmlegacy_m<0b0100001, 0b00000, Ext, Ext.PrimaryTy,
+ Ext.F32Ty, "fcvt.d.s">,
Sched<[WriteFCvtF32ToF64, ReadFCvtF32ToF64]>;
let SchedRW = [WriteFCmp64, ReadFCmp64, ReadFCmp64] in {
@@ -240,7 +240,7 @@
// 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 +248,7 @@
// 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 +256,7 @@
// 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 +281,8 @@
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 +319,7 @@
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 +356,7 @@
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/lib/Target/RISCV/RISCVInstrInfoF.td b/lib/Target/RISCV/RISCVInstrInfoF.td
index 290c03d..6ff6e4e 100644
--- a/lib/Target/RISCV/RISCVInstrInfoF.td
+++ b/lib/Target/RISCV/RISCVInstrInfoF.td
@@ -132,6 +132,26 @@
let DecoderMethod = "decodeFRMArg";
}
+// Variants of the rounding mode operand that default to 'rne'. This is used
+// for historical/legacy reasons. fcvt functions where the rounding mode
+// doesn't affect the output originally always set it to 0b000 ('rne'). As old
+// versions of LLVM and GCC will fail to decode versions of these instructions
+// with the rounding mode set to something other than 'rne', we retain this
+// default.
+def FRMArgLegacy : AsmOperandClass {
+ let Name = "FRMArgLegacy";
+ let RenderMethod = "addFRMArgOperands";
+ let ParserMethod = "parseFRMArg";
+ let IsOptional = 1;
+ let DefaultMethod = "defaultFRMArgLegacyOp";
+}
+
+def frmarglegacy : Operand<XLenVT> {
+ let ParserMatchClass = FRMArgLegacy;
+ let PrintMethod = "printFRMArgLegacy";
+ let DecoderMethod = "decodeFRMArg";
+}
+
//===----------------------------------------------------------------------===//
// Instruction class templates
//===----------------------------------------------------------------------===//
@@ -227,6 +247,24 @@
}
let hasSideEffects = 0, mayLoad = 0, mayStore = 0, mayRaiseFPException = 1,
+ UseNamedOperandTable = 1, hasPostISelHook = 1 in
+class FPUnaryOp_r_frmlegacy<bits<7> funct7, bits<5> rs2val, DAGOperand rdty,
+ DAGOperand rs1ty, string opcodestr>
+ : RVInstRFrm<funct7, OPC_OP_FP, (outs rdty:$rd),
+ (ins rs1ty:$rs1, frmarglegacy:$frm), opcodestr,
+ "$rd, $rs1$frm"> {
+ let rs2 = rs2val;
+}
+multiclass FPUnaryOp_r_frmlegacy_m<bits<7> funct7, bits<5> rs2val,
+ ExtInfo Ext, DAGOperand rdty, DAGOperand rs1ty,
+ string opcodestr, list<Predicate> ExtraPreds = []> {
+ let Predicates = !listconcat(Ext.Predicates, ExtraPreds),
+ DecoderNamespace = Ext.Space in
+ def Ext.Suffix : FPUnaryOp_r_frmlegacy<funct7, rs2val, rdty, rs1ty,
+ opcodestr>;
+}
+
+let hasSideEffects = 0, mayLoad = 0, mayStore = 0, mayRaiseFPException = 1,
IsSignExtendingOpW = 1 in
class FPCmp_rr<bits<7> funct7, bits<3> funct3, string opcodestr,
DAGOperand rty, bit Commutable = 0>
diff --git a/test/MC/RISCV/fp-default-rounding-mode.s b/test/MC/RISCV/fp-default-rounding-mode.s
index dac8da6..dfd4598 100644
--- a/test/MC/RISCV/fp-default-rounding-mode.s
+++ b/test/MC/RISCV/fp-default-rounding-mode.s
@@ -72,10 +72,14 @@
# CHECK-ALIAS: fcvt.s.d fa0, fa0{{$}}
fcvt.s.d fa0, fa0
-# FIXME: fcvt.d.s should have a default rounding mode.
+# For historical reasons defaults to frm==0b000 (rne) but doesn't print this
+# default rounding mode.
# CHECK-INST: fcvt.d.s fa0, fa0{{$}}
# CHECK-ALIAS: fcvt.d.s fa0, fa0{{$}}
fcvt.d.s fa0, fa0
+# CHECK-INST: fcvt.d.s fa0, fa0{{$}}
+# CHECK-ALIAS: fcvt.d.s fa0, fa0{{$}}
+fcvt.d.s fa0, fa0, rne
# CHECK-INST: fcvt.w.d a0, fa0, dyn{{$}}
# CHECK-ALIAS: fcvt.w.d a0, fa0{{$}}
diff --git a/test/MC/RISCV/fp-inx-default-rounding-mode.s b/test/MC/RISCV/fp-inx-default-rounding-mode.s
index 23ac3a1..06b654e 100644
--- a/test/MC/RISCV/fp-inx-default-rounding-mode.s
+++ b/test/MC/RISCV/fp-inx-default-rounding-mode.s
@@ -75,10 +75,14 @@
# CHECK-ALIAS: fcvt.s.d a0, a0{{$}}
fcvt.s.d a0, a0
-# FIXME: fcvt.d.s should have a default rounding mode.
+# For historical reasons defaults to frm==0b000 (rne) but doesn't print this
+# default rounding mode.
# CHECK-INST: fcvt.d.s a0, a0{{$}}
# CHECK-ALIAS: fcvt.d.s a0, a0{{$}}
fcvt.d.s a0, a0
+# CHECK-INST: fcvt.d.s a0, a0{{$}}
+# CHECK-ALIAS: fcvt.d.s a0, a0{{$}}
+fcvt.d.s a0, a0, rne
# CHECK-INST: fcvt.w.d a0, a0, dyn{{$}}
# CHECK-ALIAS: fcvt.w.d a0, a0{{$}}
diff --git a/test/MC/RISCV/rv32d-valid.s b/test/MC/RISCV/rv32d-valid.s
index f6254eb..eb8e9c3 100644
--- a/test/MC/RISCV/rv32d-valid.s
+++ b/test/MC/RISCV/rv32d-valid.s
@@ -96,6 +96,9 @@
# 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/test/MC/RISCV/rv32zdinx-valid.s b/test/MC/RISCV/rv32zdinx-valid.s
index 660116b..b668bab 100644
--- a/test/MC/RISCV/rv32zdinx-valid.s
+++ b/test/MC/RISCV/rv32zdinx-valid.s
@@ -59,6 +59,9 @@
# 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
