From 5537ae87b3a87b3abeb4e6983cecd9b103648243 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Tue, 10 Sep 2024 11:44:04 -0700 Subject: [PATCH] [RISCV] Fix fneg.d/fabs.d aliasing handling for Zdinx. Add missing fmv.s/d aliases. We were missing test coverage for fneg.d/fabs.d for Zdinx. When I added it revealed it only worked on RV64. The assembler was not creating a GPRPair register class on RV32 so the alias couldn't match. The disassembler was also not using GPRPair registers preventing the aliases from printing in disassembly too. I've fixed the assembler by adding new parsing methods in an attempt to get decent diagnostics. This is hard since the mnemonics are ambiguous between D and Zdinx. Tests have been adjusted for some differences in what errors are reported first. --- .../Target/RISCV/AsmParser/RISCVAsmParser.cpp | 84 +++++++++++++------ .../RISCV/Disassembler/RISCVDisassembler.cpp | 9 +- llvm/lib/Target/RISCV/RISCVInstrInfoD.td | 8 +- llvm/lib/Target/RISCV/RISCVInstrInfoF.td | 1 + llvm/test/MC/RISCV/rv32zdinx-invalid.s | 8 +- llvm/test/MC/RISCV/rv32zfinx-invalid.s | 7 +- llvm/test/MC/RISCV/rvzdinx-aliases-valid.s | 21 +++++ llvm/test/MC/RISCV/rvzfinx-aliases-valid.s | 3 + llvm/test/MC/RISCV/rvzhinx-aliases-valid.s | 3 + 9 files changed, 106 insertions(+), 38 deletions(-) diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp index 6d33a1f64195d5..6eb2058107610e 100644 --- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp +++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp @@ -97,7 +97,6 @@ class RISCVAsmParser : public MCTargetAsmParser { unsigned validateTargetOperandClass(MCParsedAsmOperand &Op, unsigned Kind) override; - unsigned checkTargetMatchPredicate(MCInst &Inst) override; bool generateImmOutOfRangeError(OperandVector &Operands, uint64_t ErrorInfo, int64_t Lower, int64_t Upper, @@ -209,6 +208,8 @@ class RISCVAsmParser : public MCTargetAsmParser { ParseStatus parseInsnDirectiveOpcode(OperandVector &Operands); ParseStatus parseInsnCDirectiveOpcode(OperandVector &Operands); ParseStatus parseGPRAsFPR(OperandVector &Operands); + ParseStatus parseGPRAsFPR64(OperandVector &Operands); + ParseStatus parseGPRPairAsFPR64(OperandVector &Operands); template ParseStatus parseGPRPair(OperandVector &Operands); ParseStatus parseGPRPair(OperandVector &Operands, bool IsRV64Inst); ParseStatus parseFRMArg(OperandVector &Operands); @@ -280,7 +281,6 @@ class RISCVAsmParser : public MCTargetAsmParser { public: enum RISCVMatchResultTy { Match_Dummy = FIRST_TARGET_MATCH_RESULT_TY, - Match_RequiresEvenGPRs, #define GET_OPERAND_DIAGNOSTIC_TYPES #include "RISCVGenAsmMatcher.inc" #undef GET_OPERAND_DIAGNOSTIC_TYPES @@ -481,6 +481,7 @@ struct RISCVOperand final : public MCParsedAsmOperand { } bool isGPRAsFPR() const { return isGPR() && Reg.IsGPRAsFPR; } + bool isGPRPairAsFPR() const { return isGPRPair() && Reg.IsGPRAsFPR; } bool isGPRPair() const { return Kind == KindTy::Register && @@ -1341,6 +1342,16 @@ unsigned RISCVAsmParser::validateTargetOperandClass(MCParsedAsmOperand &AsmOp, Op.Reg.RegNum = convertFPR64ToFPR16(Reg); return Match_Success; } + + // There are some GPRF64AsFPR instructions that have no RV32 equivalent. We + // reject them at parsing thinking we should match as GPRPairAsFPR for RV32. + // So we explicitly accept them here for RV32 to allow the generic code to + // report that the instruction requires RV64. + if (RISCVMCRegisterClasses[RISCV::GPRRegClassID].contains(Reg) && + Kind == MCK_GPRF64AsFPR && STI->hasFeature(RISCV::FeatureStdExtZdinx) && + !isRV64()) + return Match_Success; + // As the parser couldn't differentiate an VRM2/VRM4/VRM8 from an VR, coerce // the register from VR to VRM2/VRM4/VRM8 if necessary. if (IsRegVR && (Kind == MCK_VRM2 || Kind == MCK_VRM4 || Kind == MCK_VRM8)) { @@ -1352,27 +1363,6 @@ unsigned RISCVAsmParser::validateTargetOperandClass(MCParsedAsmOperand &AsmOp, return Match_InvalidOperand; } -unsigned RISCVAsmParser::checkTargetMatchPredicate(MCInst &Inst) { - const MCInstrDesc &MCID = MII.get(Inst.getOpcode()); - - for (unsigned I = 0; I < MCID.NumOperands; ++I) { - if (MCID.operands()[I].RegClass == RISCV::GPRPairRegClassID) { - const auto &Op = Inst.getOperand(I); - assert(Op.isReg()); - - MCRegister Reg = Op.getReg(); - if (RISCVMCRegisterClasses[RISCV::GPRPairRegClassID].contains(Reg)) - continue; - - // FIXME: We should form a paired register during parsing/matching. - if (((Reg.id() - RISCV::X0) & 1) != 0) - return Match_RequiresEvenGPRs; - } - } - - return Match_Success; -} - bool RISCVAsmParser::generateImmOutOfRangeError( SMLoc ErrorLoc, int64_t Lower, int64_t Upper, const Twine &Msg = "immediate must be an integer in the range") { @@ -1448,10 +1438,6 @@ bool RISCVAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode, switch (Result) { default: break; - case Match_RequiresEvenGPRs: - return Error(IDLoc, - "double precision floating point operands must use even " - "numbered X register"); case Match_InvalidImmXLenLI: if (isRV64()) { SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc(); @@ -2318,6 +2304,13 @@ ParseStatus RISCVAsmParser::parseMaskReg(OperandVector &Operands) { return ParseStatus::Success; } +ParseStatus RISCVAsmParser::parseGPRAsFPR64(OperandVector &Operands) { + if (!isRV64() || getSTI().hasFeature(RISCV::FeatureStdExtF)) + return ParseStatus::NoMatch; + + return parseGPRAsFPR(Operands); +} + ParseStatus RISCVAsmParser::parseGPRAsFPR(OperandVector &Operands) { if (getLexer().isNot(AsmToken::Identifier)) return ParseStatus::NoMatch; @@ -2335,6 +2328,43 @@ ParseStatus RISCVAsmParser::parseGPRAsFPR(OperandVector &Operands) { return ParseStatus::Success; } +ParseStatus RISCVAsmParser::parseGPRPairAsFPR64(OperandVector &Operands) { + if (isRV64() || getSTI().hasFeature(RISCV::FeatureStdExtF)) + return ParseStatus::NoMatch; + + if (getLexer().isNot(AsmToken::Identifier)) + return ParseStatus::NoMatch; + + StringRef Name = getLexer().getTok().getIdentifier(); + MCRegister Reg = matchRegisterNameHelper(Name); + + if (!Reg) + return ParseStatus::NoMatch; + + if (!RISCVMCRegisterClasses[RISCV::GPRRegClassID].contains(Reg)) + return ParseStatus::NoMatch; + + if ((Reg - RISCV::X0) & 1) { + // Only report the even register error if we have at least Zfinx so we know + // some FP is enabled. We already checked F earlier. + if (getSTI().hasFeature(RISCV::FeatureStdExtZfinx)) + return TokError("double precision floating point operands must use even " + "numbered X register"); + return ParseStatus::NoMatch; + } + + SMLoc S = getLoc(); + SMLoc E = SMLoc::getFromPointer(S.getPointer() + Name.size()); + getLexer().Lex(); + + const MCRegisterInfo *RI = getContext().getRegisterInfo(); + MCRegister Pair = RI->getMatchingSuperReg( + Reg, RISCV::sub_gpr_even, + &RISCVMCRegisterClasses[RISCV::GPRPairRegClassID]); + Operands.push_back(RISCVOperand::createReg(Pair, S, E, /*isGPRAsFPR=*/true)); + return ParseStatus::Success; +} + template ParseStatus RISCVAsmParser::parseGPRPair(OperandVector &Operands) { return parseGPRPair(Operands, IsRV64); diff --git a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp index 23897e2d98f634..b869458a256147 100644 --- a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp +++ b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp @@ -181,10 +181,15 @@ static DecodeStatus DecodeGPRCRegisterClass(MCInst &Inst, uint32_t RegNo, static DecodeStatus DecodeGPRPairRegisterClass(MCInst &Inst, uint32_t RegNo, uint64_t Address, const MCDisassembler *Decoder) { - if (RegNo >= 32 || RegNo & 1) + if (RegNo >= 32 || RegNo % 2) return MCDisassembler::Fail; - MCRegister Reg = RISCV::X0 + RegNo; + const RISCVDisassembler *Dis = + static_cast(Decoder); + const MCRegisterInfo *RI = Dis->getContext().getRegisterInfo(); + MCRegister Reg = RI->getMatchingSuperReg( + RISCV::X0 + RegNo, RISCV::sub_gpr_even, + &RISCVMCRegisterClasses[RISCV::GPRPairRegClassID]); Inst.addOperand(MCOperand::createReg(Reg)); return MCDisassembler::Success; } diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoD.td b/llvm/lib/Target/RISCV/RISCVInstrInfoD.td index ed0ad27ac9d29f..383a2fdede8364 100644 --- a/llvm/lib/Target/RISCV/RISCVInstrInfoD.td +++ b/llvm/lib/Target/RISCV/RISCVInstrInfoD.td @@ -35,15 +35,15 @@ def AddrRegImmINX : ComplexPattern; def GPRPairAsFPR : AsmOperandClass { let Name = "GPRPairAsFPR"; - let ParserMethod = "parseGPRAsFPR"; - let PredicateMethod = "isGPRAsFPR"; + let ParserMethod = "parseGPRPairAsFPR64"; + let PredicateMethod = "isGPRPairAsFPR"; let RenderMethod = "addRegOperands"; } def GPRF64AsFPR : AsmOperandClass { let Name = "GPRF64AsFPR"; let PredicateMethod = "isGPRAsFPR"; - let ParserMethod = "parseGPRAsFPR"; + let ParserMethod = "parseGPRAsFPR64"; let RenderMethod = "addRegOperands"; } @@ -205,6 +205,7 @@ def PseudoQuietFLT_D : PseudoQuietFCMP; } // Predicates = [HasStdExtD] let Predicates = [HasStdExtZdinx, IsRV64] in { +def : InstAlias<"fmv.d $rd, $rs", (FSGNJ_D_INX FPR64INX:$rd, FPR64INX:$rs, FPR64INX:$rs)>; def : InstAlias<"fabs.d $rd, $rs", (FSGNJX_D_INX FPR64INX:$rd, FPR64INX:$rs, FPR64INX:$rs)>; def : InstAlias<"fneg.d $rd, $rs", (FSGNJN_D_INX FPR64INX:$rd, FPR64INX:$rs, FPR64INX:$rs)>; @@ -219,6 +220,7 @@ def PseudoQuietFLT_D_INX : PseudoQuietFCMP; } // Predicates = [HasStdExtZdinx, IsRV64] let Predicates = [HasStdExtZdinx, IsRV32] in { +def : InstAlias<"fmv.d $rd, $rs", (FSGNJ_D_IN32X FPR64IN32X:$rd, FPR64IN32X:$rs, FPR64IN32X:$rs)>; def : InstAlias<"fabs.d $rd, $rs", (FSGNJX_D_IN32X FPR64IN32X:$rd, FPR64IN32X:$rs, FPR64IN32X:$rs)>; def : InstAlias<"fneg.d $rd, $rs", (FSGNJN_D_IN32X FPR64IN32X:$rd, FPR64IN32X:$rs, FPR64IN32X:$rs)>; diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoF.td b/llvm/lib/Target/RISCV/RISCVInstrInfoF.td index 7e5f96edcdbda0..1442bc1cbc4feb 100644 --- a/llvm/lib/Target/RISCV/RISCVInstrInfoF.td +++ b/llvm/lib/Target/RISCV/RISCVInstrInfoF.td @@ -454,6 +454,7 @@ def PseudoQuietFLT_S : PseudoQuietFCMP; } // Predicates = [HasStdExtF] let Predicates = [HasStdExtZfinx] in { +def : InstAlias<"fmv.s $rd, $rs", (FSGNJ_S_INX FPR32INX:$rd, FPR32INX:$rs, FPR32INX:$rs)>; def : InstAlias<"fabs.s $rd, $rs", (FSGNJX_S_INX FPR32INX:$rd, FPR32INX:$rs, FPR32INX:$rs)>; def : InstAlias<"fneg.s $rd, $rs", (FSGNJN_S_INX FPR32INX:$rd, FPR32INX:$rs, FPR32INX:$rs)>; diff --git a/llvm/test/MC/RISCV/rv32zdinx-invalid.s b/llvm/test/MC/RISCV/rv32zdinx-invalid.s index 879157a08d60fb..e79caa074d255c 100644 --- a/llvm/test/MC/RISCV/rv32zdinx-invalid.s +++ b/llvm/test/MC/RISCV/rv32zdinx-invalid.s @@ -1,7 +1,7 @@ # RUN: not llvm-mc -triple riscv32 -mattr=+zdinx %s 2>&1 | FileCheck %s # Unsupport Odd Registers in RV32 -fadd.d a0, a1, a2 # CHECK: :[[@LINE]]:1: error: double precision floating point operands must use even numbered X register +fadd.d a0, a1, a2 # CHECK: :[[@LINE]]:12: error: double precision floating point operands must use even numbered X register # Not support float registers flw fa4, 12(sp) # CHECK: :[[@LINE]]:1: error: instruction requires the following: 'F' (Single-Precision Floating-Point){{$}} @@ -12,8 +12,8 @@ fsw a5, 12(sp) # CHECK: :[[@LINE]]:5: error: invalid operand for instruction fmv.x.w s0, s1 # CHECK: :[[@LINE]]:13: error: invalid operand for instruction # Invalid register names -fadd.d a100, a2, a3 # CHECK: :[[@LINE]]:8: error: invalid operand for instruction -fsgnjn.d a100, a2, a3 # CHECK: :[[@LINE]]:10: error: invalid operand for instruction +fadd.d a100, a2, a4 # CHECK: :[[@LINE]]:8: error: invalid operand for instruction +fsgnjn.d a100, a2, a4 # CHECK: :[[@LINE]]:10: error: invalid operand for instruction # Rounding mode when a register is expected fmadd.d x10, x12, x14, ree # CHECK: :[[@LINE]]:24: error: invalid operand for instruction @@ -24,4 +24,4 @@ fmsub.d x10, x12, x14, x16, 0 # CHECK: :[[@LINE]]:29: error: operand must be a v fnmsub.d x10, x12, x14, x16, 0b111 # CHECK: :[[@LINE]]:30: error: operand must be a valid floating point rounding mode mnemonic # FP registers where integer regs are expected -fcvt.wu.d ft2, a1 # CHECK: :[[@LINE]]:11: error: invalid operand for instruction +fcvt.wu.d ft2, a2 # CHECK: :[[@LINE]]:11: error: invalid operand for instruction diff --git a/llvm/test/MC/RISCV/rv32zfinx-invalid.s b/llvm/test/MC/RISCV/rv32zfinx-invalid.s index b4675f85fa6e5a..0cd1700b3363cf 100644 --- a/llvm/test/MC/RISCV/rv32zfinx-invalid.s +++ b/llvm/test/MC/RISCV/rv32zfinx-invalid.s @@ -10,7 +10,7 @@ fmv.x.w s0, s1 # CHECK: :[[@LINE]]:13: error: invalid operand for instruction fadd.d t1, t3, t5 # CHECK: :[[@LINE]]:1: error: instruction requires the following: 'Zdinx' (Double in Integer){{$}} # Invalid register names -fadd.d a100, a2, a3 # CHECK: :[[@LINE]]:8: error: invalid operand for instruction +fadd.s a100, a2, a3 # CHECK: :[[@LINE]]:8: error: invalid operand for instruction fsgnjn.s a100, a2, a3 # CHECK: :[[@LINE]]:10: error: invalid operand for instruction # Rounding mode when a register is expected @@ -22,4 +22,7 @@ fmsub.s x14, x15, x16, x17, 0 # CHECK: :[[@LINE]]:29: error: operand must be a v fnmsub.s x18, x19, x20, x21, 0b111 # CHECK: :[[@LINE]]:30: error: operand must be a valid floating point rounding mode mnemonic # Using 'Zdinx' instructions for an 'Zfinx'-only target -fadd.d t0, t1, t2 # CHECK: :[[@LINE]]:1: error: instruction requires the following: 'Zdinx' (Double in Integer){{$}} +# odd registers report incorrect registers +fadd.d t0, t1, t2 # CHECK: :[[@LINE]]:8: error: double precision floating point operands must use even numbered X register +# even registers report the required extension +fadd.d t3, t1, t5 # CHECK: :[[@LINE]]:1: error: instruction requires the following: 'Zdinx' (Double in Integer){{$}} diff --git a/llvm/test/MC/RISCV/rvzdinx-aliases-valid.s b/llvm/test/MC/RISCV/rvzdinx-aliases-valid.s index 3262c88e92816b..96ec4a4e1041ba 100644 --- a/llvm/test/MC/RISCV/rvzdinx-aliases-valid.s +++ b/llvm/test/MC/RISCV/rvzdinx-aliases-valid.s @@ -19,6 +19,27 @@ # RUN: | llvm-objdump -d --mattr=+zdinx - \ # RUN: | FileCheck -check-prefix=CHECK-ALIAS %s +##===----------------------------------------------------------------------===## +## Assembler Pseudo Instructions (User-Level ISA, Version 2.2, Chapter 20) +##===----------------------------------------------------------------------===## + +# CHECK-INST: fsgnj.d a0, a2, a2 +# CHECK-ALIAS: fmv.d a0, a2 +fmv.d a0, a2 +# CHECK-INST: fsgnjx.d a2, a4, a4 +# CHECK-ALIAS: fabs.d a2, a4 +fabs.d a2, a4 +# CHECK-INST: fsgnjn.d a4, a6, a6 +# CHECK-ALIAS: fneg.d a4, a6 +fneg.d a4, a6 + +# CHECK-INST: flt.d tp, s2, a6 +# CHECK-ALIAS: flt.d tp, s2, a6 +fgt.d x4, a6, s2 +# CHECK-INST: fle.d t2, s4, s2 +# CHECK-ALIAS: fle.d t2, s4, s2 +fge.d x7, s2, s4 + ##===----------------------------------------------------------------------===## ## Aliases which omit the rounding mode. ##===----------------------------------------------------------------------===## diff --git a/llvm/test/MC/RISCV/rvzfinx-aliases-valid.s b/llvm/test/MC/RISCV/rvzfinx-aliases-valid.s index 71e9977404d7a4..f624c17f78a67b 100644 --- a/llvm/test/MC/RISCV/rvzfinx-aliases-valid.s +++ b/llvm/test/MC/RISCV/rvzfinx-aliases-valid.s @@ -23,6 +23,9 @@ ## Assembler Pseudo Instructions (User-Level ISA, Version 2.2, Chapter 20) ##===----------------------------------------------------------------------===## +# CHECK-INST: fsgnj.s s1, s2, s2 +# CHECK-ALIAS: fmv.s s1, s2 +fmv.s s1, s2 # CHECK-INST: fsgnjx.s s1, s2, s2 # CHECK-ALIAS: fabs.s s1, s2 fabs.s s1, s2 diff --git a/llvm/test/MC/RISCV/rvzhinx-aliases-valid.s b/llvm/test/MC/RISCV/rvzhinx-aliases-valid.s index 9a328e44412447..dbefc5a91b2754 100644 --- a/llvm/test/MC/RISCV/rvzhinx-aliases-valid.s +++ b/llvm/test/MC/RISCV/rvzhinx-aliases-valid.s @@ -23,6 +23,9 @@ ## Assembler Pseudo Instructions (User-Level ISA, Version 2.2, Chapter 20) ##===----------------------------------------------------------------------===## +# CHECK-INST: fsgnj.h s1, s2, s2 +# CHECK-ALIAS: fmv.h s1, s2 +fmv.h s1, s2 # CHECK-INST: fsgnjx.h s1, s2, s2 # CHECK-ALIAS: fabs.h s1, s2 fabs.h s1, s2