[RISCV] Make All VType Parts Optional (#144971)

This matches the current binutils behaviour, and the orignal ratified
spec that states that LMUL=1 when the lmul operand is omitted. A variety
of previously invalid vtype instructions are now accepted by the
assembler.

To match binutils, at least one of the vtype operands must be provided.

Also fixes the MCOperandPredicate definition in VTypeIOp, which had one
logic issue and one syntax issue. This is not used by the MC layer
currently, so this change should not affect the existing implementation.

Fixes #143744

Author: lenary

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

@@ -66,16 +66,14 @@ struct ParserOptionsSet {
 };
 
 class RISCVAsmParser : public MCTargetAsmParser {
-  // This tracks the parsing of the 4 operands that make up the vtype portion
-  // of vset(i)vli instructions which are separated by commas. The state names
-  // represent the next expected operand with Done meaning no other operands are
-  // expected.
-  enum VTypeState {
-    VTypeState_SEW,
-    VTypeState_LMUL,
-    VTypeState_TailPolicy,
-    VTypeState_MaskPolicy,
-    VTypeState_Done,
+  // This tracks the parsing of the 4 optional operands that make up the vtype
+  // portion of vset(i)vli instructions which are separated by commas.
+  enum class VTypeState {
+    SeenNothingYet,
+    SeenSew,
+    SeenLmul,
+    SeenTailPolicy,
+    SeenMaskPolicy,
   };
 
   SmallVector<FeatureBitset, 4> FeatureBitStack;
@@ -2229,25 +2227,30 @@ bool RISCVAsmParser::parseVTypeToken(const AsmToken &Tok, VTypeState &State,
     return true;
 
   StringRef Identifier = Tok.getIdentifier();
-
-  switch (State) {
-  case VTypeState_SEW:
-    if (!Identifier.consume_front("e"))
-      break;
+  if (State < VTypeState::SeenSew && Identifier.consume_front("e")) {
     if (Identifier.getAsInteger(10, Sew))
-      break;
+      return true;
     if (!RISCVVType::isValidSEW(Sew))
-      break;
-    State = VTypeState_LMUL;
+      return true;
+
+    State = VTypeState::SeenSew;
     return false;
-  case VTypeState_LMUL: {
-    if (!Identifier.consume_front("m"))
-      break;
+  }
+
+  if (State < VTypeState::SeenLmul && Identifier.consume_front("m")) {
+    // Might arrive here if lmul and tail policy unspecified, if so we're
+    // parsing a MaskPolicy not an LMUL.
+    if (Identifier == "a" || Identifier == "u") {
+      MaskAgnostic = (Identifier == "a");
+      State = VTypeState::SeenMaskPolicy;
+      return false;
+    }
+
     Fractional = Identifier.consume_front("f");
     if (Identifier.getAsInteger(10, Lmul))
-      break;
+      return true;
     if (!RISCVVType::isValidLMUL(Lmul, Fractional))
-      break;
+      return true;
 
     if (Fractional) {
       unsigned ELEN = STI->hasFeature(RISCV::FeatureStdExtZve64x) ? 64 : 32;
@@ -2258,30 +2261,32 @@ bool RISCVAsmParser::parseVTypeToken(const AsmToken &Tok, VTypeState &State,
                     Twine(MinLMUL) + " is reserved");
     }
 
-    State = VTypeState_TailPolicy;
+    State = VTypeState::SeenLmul;
     return false;
   }
-  case VTypeState_TailPolicy:
+
+  if (State < VTypeState::SeenTailPolicy && Identifier.starts_with("t")) {
     if (Identifier == "ta")
       TailAgnostic = true;
     else if (Identifier == "tu")
       TailAgnostic = false;
     else
-      break;
-    State = VTypeState_MaskPolicy;
+      return true;
+
+    State = VTypeState::SeenTailPolicy;
     return false;
-  case VTypeState_MaskPolicy:
+  }
+
+  if (State < VTypeState::SeenMaskPolicy && Identifier.starts_with("m")) {
     if (Identifier == "ma")
       MaskAgnostic = true;
     else if (Identifier == "mu")
       MaskAgnostic = false;
     else
-      break;
-    State = VTypeState_Done;
+      return true;
+
+    State = VTypeState::SeenMaskPolicy;
     return false;
-  case VTypeState_Done:
-    // Extra token?
-    break;
   }
 
   return true;
@@ -2290,49 +2295,45 @@ bool RISCVAsmParser::parseVTypeToken(const AsmToken &Tok, VTypeState &State,
 ParseStatus RISCVAsmParser::parseVTypeI(OperandVector &Operands) {
   SMLoc S = getLoc();
 
-  unsigned Sew = 0;
-  unsigned Lmul = 0;
+  // Default values
+  unsigned Sew = 8;
+  unsigned Lmul = 1;
   bool Fractional = false;
   bool TailAgnostic = false;
   bool MaskAgnostic = false;
 
-  VTypeState State = VTypeState_SEW;
-  SMLoc SEWLoc = S;
-
-  if (parseVTypeToken(getTok(), State, Sew, Lmul, Fractional, TailAgnostic,
-                      MaskAgnostic))
-    return ParseStatus::NoMatch;
-
-  getLexer().Lex();
-
-  while (parseOptionalToken(AsmToken::Comma)) {
+  VTypeState State = VTypeState::SeenNothingYet;
+  do {
     if (parseVTypeToken(getTok(), State, Sew, Lmul, Fractional, TailAgnostic,
-                        MaskAgnostic))
+                        MaskAgnostic)) {
+      // The first time, errors return NoMatch rather than Failure
+      if (State == VTypeState::SeenNothingYet)
+        return ParseStatus::NoMatch;
       break;
+    }
 
     getLexer().Lex();
-  }
+  } while (parseOptionalToken(AsmToken::Comma));
 
-  if (getLexer().is(AsmToken::EndOfStatement) && State == VTypeState_Done) {
-    RISCVVType::VLMUL VLMUL = RISCVVType::encodeLMUL(Lmul, Fractional);
-    if (Fractional) {
-      unsigned ELEN = STI->hasFeature(RISCV::FeatureStdExtZve64x) ? 64 : 32;
-      unsigned MaxSEW = ELEN / Lmul;
-      // If MaxSEW < 8, we should have printed warning about reserved LMUL.
-      if (MaxSEW >= 8 && Sew > MaxSEW)
-        Warning(SEWLoc,
-                "use of vtype encodings with SEW > " + Twine(MaxSEW) +
-                    " and LMUL == mf" + Twine(Lmul) +
-                    " may not be compatible with all RVV implementations");
-    }
+  if (!getLexer().is(AsmToken::EndOfStatement) ||
+      State == VTypeState::SeenNothingYet)
+    return generateVTypeError(S);
 
-    unsigned VTypeI =
-        RISCVVType::encodeVTYPE(VLMUL, Sew, TailAgnostic, MaskAgnostic);
-    Operands.push_back(RISCVOperand::createVType(VTypeI, S));
-    return ParseStatus::Success;
+  RISCVVType::VLMUL VLMUL = RISCVVType::encodeLMUL(Lmul, Fractional);
+  if (Fractional) {
+    unsigned ELEN = STI->hasFeature(RISCV::FeatureStdExtZve64x) ? 64 : 32;
+    unsigned MaxSEW = ELEN / Lmul;
+    // If MaxSEW < 8, we should have printed warning about reserved LMUL.
+    if (MaxSEW >= 8 && Sew > MaxSEW)
+      Warning(S, "use of vtype encodings with SEW > " + Twine(MaxSEW) +
+                     " and LMUL == mf" + Twine(Lmul) +
+                     " may not be compatible with all RVV implementations");
   }
 
-  return generateVTypeError(S);
+  unsigned VTypeI =
+      RISCVVType::encodeVTYPE(VLMUL, Sew, TailAgnostic, MaskAgnostic);
+  Operands.push_back(RISCVOperand::createVType(VTypeI, S));
+  return ParseStatus::Success;
 }
 
 bool RISCVAsmParser::generateVTypeError(SMLoc ErrorLoc) {

llvm/lib/Target/RISCV/RISCVInstrInfoV.td

@@ -30,8 +30,8 @@ class VTypeIOp<int VTypeINum> : RISCVOp {
   let MCOperandPredicate = [{
     int64_t Imm;
     if (MCOp.evaluateAsConstantImm(Imm))
-      return isUInt<VTypeINum>(Imm);
-    return MCOp.isBareSymbolRef();
+      return isUInt<}] # VTypeINum # [{>(Imm);
+    return MCOp.Kind == KindTy::VType;
   }];
 }
 

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

@@ -1,12 +1,6 @@
 # RUN: not llvm-mc -triple=riscv64 --mattr=+v --mattr=+f %s 2>&1 \
 # RUN:        | FileCheck %s --check-prefix=CHECK-ERROR
 
-vsetivli a2, 32, e8,m1
-# CHECK-ERROR: operand must be e[8|16|32|64],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
-
-vsetivli a2, zero, e8,m1
-# CHECK-ERROR: operand must be e[8|16|32|64],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
-
 vsetivli a2, 5, (1 << 10)
 # CHECK-ERROR: operand must be e[8|16|32|64],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
 
@@ -22,10 +16,6 @@ vsetvli a2, a0, (1 << 11)
 vsetvli a2, a0, 0x800
 # CHECK-ERROR: operand must be e[8|16|32|64],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],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],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
 
@@ -59,12 +49,6 @@ vsetvli a2, a0, e8,m1,tx
 vsetvli a2, a0, e8,m1,ta,mx
 # CHECK-ERROR: operand must be e[8|16|32|64],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
 
-vsetvli a2, a0, e8,m1,ma
-# CHECK-ERROR: operand must be e[8|16|32|64],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
-
-vsetvli a2, a0, e8,m1,mu
-# CHECK-ERROR: operand must be e[8|16|32|64],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
-
 vsetvli a2, a0, e8x,m1,tu,mu
 # CHECK-ERROR: operand must be e[8|16|32|64],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
 
@@ -80,17 +64,14 @@ vsetvli a2, a0, e8,m1,tu,mut
 vsetvli a2, a0, e8,m1,tut,mu
 # CHECK-ERROR: operand must be e[8|16|32|64],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
 
-vsetvli a2, a0, e8
-# CHECK-ERROR: operand must be e[8|16|32|64],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
-
-vsetvli a2, a0, e8,m1
+vsetvli a2, a0, e8,1,ta,ma
 # CHECK-ERROR: operand must be e[8|16|32|64],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
 
-vsetvli a2, a0, e8,m1,ta
+vsetvli a2, a0, ma,tu,m1,e8
 # CHECK-ERROR: operand must be e[8|16|32|64],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
 
-vsetvli a2, a0, e8,1,ta,ma
-# CHECK-ERROR: operand must be e[8|16|32|64],m[1|2|4|8|f2|f4|f8],[ta|tu],[ma|mu]
+vsetvli a2, a0,
+# CHECK-ERROR: unknown operand
 
 vadd.vv v1, v3, v2, v4.t
 # CHECK-ERROR: operand must be v0.t