[RISCV] Fix fneg.d/fabs.d aliasing handling for Zdinx. Add missing fmv.s/d aliases.

topperc · topperc · commit 5537ae87b3a8 · 2024-09-10T11:44:04.000-07:00
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.
diff --git 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 <bool IsRV64Inst> 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 <bool IsRV64>
 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
@@ -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<const RISCVDisassembler *>(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
@@ -35,15 +35,15 @@ def AddrRegImmINX : ComplexPattern<iPTR, 2, "SelectAddrRegImmRV32Zdinx">;
 
 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<FPR64>;
 } // 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<FPR64INX>;
 } // 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
@@ -454,6 +454,7 @@ def PseudoQuietFLT_S : PseudoQuietFCMP<FPR32>;
 } // 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
