[RISCV] Support immediate vtype of VSETVLI/VSETIVLI in asm parser

[benshi001]

This patch impements TODOs in https://reviews.llvm.org/D114581

[craig.topper]

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

1673

Does binutils only except literal integers or does it support expressions?

1677

There are no illegal values in the immediate form for vlmul or vsew. The intention was to allow the disassembler to print immediates for reserved encodings. Anything that the disassembler prints must be parseable.

We do need to make sure the immedaite fits in 11 bits for vsetvli and 10 bits for vsetivli.

[benshi001]

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

1673

I have tested the newest master of binutils,

1. it does not support vtype being an expression, in fact there is no expression cases in binutils's unit tests.

2. it still generate wrong dissambly against illegal vtype, for example, larger types such as e128

[craig.topper]

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

1673

Our internal version of bintuils does accept this vsetvli x0, x0, 0xff + 1. I haven't checked binutils compiled with public sources, but I expect they would be the same.

The disassembly bugs have been fixed in our internal branch of binutils. I'll find out when they'll be fixed in publicly available sources.

[Miss_Grape]

Support non-canonical syntax：

1. vsetivli rd, uimm, 0xabc or vsetvli rd, rs1, 0xabc
2. vsetivli rd, uimm, (0xc << N) or vsetvli rd, rs1, (0xc << N)

[Miss_Grape]

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

1673

The latest patch already supports this representation.

[craig.topper]

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

432

Capitalize 'if'

433

Immedidate -> Immediate

435

Drop curly braces

441

Drop curly braces

llvm/test/MC/RISCV/rvv/vsetvl.s

19

Why 2 spaces between each operand?

[Miss_Grape]

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

432

Done

433

Done

435

Done

441

Done

llvm/test/MC/RISCV/rvv/vsetvl.s

19

Done

[craig.topper]

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

423–443

This doesn't need to be a template. Just pass the bitwidth and use isUIntN instead of isUint

927

auto *CE Don't hide the * with auto.

[Miss_Grape]

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

423–443

Done

927

Done

[craig.topper]

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

927

I think we need to call evaluateConstantImm here. Since we used evaluateConstantImm in isVTypeImm which can find an immediate that isn't from an MCConstantExpr. I think we should assert that evaluateConstantImm returns true since we checked that in isVTypeImm.

[Miss_Grape]

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

927

Done.

[Miss_Grape]

ping

[craig.topper]

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

928

You can't call evaluateConstantImm from inside the assert. The contents of the assert aren't evaluated in release builds. You'll need to use a variable to capture the result. And cast the variable to void to avoid an unused variable warning in release builds.

[Miss_Grape]

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

928

done

[Miss_Grape]

clang-format

[craig.topper]

LGTM other than trying to use RenderMethod.

llvm/lib/Target/RISCV/RISCVInstrInfoV.td

25–26

I think you can add

let RenderMethod = "addVTypeIOperands";

And avoid needing addVTypeI10Operands and addVTypeI11Operands in the assembler

[benshi001]

llvm/lib/Target/RISCV/RISCVInstrInfoV.td

25–26

Thanks for your review, I will commit this patch for her. And will correct this when when doing git commit and git push.

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

public:
   bool isToken() const override { return Kind == KindTy::Token; }
   bool isReg() const override { return Kind == KindTy::Register; }
   bool isV0Reg() const {
     return Kind == KindTy::Register && Reg.RegNum == RISCV::V0;
   }
   bool isImm() const override { return Kind == KindTy::Immediate; }
   bool isMem() const override { return false; }
   bool isSystemRegister() const { return Kind == KindTy::SystemRegister; }
-  bool isVType() const { return Kind == KindTy::VType; }
 
   bool isGPR() const {
     return Kind == KindTy::Register &&
            RISCVMCRegisterClasses[RISCV::GPRRegClassID].contains(Reg.RegNum);
   }
 
   static bool evaluateConstantImm(const MCExpr *Expr, int64_t &Imm,
                                   RISCVMCExpr::VariantKind &VK) {
bool isTPRelAddSymbol() const {
     if (!isImm() || evaluateConstantImm(getImm(), Imm, VK))
       return false;
     return RISCVAsmParser::classifySymbolRef(getImm(), VK) &&
            VK == RISCVMCExpr::VK_RISCV_TPREL_ADD;
   }
 
   bool isCSRSystemRegister() const { return isSystemRegister(); }
 
-  bool isVTypeI() const { return isVType(); }
+  bool isVTypeImm(unsigned N) const {
+    int64_t Imm;
+    RISCVMCExpr::VariantKind VK = RISCVMCExpr::VK_RISCV_None;
+    if (!isImm())
+      return false;
+    bool IsConstantImm = evaluateConstantImm(getImm(), Imm, VK);
+    return IsConstantImm && isUIntN(N, Imm) && VK == RISCVMCExpr::VK_RISCV_None;
+  }
+
+  // If the last operand of the vsetvli/vsetvli instruction is a constant
+  // expression, KindTy is Immediate.
+  bool isVTypeI10() const {
+    if (Kind == KindTy::Immediate)
+      return isVTypeImm(10);
+    return Kind == KindTy::VType;
+  }
+  bool isVTypeI11() const {
+    if (Kind == KindTy::Immediate)
+      return isVTypeImm(11);
+    return Kind == KindTy::VType;
+  }
 
   /// Return true if the operand is a valid for the fence instruction e.g.
   /// ('iorw').
   bool isFenceArg() const {
     if (!isImm())
       return false;
     const MCExpr *Val = getImm();
     auto *SVal = dyn_cast<MCSymbolRefExpr>(Val);
void addFenceArgOperands(MCInst &Inst, unsigned N) const {
     Inst.addOperand(MCOperand::createImm(Imm));
   }
 
   void addCSRSystemRegisterOperands(MCInst &Inst, unsigned N) const {
     assert(N == 1 && "Invalid number of operands!");
     Inst.addOperand(MCOperand::createImm(SysReg.Encoding));
   }
 
+  // Support non-canonical syntax:
+  // "vsetivli rd, uimm, 0xabc" or "vsetvli rd, rs1, 0xabc"
+  // "vsetivli rd, uimm, (0xc << N)" or "vsetvli rd, rs1, (0xc << N)"
   void addVTypeIOperands(MCInst &Inst, unsigned N) const {
     assert(N == 1 && "Invalid number of operands!");
-    Inst.addOperand(MCOperand::createImm(getVType()));
+    int64_t Imm = 0;
+    if (Kind == KindTy::Immediate) {
+      RISCVMCExpr::VariantKind VK = RISCVMCExpr::VK_RISCV_None;
+      bool IsConstantImm = evaluateConstantImm(getImm(), Imm, VK);
+      (void)IsConstantImm;
+      assert(IsConstantImm && "Invalid VTypeI Operand!");
+    } else {
+      Imm = getVType();
+    }
+    Inst.addOperand(MCOperand::createImm(Imm));
   }
 
   // Returns the rounding mode represented by this RISCVOperand. Should only
   // be called after checking isFRMArg.
   RISCVFPRndMode::RoundingMode getRoundingMode() const {
     // isFRMArg has validated the operand, meaning this cast is safe.
     auto SE = cast<MCSymbolRefExpr>(getImm());
     RISCVFPRndMode::RoundingMode FRM =
OperandMatchResultTy RISCVAsmParser::parseVTypeI(OperandVector &Operands) {
   SMLoc S = getLoc();
   if (getLexer().isNot(AsmToken::Identifier))
     return MatchOperand_NoMatch;
 
   SmallVector<AsmToken, 7> VTypeIElements;
   // Put all the tokens for vtypei operand into VTypeIElements vector.
   while (getLexer().isNot(AsmToken::EndOfStatement)) {
     VTypeIElements.push_back(getLexer().getTok());
     getLexer().Lex();
     if (getLexer().is(AsmToken::EndOfStatement))
       break;
     if (getLexer().isNot(AsmToken::Comma))
       goto MatchFail;
     AsmToken Comma = getLexer().getTok();
     VTypeIElements.push_back(Comma);
     getLexer().Lex();
   }
 
   if (VTypeIElements.size() == 7) {
     // The VTypeIElements layout is:
     // SEW comma LMUL comma TA comma MA

llvm/lib/Target/RISCV/RISCVInstrInfoV.td

 //===----------------------------------------------------------------------===//
 
 include "RISCVInstrFormatsV.td"
 
 //===----------------------------------------------------------------------===//
 // Operand and SDNode transformation definitions.
 //===----------------------------------------------------------------------===//
 
-def VTypeIAsmOperand : AsmOperandClass {
-  let Name = "VTypeI";
+class VTypeIAsmOperand<int VTypeINum> : AsmOperandClass {
+  let Name = "VTypeI" # VTypeINum;
   let ParserMethod = "parseVTypeI";
   let DiagnosticType = "InvalidVTypeI";
+  let RenderMethod = "addVTypeIOperands";
 }
 
-def VTypeIOp : Operand<XLenVT> {
-  let ParserMatchClass = VTypeIAsmOperand;
+class VTypeIOp<int VTypeINum> : Operand<XLenVT> {
+  let ParserMatchClass = VTypeIAsmOperand<VTypeINum>;
   let PrintMethod = "printVTypeI";
-  let DecoderMethod = "decodeUImmOperand<11>";
+  let DecoderMethod = "decodeUImmOperand<"#VTypeINum#">";
 }
 
+def VTypeIOp10 : VTypeIOp<10>;
+def VTypeIOp11 : VTypeIOp<11>;
+
 def VMaskAsmOperand : AsmOperandClass {
   let Name = "RVVMaskRegOpOperand";
   let RenderMethod = "addRegOperands";
   let PredicateMethod = "isV0Reg";
   let ParserMethod = "parseMaskReg";
   let IsOptional = 1;
   let DefaultMethod = "defaultMaskRegOp";
   let DiagnosticType = "InvalidVMaskRegister";

 }
 
 //===----------------------------------------------------------------------===//
 // Instructions
 //===----------------------------------------------------------------------===//
 
 let Predicates = [HasStdExtV] in {
 let hasSideEffects = 1, mayLoad = 0, mayStore = 0 in {
-def VSETVLI : RVInstSetVLi<(outs GPR:$rd), (ins GPR:$rs1, VTypeIOp:$vtypei),
+def VSETVLI : RVInstSetVLi<(outs GPR:$rd), (ins GPR:$rs1, VTypeIOp11:$vtypei),
                            "vsetvli", "$rd, $rs1, $vtypei">;
 
-def VSETIVLI : RVInstSetiVLi<(outs GPR:$rd), (ins uimm5:$uimm, VTypeIOp:$vtypei),
+def VSETIVLI : RVInstSetiVLi<(outs GPR:$rd), (ins uimm5:$uimm, VTypeIOp10:$vtypei),
                              "vsetivli", "$rd, $uimm, $vtypei">;
 
 def VSETVL : RVInstSetVL<(outs GPR:$rd), (ins GPR:$rs1, GPR:$rs2),
                          "vsetvl", "$rd, $rs1, $rs2">;
 } // hasSideEffects = 1, mayLoad = 0, mayStore = 0
 foreach eew = [8, 16, 32, 64] in {
   defvar w = !cast<RISCVWidth>("LSWidth" # eew);
 

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td

 //===----------------------------------------------------------------------===//
 
 // Pseudos.
 let hasSideEffects = 1, mayLoad = 0, mayStore = 0, Defs = [VL, VTYPE] in {
 // Due to rs1=X0 having special meaning, we need a GPRNoX0 register class for
 // the when we aren't using one of the special X0 encodings. Otherwise it could
 // be accidentally be made X0 by MachineIR optimizations. To satisfy the
 // verifier, we also need a GPRX0 instruction for the special encodings.
-def PseudoVSETVLI : Pseudo<(outs GPR:$rd), (ins GPRNoX0:$rs1, VTypeIOp:$vtypei), []>;
-def PseudoVSETVLIX0 : Pseudo<(outs GPR:$rd), (ins GPRX0:$rs1, VTypeIOp:$vtypei), []>;
-def PseudoVSETIVLI : Pseudo<(outs GPR:$rd), (ins uimm5:$rs1, VTypeIOp:$vtypei), []>;
+def PseudoVSETVLI : Pseudo<(outs GPR:$rd), (ins GPRNoX0:$rs1, VTypeIOp11:$vtypei), []>;
+def PseudoVSETVLIX0 : Pseudo<(outs GPR:$rd), (ins GPRX0:$rs1, VTypeIOp11:$vtypei), []>;
+def PseudoVSETIVLI : Pseudo<(outs GPR:$rd), (ins uimm5:$rs1, VTypeIOp10:$vtypei), []>;
 }
 
 //===----------------------------------------------------------------------===//
 // 7. Vector Loads and Stores
 //===----------------------------------------------------------------------===//
 
 //===----------------------------------------------------------------------===//
 // 7.4 Vector Unit-Stride Instructions

llvm/test/MC/RISCV/rvv/invalid.s

 # RUN: not llvm-mc -triple=riscv64 --mattr=+experimental-v --mattr=+f %s 2>&1 \
 # RUN:        | FileCheck %s --check-prefix=CHECK-ERROR
 
 vsetivli a2, 32, e8,m1
 # CHECK-ERROR: immediate must be an integer in the range [0, 31]
 
 vsetivli a2, zero, e8,m1
 # CHECK-ERROR: immediate must be an integer in the range [0, 31]
 
+vsetivli a2, 5, (1 << 10)
+# CHECK-ERROR: operand must be e[8|16|32|64|128|256|512|1024],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
+
+vsetivli a2, 5, 0x400
+# CHECK-ERROR: operand must be e[8|16|32|64|128|256|512|1024],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
+
 vsetivli a2, 5, e31
 # CHECK-ERROR: operand must be e[8|16|32|64|128|256|512|1024],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
 
+vsetvli a2, a0, (1 << 11)
+# CHECK-ERROR: operand must be e[8|16|32|64|128|256|512|1024],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
+
+vsetvli a2, a0, 0x800
+# CHECK-ERROR: operand must be e[8|16|32|64|128|256|512|1024],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
+
+
 vsetvli a2, a0, e31
 # CHECK-ERROR: operand must be e[8|16|32|64|128|256|512|1024],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
 
 vsetvli a2, a0, e32,m3
 # CHECK-ERROR: operand must be e[8|16|32|64|128|256|512|1024],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
 
 vsetvli a2, a0, m1,e32
 # CHECK-ERROR: operand must be e[8|16|32|64|128|256|512|1024],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]

llvm/test/MC/RISCV/rvv/vsetvl.s

 # RUN: llvm-mc -triple=riscv64 -show-encoding --mattr=+experimental-v %s \
 # RUN:        | FileCheck %s --check-prefixes=CHECK-ENCODING,CHECK-INST
 # RUN: not llvm-mc -triple=riscv64 -show-encoding %s 2>&1 \
 # RUN:        | FileCheck %s --check-prefix=CHECK-ERROR
 # RUN: llvm-mc -triple=riscv64 -filetype=obj --mattr=+experimental-v %s \
 # RUN:        | llvm-objdump -d --mattr=+experimental-v - \
 # RUN:        | FileCheck %s --check-prefix=CHECK-INST
 # RUN: llvm-mc -triple=riscv64 -filetype=obj --mattr=+experimental-v %s \
 # RUN:        | llvm-objdump -d - | FileCheck %s --check-prefix=CHECK-UNKNOWN
 
+# reserved filed: vlmul[2:0]=4, vsew[2:0]=0b1xx, non-zero bits 8/9/10.
+vsetvli a2, a0, 0x224
+# CHECK-INST: vsetvli a2, a0, 548
+# CHECK-ENCODING: [0x57,0x76,0x45,0x22]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 45 22 <unknown>
+
+vsetvli a2, a0, 0xd0
+# CHECK-INST: vsetvli a2, a0, e32, m1, ta, ma
+# CHECK-ENCODING: [0x57,0x76,0x05,0x0d]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 05 0d <unknown>
+
+vsetvli a2, a0, 0xd1
+# CHECK-INST: vsetvli a2, a0, e32, m2, ta, ma
+# CHECK-ENCODING: [0x57,0x76,0x15,0x0d]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 15 0d <unknown>
+
+vsetvli a2, a0, 0x50
+# CHECK-INST: vsetvli a2, a0, e32, m1, ta, mu
+# CHECK-ENCODING: [0x57,0x76,0x05,0x05]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 05 05 <unknown>
+
+vsetvli a2, a0, 0x90
+# CHECK-INST: vsetvli a2, a0, e32, m1, tu, ma
+# CHECK-ENCODING: [0x57,0x76,0x05,0x09]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 05 09 <unknown>
+
+vsetvli a2, a0, 144
+# CHECK-INST: vsetvli a2, a0, e32, m1, tu, ma
+# CHECK-ENCODING: [0x57,0x76,0x05,0x09]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 05 09 <unknown>
+
 vsetvli a2, a0, e32, m1, ta, ma
 # CHECK-INST: vsetvli a2, a0, e32,  m1,  ta,  ma
 # CHECK-ENCODING: [0x57,0x76,0x05,0x0d]
 # CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
 # CHECK-UNKNOWN: 57 76 05 0d <unknown>
 
 vsetvli a2, a0, e32, m2, ta, ma
 # CHECK-INST: vsetvli a2, a0, e32, m2, ta, ma

 # CHECK-UNKNOWN: 57 76 05 01 <unknown>
 
 vsetvl a2, a0, a1
 # CHECK-INST: vsetvl a2, a0, a1
 # CHECK-ENCODING: [0x57,0x76,0xb5,0x80]
 # CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
 # CHECK-UNKNOWN: 57 76 b5 80 <unknown>
 
+# reserved filed: vlmul[2:0]=4, vsew[2:0]=0b1xx, non-zero bits 8/9/10.
+vsetivli a2, 0, 0x224
+# CHECK-INST: vsetivli a2, 0, 548
+# CHECK-ENCODING: [0x57,0x76,0x40,0xe2]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 40 e2 <unknown>
+
+vsetivli a2, 0, 0xd0
+# CHECK-INST: vsetivli a2, 0, e32, m1, ta, ma
+# CHECK-ENCODING: [0x57,0x76,0x00,0xcd]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 76 00 cd <unknown>
+
+vsetivli a2, 15, 0xd0
+# CHECK-INST: vsetivli a2, 15, e32, m1, ta, ma
+# CHECK-ENCODING: [0x57,0xf6,0x07,0xcd]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 f6 07 cd <unknown>
+
+vsetivli a2, 15, 208
+# CHECK-INST: vsetivli a2, 15, e32, m1, ta, ma
+# CHECK-ENCODING: [0x57,0xf6,0x07,0xcd]
+# CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
+# CHECK-UNKNOWN: 57 f6 07 cd <unknown>
+
 vsetivli a2, 0, e32, m1, ta, ma
 # CHECK-INST: vsetivli a2, 0, e32, m1, ta, ma
 # CHECK-ENCODING: [0x57,0x76,0x00,0xcd]
 # CHECK-ERROR: instruction requires the following: 'V' (Vector Instructions)
 # CHECK-UNKNOWN: 57 76 00 cd <unknown>
 
 vsetivli a2, 15, e32, m1, ta, ma
 # CHECK-INST: vsetivli a2, 15, e32, m1, ta, ma