[RISCV] Move the even register check for rv32zdinx later in the matching process.

[craig.topper]

Summary

And remove the IsRV64 checks for isGPRAsFPR and isGPRPF64AsFPR.

Overall I think this results in a better diagnostic experience. We
now do a better job of matching Zdinx instructions even if the registers
aren't correct and report an error for missing features like RV64.

Unfortunately, this makes it difficult to recover the error location
for the invalid odd register when we do report it. But to make up
for it, I gave a more specific error message.

It doesn't look like binutils gives any warning or error for even registers.

[asb]

LGTM. A better error message but less precise error location seems like a fair trade-off to me.

[jrtc27]

Hm, teaching AsmMatcherEmitter to pass ErrorInfo into checkTargetMatchPredicate (and I guess checkEarlyTargetMatchPredicate for completeness) looks pretty straightforward and would restore the ability to point at the operand's loc

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

377–379

Is there a reason we'd want to keep these two and not just use isGPRAsFPR everywhere?

[craig.topper]

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

377–379

No. I'm going to do another pass through this as a follow up. My focus was on getting rid of IsRV64 on register operands.

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

 //===-- RISCVAsmParser.cpp - Parse RISCV assembly to MCInst instructions --===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
RISCVTargetStreamer &getTargetStreamer() {
     assert(getParser().getStreamer().getTargetStreamer() &&
            "do not have a target streamer");
     MCTargetStreamer &TS = *getParser().getStreamer().getTargetStreamer();
     return static_cast<RISCVTargetStreamer &>(TS);
   }
 
   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, Twine Msg);
 
   bool MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
                                OperandVector &Operands, MCStreamer &Out,
                                uint64_t &ErrorInfo,
                                bool MatchingInlineAsm) override;
#include "RISCVGenAsmMatcher.inc"
   }
 
   std::unique_ptr<RISCVOperand> defaultMaskRegOp() const;
   std::unique_ptr<RISCVOperand> defaultFRMArgOp() const;
 
 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
   };
 
   static bool classifySymbolRef(const MCExpr *Expr,
                                 RISCVMCExpr::VariantKind &Kind);
 
enum class KindTy {
     SystemRegister,
     VType,
     FRM,
     Fence,
   } Kind;
 
   struct RegOp {
     MCRegister RegNum;
-    bool IsRV64;
     bool IsGPRAsFPR;
   };
 
   struct ImmOp {
     const MCExpr *Val;
     bool IsRV64;
   };
 
public:
 
   bool isGPR() const {
     return Kind == KindTy::Register &&
            RISCVMCRegisterClasses[RISCV::GPRRegClassID].contains(Reg.RegNum);
   }
 
   bool isGPRAsFPR() const { return isGPR() && Reg.IsGPRAsFPR; }
 
-  bool isGPRF64AsFPR() const { return isGPR() && Reg.IsGPRAsFPR && Reg.IsRV64; }
+  bool isGPRF64AsFPR() const { return isGPR() && Reg.IsGPRAsFPR; }
 
-  bool isGPRPF64AsFPR() const {
-    return isGPR() && Reg.IsGPRAsFPR && !Reg.IsRV64 &&
-           !((Reg.RegNum - RISCV::X0) & 1);
-  }
+  bool isGPRPF64AsFPR() const { return isGPR() && Reg.IsGPRAsFPR; }
 
   static bool evaluateConstantImm(const MCExpr *Expr, int64_t &Imm,
                                   RISCVMCExpr::VariantKind &VK) {
     if (auto *RE = dyn_cast<RISCVMCExpr>(Expr)) {
       VK = RE->getKind();
       return RE->evaluateAsConstant(Imm);
     }
 
public:
   static std::unique_ptr<RISCVOperand> createToken(StringRef Str, SMLoc S) {
     auto Op = std::make_unique<RISCVOperand>(KindTy::Token);
     Op->Tok = Str;
     Op->StartLoc = S;
     Op->EndLoc = S;
     return Op;
   }
 
-  static std::unique_ptr<RISCVOperand> createReg(unsigned RegNo, SMLoc S,
-                                                 SMLoc E, bool IsRV64,
-                                                 bool IsGPRAsFPR = false) {
+  static std::unique_ptr<RISCVOperand>
+  createReg(unsigned RegNo, SMLoc S, SMLoc E, bool IsGPRAsFPR = false) {
     auto Op = std::make_unique<RISCVOperand>(KindTy::Register);
     Op->Reg.RegNum = RegNo;
-    Op->Reg.IsRV64 = IsRV64;
     Op->Reg.IsGPRAsFPR = IsGPRAsFPR;
     Op->StartLoc = S;
     Op->EndLoc = E;
     return Op;
   }
 
   static std::unique_ptr<RISCVOperand> createImm(const MCExpr *Val, SMLoc S,
                                                  SMLoc E, bool IsRV64) {
if (IsRegVR && (Kind == MCK_VRM2 || Kind == MCK_VRM4 || Kind == MCK_VRM8)) {
     Op.Reg.RegNum = convertVRToVRMx(*getContext().getRegisterInfo(), Reg, Kind);
     if (Op.Reg.RegNum == 0)
       return Match_InvalidOperand;
     return Match_Success;
   }
   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::GPRPF64RegClassID) {
+      const auto &Op = Inst.getOperand(I);
+      assert(Op.isReg());
+
+      MCRegister Reg = Op.getReg();
+      if (((Reg.id() - RISCV::X0) & 1) != 0)
+        return Match_RequiresEvenGPRs;
+    }
+  }
+
+  return Match_Success;
+}
+
 bool RISCVAsmParser::generateImmOutOfRangeError(
     OperandVector &Operands, uint64_t ErrorInfo, int64_t Lower, int64_t Upper,
     Twine Msg = "immediate must be an integer in the range") {
   SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
   return Error(ErrorLoc, Msg + " [" + Twine(Lower) + ", " + Twine(Upper) + "]");
 }
 
 bool RISCVAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
if (Result > FIRST_TARGET_MATCH_RESULT_TY) {
     SMLoc ErrorLoc = IDLoc;
     if (ErrorInfo != ~0ULL && ErrorInfo >= Operands.size())
       return Error(ErrorLoc, "too few operands for instruction");
   }
 
   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();
       return Error(ErrorLoc, "operand must be a constant 64-bit integer");
     }
     return generateImmOutOfRangeError(Operands, ErrorInfo,
                                       std::numeric_limits<int32_t>::min(),
                                       std::numeric_limits<uint32_t>::max());
if (RegNo == RISCV::NoRegister) {
         getLexer().UnLex(LParen);
       return MatchOperand_NoMatch;
     }
     if (HadParens)
       Operands.push_back(RISCVOperand::createToken("(", FirstS));
     SMLoc S = getLoc();
     SMLoc E = SMLoc::getFromPointer(S.getPointer() + Name.size());
     getLexer().Lex();
-    Operands.push_back(RISCVOperand::createReg(RegNo, S, E, isRV64()));
+    Operands.push_back(RISCVOperand::createReg(RegNo, S, E));
   }
 
   if (HadParens) {
     getParser().Lex(); // Eat ')'
     Operands.push_back(RISCVOperand::createToken(")", getLoc()));
   }
 
   return MatchOperand_Success;
OperandMatchResultTy RISCVAsmParser::parseMaskReg(OperandVector &Operands) {
 
   if (RegNo == RISCV::NoRegister)
     return MatchOperand_NoMatch;
   if (RegNo != RISCV::V0)
     return MatchOperand_NoMatch;
   SMLoc S = getLoc();
   SMLoc E = SMLoc::getFromPointer(S.getPointer() + Name.size());
   getLexer().Lex();
-  Operands.push_back(RISCVOperand::createReg(RegNo, S, E, isRV64()));
+  Operands.push_back(RISCVOperand::createReg(RegNo, S, E));
   return MatchOperand_Success;
 }
 
 OperandMatchResultTy RISCVAsmParser::parseGPRAsFPR(OperandVector &Operands) {
   if (getLexer().isNot(AsmToken::Identifier))
     return MatchOperand_NoMatch;
 
   StringRef Name = getLexer().getTok().getIdentifier();
   MCRegister RegNo;
   matchRegisterNameHelper(isRV32E(), RegNo, Name);
 
   if (RegNo == RISCV::NoRegister)
     return MatchOperand_NoMatch;
   SMLoc S = getLoc();
   SMLoc E = SMLoc::getFromPointer(S.getPointer() + Name.size());
   getLexer().Lex();
   Operands.push_back(RISCVOperand::createReg(
-      RegNo, S, E, isRV64(), !getSTI().hasFeature(RISCV::FeatureStdExtF)));
+      RegNo, S, E, !getSTI().hasFeature(RISCV::FeatureStdExtF)));
   return MatchOperand_Success;
 }
 
 OperandMatchResultTy RISCVAsmParser::parseFRMArg(OperandVector &Operands) {
   if (getLexer().isNot(AsmToken::Identifier)) {
     TokError("operand must be a valid floating point rounding mode mnemonic");
     return MatchOperand_ParseFail;
   }
return Error(ErrorLoc, "the second input operand must be tp/x4 when using "
                            "%tprel_add modifier");
   }
 
   return false;
 }
 
 std::unique_ptr<RISCVOperand> RISCVAsmParser::defaultMaskRegOp() const {
   return RISCVOperand::createReg(RISCV::NoRegister, llvm::SMLoc(),
-                                 llvm::SMLoc(), isRV64());
+                                 llvm::SMLoc());
 }
 
 std::unique_ptr<RISCVOperand> RISCVAsmParser::defaultFRMArgOp() const {
   return RISCVOperand::createFRMArg(RISCVFPRndMode::RoundingMode::DYN,
                                     llvm::SMLoc());
 }
 
 bool RISCVAsmParser::validateInstruction(MCInst &Inst,

llvm/test/MC/RISCV/rv32zdinx-invalid.s

 # 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]]:12: error: invalid operand for instruction
+fadd.d a0, a1, a2 # CHECK: :[[@LINE]]:1: 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){{$}}
 fadd.d fa0, fa1, fa2 # CHECK: :[[@LINE]]:1: error: instruction requires the following: 'D' (Double-Precision Floating-Point){{$}}
 
 # Invalid instructions
 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

llvm/test/MC/RISCV/rv32zfinx-invalid.s

 fmadd.s x10, x11, x12, ree # CHECK: :[[@LINE]]:24: error: invalid operand for instruction
 
 # Invalid rounding modes
 fmadd.s x10, x11, x12, x13, ree # CHECK: :[[@LINE]]:29: error: operand must be a valid floating point rounding mode mnemonic
 fmsub.s x14, x15, x16, x17, 0 # CHECK: :[[@LINE]]:29: error: operand must be a valid floating point rounding mode mnemonic
 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]]:8: error: invalid operand for instruction
+fadd.d t0, t1, t2 # CHECK: :[[@LINE]]:1: error: instruction requires the following: 'Zdinx' (Double in Integer){{$}}

llvm/test/MC/RISCV/rv64zdinx-valid.s

 # RUN: llvm-mc %s -triple=riscv64 -mattr=+zdinx -riscv-no-aliases -show-encoding \
 # RUN:     | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
 # RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+zdinx %s \
 # RUN:     | llvm-objdump --mattr=+zdinx -M no-aliases -d -r - \
 # RUN:     | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
 #
 # RUN: not llvm-mc -triple riscv32 -mattr=+zdinx %s 2>&1 \
 # RUN:     | FileCheck -check-prefix=CHECK-RV32 %s
 
 # CHECK-ASM-AND-OBJ: fcvt.l.d a0, t0, dyn
 # CHECK-ASM: encoding: [0x53,0xf5,0x22,0xc2]
-# CHECK-RV32: :[[#@LINE+1]]:14: error: invalid operand for instruction
+# CHECK-RV32: :[[#@LINE+1]]:1: error: instruction requires the following: RV64I Base Instruction Set
 fcvt.l.d a0, t0, dyn
 # CHECK-ASM-AND-OBJ: fcvt.lu.d a1, t1, dyn
 # CHECK-ASM: encoding: [0xd3,0x75,0x33,0xc2]
-# CHECK-RV32: :[[#@LINE+1]]:15: error: invalid operand for instruction
+# CHECK-RV32: :[[#@LINE+1]]:1: error: instruction requires the following: RV64I Base Instruction Set
 fcvt.lu.d a1, t1, dyn
 # CHECK-ASM-AND-OBJ: fcvt.d.l t3, a3, dyn
 # CHECK-ASM: encoding: [0x53,0xfe,0x26,0xd2]
-# CHECK-RV32: :[[#@LINE+1]]:10: error: invalid operand for instruction
+# CHECK-RV32: :[[#@LINE+1]]:1: error: instruction requires the following: RV64I Base Instruction Set
 fcvt.d.l t3, a3, dyn
 # CHECK-ASM-AND-OBJ: fcvt.d.lu t4, a4, dyn
 # CHECK-ASM: encoding: [0xd3,0x7e,0x37,0xd2]
-# CHECK-RV32: :[[#@LINE+1]]:11: error: invalid operand for instruction
+# CHECK-RV32: :[[#@LINE+1]]:1: error: instruction requires the following: RV64I Base Instruction Set
 fcvt.d.lu t4, a4, dyn
 
 # Rounding modes
 # CHECK-ASM-AND-OBJ: fcvt.d.l t3, a3, rne
 # CHECK-ASM: encoding: [0x53,0x8e,0x26,0xd2]
-# CHECK-RV32: :[[#@LINE+1]]:10: error: invalid operand for instruction
+# CHECK-RV32: :[[#@LINE+1]]:1: error: instruction requires the following: RV64I Base Instruction Set
 fcvt.d.l t3, a3, rne
 # CHECK-ASM-AND-OBJ: fcvt.d.lu t4, a4, rtz
 # CHECK-ASM: encoding: [0xd3,0x1e,0x37,0xd2]
-# CHECK-RV32: :[[#@LINE+1]]:11: error: invalid operand for instruction
+# CHECK-RV32: :[[#@LINE+1]]:1: error: instruction requires the following: RV64I Base Instruction Set
 fcvt.d.lu t4, a4, rtz
 # CHECK-ASM-AND-OBJ: fcvt.l.d a0, t0, rdn
 # CHECK-ASM: encoding: [0x53,0xa5,0x22,0xc2]
-# CHECK-RV32: :[[#@LINE+1]]:14: error: invalid operand for instruction
+# CHECK-RV32: :[[#@LINE+1]]:1: error: instruction requires the following: RV64I Base Instruction Set
 fcvt.l.d a0, t0, rdn
 # CHECK-ASM-AND-OBJ: fcvt.lu.d a1, t1, rup
 # CHECK-ASM: encoding: [0xd3,0x35,0x33,0xc2]
-# CHECK-RV32: :[[#@LINE+1]]:15: error: invalid operand for instruction
+# CHECK-RV32: :[[#@LINE+1]]:1: error: instruction requires the following: RV64I Base Instruction Set
 fcvt.lu.d a1, t1, rup