[AMDGPU] Assembler: rework parsing of optional operands.

[SamWot]

Summary

Change process of parsing of optional operands. All optional operands use same parsing method - parseOptionalOperand().
No default values are added to OperandsVector.
Get rid of WORKAROUND_USE_DUMMY_OPERANDS_INSTEAD_MUTIPLE_DEFAULT_OPERANDS.

[artem.tamazov]

Please go ahead after minor fixing. Overall, good job, thanks!

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

93 ↗

(On Diff #58108)

There are many places like this, with comments which look useless. If you plan to get rid of those later, please put TODO or FIXME (or use #ifdef). Otherwise, remove.

2664 ↗

(On Diff #58108)

newline

[SamWot]

Removed unnecessary commented code.

[vpykhtin]

I love to delete code.

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

2250 ↗

(On Diff #58209)

while this can be submitted for now I think this can be enhanced to determine optional parser by operand prefix. Most of them starts with identifier (except for bits that may have "no" before them) which can be used as a key to select optional operand parser.

llvm/trunk/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

 };
 
 } // namespace SendMsg
 } // namespace AMDGPU
 } // namespace llvm
 
 using namespace llvm;
 
-// In some cases (e.g. buffer atomic instructions) MatchOperandParserImpl()
-// may invoke tryCustomParseOperand() multiple times with the same MCK value.
-// That leads to adding of the same "default" operand multiple times in a row,
-// which is wrong. The workaround adds only the 1st default operand, while for
-// the rest the "dummy" operands being added. The reason for dummies is that if
-// we just skip adding an operand, then parser would get stuck in endless loop.
-// Dummies shall be removed prior matching & emitting MCInsts.
-//
-// Comment out this macro to disable the workaround.
-#define WORKAROUND_USE_DUMMY_OPERANDS_INSTEAD_MUTIPLE_DEFAULT_OPERANDS
-
 namespace {
 
 struct OptionalOperand;
 
 enum RegisterKind { IS_UNKNOWN, IS_VGPR, IS_SGPR, IS_TTMP, IS_SPECIAL };
 
 class AMDGPUOperand : public MCParsedAsmOperand {
   enum KindTy {
     Token,
     Immediate,
     Register,
     Expression
-#ifdef WORKAROUND_USE_DUMMY_OPERANDS_INSTEAD_MUTIPLE_DEFAULT_OPERANDS
-    ,Dummy
-#endif
   } Kind;
 
   SMLoc StartLoc, EndLoc;
 
 public:
   AMDGPUOperand(enum KindTy K) : MCParsedAsmOperand(), Kind(K) {}
 
   MCContext *Ctx;
void addSoppBrTargetOperands(MCInst &Inst, unsigned N) const {
     if (isImm())
       addImmOperands(Inst, N);
     else {
       assert(isExpr());
       Inst.addOperand(MCOperand::createExpr(Expr));
     }
   }
 
-#ifdef WORKAROUND_USE_DUMMY_OPERANDS_INSTEAD_MUTIPLE_DEFAULT_OPERANDS
-  bool isDummy() const {
-    return Kind == Dummy;
-  }
-#endif
-
   bool isToken() const override {
     return Kind == Token;
   }
 
   bool isImm() const override {
     return Kind == Immediate;
   }
 
case Immediate:
       OS << " mods: " << Imm.Modifiers << '>';
       break;
     case Token:
       OS << '\'' << getToken() << '\'';
       break;
     case Expression:
       OS << "<expr " << *Expr << '>';
       break;
-#ifdef WORKAROUND_USE_DUMMY_OPERANDS_INSTEAD_MUTIPLE_DEFAULT_OPERANDS
-    case Dummy:
-      OS << "<dummy>";
-      break;
-#endif
     }
   }
 
   static AMDGPUOperand::Ptr CreateImm(int64_t Val, SMLoc Loc,
                                       enum ImmTy Type = ImmTyNone,
                                       bool IsFPImm = false) {
     auto Op = llvm::make_unique<AMDGPUOperand>(Immediate);
     Op->Imm.Val = Val;
public:
   static AMDGPUOperand::Ptr CreateExpr(const class MCExpr *Expr, SMLoc S) {
     auto Op = llvm::make_unique<AMDGPUOperand>(Expression);
     Op->Expr = Expr;
     Op->StartLoc = S;
     Op->EndLoc = S;
     return Op;
   }
 
-#ifdef WORKAROUND_USE_DUMMY_OPERANDS_INSTEAD_MUTIPLE_DEFAULT_OPERANDS
-  static AMDGPUOperand::Ptr CreateDummy(SMLoc S) {
-    auto Op = llvm::make_unique<AMDGPUOperand>(Dummy);
-    Op->StartLoc = S;
-    Op->EndLoc = S;
-    return Op;
-  }
-#endif
-
   bool isSWaitCnt() const;
   bool isHwreg() const;
   bool isSendMsg() const;
   bool isMubufOffset() const;
   bool isSMRDOffset() const;
   bool isSMRDLiteralOffset() const;
   bool isDPPCtrl() const;
 };
public:
 
   bool isForcedVOP3() const {
     return ForcedEncodingSize == 64;
   }
 
   std::unique_ptr<AMDGPUOperand> parseRegister();
   bool ParseRegister(unsigned &RegNo, SMLoc &StartLoc, SMLoc &EndLoc) override;
   unsigned checkTargetMatchPredicate(MCInst &Inst) override;
+  unsigned validateTargetOperandClass(MCParsedAsmOperand &Op,
+                                      unsigned Kind) override;
   bool MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
                                OperandVector &Operands, MCStreamer &Out,
                                uint64_t &ErrorInfo,
                                bool MatchingInlineAsm) override;
   bool ParseDirective(AsmToken DirectiveID) override;
   OperandMatchResultTy parseOperand(OperandVector &Operands, StringRef Mnemonic);
   bool ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
                         SMLoc NameLoc, OperandVector &Operands) override;
 
-  OperandMatchResultTy parseIntWithPrefix(const char *Prefix, int64_t &Int,
-                                          int64_t Default = 0, bool AddDefault = false);
+  OperandMatchResultTy parseIntWithPrefix(const char *Prefix, int64_t &Int);
   OperandMatchResultTy parseIntWithPrefix(const char *Prefix,
                                           OperandVector &Operands,
                                           enum AMDGPUOperand::ImmTy ImmTy = AMDGPUOperand::ImmTyNone,
-                                          int64_t Default = 0, bool AddDefault = false,
                                           bool (*ConvertResult)(int64_t&) = 0);
   OperandMatchResultTy parseNamedBit(const char *Name, OperandVector &Operands,
-                                     enum AMDGPUOperand::ImmTy ImmTy =
-                                                      AMDGPUOperand::ImmTyNone,
-                                     bool AddDefault = false);
-  OperandMatchResultTy parseOptionalOps(
-                                   const ArrayRef<OptionalOperand> &OptionalOps,
-                                   OperandVector &Operands);
+                                     enum AMDGPUOperand::ImmTy ImmTy = AMDGPUOperand::ImmTyNone);
   OperandMatchResultTy parseStringWithPrefix(const char *Prefix, StringRef &Value);
 
   OperandMatchResultTy parseImm(OperandVector &Operands);
   OperandMatchResultTy parseRegOrImm(OperandVector &Operands);
   OperandMatchResultTy parseRegOrImmWithInputMods(OperandVector &Operands);
 
-  OperandMatchResultTy parseOptionalOperand(OperandVector &Operands, const OptionalOperand& Op, bool AddDefault);
-  OperandMatchResultTy parseAMDGPUOperand(OperandVector &Operands, StringRef Name);
-
   void cvtDSOffset01(MCInst &Inst, const OperandVector &Operands);
   void cvtDS(MCInst &Inst, const OperandVector &Operands);
 
   bool parseCnt(int64_t &IntVal);
   OperandMatchResultTy parseSWaitCntOps(OperandVector &Operands);
   bool parseHwregOperand(int64_t &HwRegCode, int64_t &Offset, int64_t &Width, bool &IsIdentifier);
   OperandMatchResultTy parseHwreg(OperandVector &Operands);
+
 private:
   struct OperandInfoTy {
     int64_t Id;
     bool IsSymbolic;
     OperandInfoTy(int64_t Id_) : Id(Id_), IsSymbolic(false) { }
   };
+
   bool parseSendMsg(OperandInfoTy &Msg, OperandInfoTy &Operation, int64_t &StreamId);
+
 public:
+  OperandMatchResultTy parseOptionalOperand(OperandVector &Operands);
+
   OperandMatchResultTy parseSendMsgOp(OperandVector &Operands);
   OperandMatchResultTy parseSOppBrTarget(OperandVector &Operands);
   AMDGPUOperand::Ptr defaultHwreg() const;
 
   void cvtMubuf(MCInst &Inst, const OperandVector &Operands) { cvtMubufImpl(Inst, Operands, false, false); }
   void cvtMubufAtomic(MCInst &Inst, const OperandVector &Operands) { cvtMubufImpl(Inst, Operands, true, false); }
   void cvtMubufAtomicReturn(MCInst &Inst, const OperandVector &Operands) { cvtMubufImpl(Inst, Operands, true, true); }
   AMDGPUOperand::Ptr defaultMubufOffset() const;
   AMDGPUOperand::Ptr defaultGLC() const;
   AMDGPUOperand::Ptr defaultSLC() const;
   AMDGPUOperand::Ptr defaultTFE() const;
 
-  OperandMatchResultTy parseOModSI(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "omod"); }
-  OperandMatchResultTy parseClampSI(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "clamp"); }
-  OperandMatchResultTy parseSMRDOffset(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "smrd_offset"); }
-  OperandMatchResultTy parseSMRDLiteralOffset(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "smrd_literal_offset"); }
-  OperandMatchResultTy parseDPPCtrl(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "dpp_ctrl"); }
-  OperandMatchResultTy parseRowMask(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "row_mask"); }
-  OperandMatchResultTy parseBankMask(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "bank_mask"); }
-  OperandMatchResultTy parseBoundCtrl(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "bound_ctrl"); }
-  OperandMatchResultTy parseOffen(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "offen"); }
-  OperandMatchResultTy parseIdxen(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "idxen"); }
-  OperandMatchResultTy parseAddr64(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "addr64"); }
-  OperandMatchResultTy parseOffset(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "offset"); }
-  OperandMatchResultTy parseOffset0(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "offset0"); }
-  OperandMatchResultTy parseOffset1(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "offset1"); }
-  OperandMatchResultTy parseGLC(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "glc"); }
-  OperandMatchResultTy parseSLC(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "slc"); }
-  OperandMatchResultTy parseTFE(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "tfe"); }
-  OperandMatchResultTy parseGDS(OperandVector &Operands) { return parseAMDGPUOperand(Operands, "gds"); }
-
-  OperandMatchResultTy parseDMask(OperandVector &Operands);
-  OperandMatchResultTy parseUNorm(OperandVector &Operands);
-  OperandMatchResultTy parseDA(OperandVector &Operands);
-  OperandMatchResultTy parseR128(OperandVector &Operands);
-  OperandMatchResultTy parseLWE(OperandVector &Operands);
   AMDGPUOperand::Ptr defaultDMask() const;
   AMDGPUOperand::Ptr defaultUNorm() const;
   AMDGPUOperand::Ptr defaultDA() const;
   AMDGPUOperand::Ptr defaultR128() const;
   AMDGPUOperand::Ptr defaultLWE() const;
   AMDGPUOperand::Ptr defaultSMRDOffset() const;
   AMDGPUOperand::Ptr defaultSMRDLiteralOffset() const;
   
   AMDGPUOperand::Ptr defaultClampSI() const;
   AMDGPUOperand::Ptr defaultOModSI() const;
   
   OperandMatchResultTy parseOModOperand(OperandVector &Operands);
 
   void cvtId(MCInst &Inst, const OperandVector &Operands);
   void cvtVOP3_2_mod(MCInst &Inst, const OperandVector &Operands);
   void cvtVOP3(MCInst &Inst, const OperandVector &Operands);
 
   void cvtMIMG(MCInst &Inst, const OperandVector &Operands);
   void cvtMIMGAtomic(MCInst &Inst, const OperandVector &Operands);
 
-  OperandMatchResultTy parseDPPCtrlOps(OperandVector &Operands, bool AddDefault);
+  OperandMatchResultTy parseDPPCtrl(OperandVector &Operands);
   AMDGPUOperand::Ptr defaultRowMask() const;
   AMDGPUOperand::Ptr defaultBankMask() const;
   AMDGPUOperand::Ptr defaultBoundCtrl() const;
   void cvtDPP(MCInst &Inst, const OperandVector &Operands);
 
   OperandMatchResultTy parseSDWASel(OperandVector &Operands);
   OperandMatchResultTy parseSDWADstUnused(OperandVector &Operands);
   AMDGPUOperand::Ptr defaultSDWASel() const;
   AMDGPUOperand::Ptr defaultSDWADstUnused() const;
 };
 
 struct OptionalOperand {
   const char *Name;
   AMDGPUOperand::ImmTy Type;
   bool IsBit;
-  int64_t Default;
   bool (*ConvertResult)(int64_t&);
 };
 
 }
 
 static int getRegClass(RegisterKind Is, unsigned RegWidth) {
   if (Is == IS_VGPR) {
     switch (RegWidth) {
unsigned AMDGPUAsmParser::checkTargetMatchPredicate(MCInst &Inst) {
   if ((TSFlags & SIInstrFlags::VOP3) &&
       (TSFlags & SIInstrFlags::VOPAsmPrefer32Bit) &&
       getForcedEncodingSize() != 64)
     return Match_PreferE32;
 
   return Match_Success;
 }
 
-
 bool AMDGPUAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
                                               OperandVector &Operands,
                                               MCStreamer &Out,
                                               uint64_t &ErrorInfo,
                                               bool MatchingInlineAsm) {
   MCInst Inst;
 
-#ifdef WORKAROUND_USE_DUMMY_OPERANDS_INSTEAD_MUTIPLE_DEFAULT_OPERANDS
-  // Remove dummies prior matching. Iterate backwards becase vector::erase()
-  // invalidates all iterators which refer after erase point.
-  for (auto I = Operands.rbegin(), E = Operands.rend(); I != E; ) {
-    auto X = I++;
-    if (static_cast<AMDGPUOperand*>(X->get())->isDummy()) {
-      Operands.erase(X.base() -1);
-    }
-  }
-#endif
-
   switch (MatchInstructionImpl(Operands, Inst, ErrorInfo, MatchingInlineAsm)) {
     default: break;
     case Match_Success:
       Inst.setLoc(IDLoc);
       Out.EmitInstruction(Inst, getSTI());
       return false;
     case Match_MissingFeature:
       return Error(IDLoc, "instruction not supported on this GPU");
bool AMDGPUAsmParser::ParseInstruction(ParseInstructionInfo &Info,
   return false;
 }
 
 //===----------------------------------------------------------------------===//
 // Utility functions
 //===----------------------------------------------------------------------===//
 
 AMDGPUAsmParser::OperandMatchResultTy
-AMDGPUAsmParser::parseIntWithPrefix(const char *Prefix, int64_t &Int,
-                                    int64_t Default, bool AddDefault) {
-  // We are at the end of the statement, and this is a default argument, so
-  // use a default value.
-  if (getLexer().is(AsmToken::EndOfStatement)) {
-    Int = Default;
-    return MatchOperand_Success;
-  }
-
+AMDGPUAsmParser::parseIntWithPrefix(const char *Prefix, int64_t &Int) {
   switch(getLexer().getKind()) {
     default: return MatchOperand_NoMatch;
     case AsmToken::Identifier: {
       StringRef Name = Parser.getTok().getString();
       if (!Name.equals(Prefix)) {
-        if (AddDefault) {
-          Int = Default;
-          return MatchOperand_Success;
-        }
         return MatchOperand_NoMatch;
       }
 
       Parser.Lex();
       if (getLexer().isNot(AsmToken::Colon))
         return MatchOperand_ParseFail;
 
       Parser.Lex();
       if (getLexer().isNot(AsmToken::Integer))
         return MatchOperand_ParseFail;
 
       if (getParser().parseAbsoluteExpression(Int))
         return MatchOperand_ParseFail;
       break;
     }
   }
   return MatchOperand_Success;
 }
 
 AMDGPUAsmParser::OperandMatchResultTy
 AMDGPUAsmParser::parseIntWithPrefix(const char *Prefix, OperandVector &Operands,
                                     enum AMDGPUOperand::ImmTy ImmTy,
-                                    int64_t Default, bool AddDefault,
                                     bool (*ConvertResult)(int64_t&)) {
 
   SMLoc S = Parser.getTok().getLoc();
   int64_t Value = 0;
 
-  AMDGPUAsmParser::OperandMatchResultTy Res = parseIntWithPrefix(Prefix, Value, Default, AddDefault);
+  AMDGPUAsmParser::OperandMatchResultTy Res = parseIntWithPrefix(Prefix, Value);
   if (Res != MatchOperand_Success)
     return Res;
 
   if (ConvertResult && !ConvertResult(Value)) {
     return MatchOperand_ParseFail;
   }
 
   Operands.push_back(AMDGPUOperand::CreateImm(Value, S, ImmTy));
-
-#ifdef WORKAROUND_USE_DUMMY_OPERANDS_INSTEAD_MUTIPLE_DEFAULT_OPERANDS
-  if (Value == Default && AddDefault) {
-    // Reverse lookup in previously added operands (skip just added one)
-    // for the first non-dummy operand. If it is of the same type,
-    // then replace just added default operand with dummy.
-    for (auto I = Operands.rbegin(), E = Operands.rend(); I != E; ++I) {
-      if (I == Operands.rbegin())
-        continue;
-      if (static_cast<AMDGPUOperand*>(I->get())->isDummy())
-        continue;
-      if (static_cast<AMDGPUOperand*>(I->get())->isImmTy(ImmTy)) {
-        Operands.pop_back();
-        Operands.push_back(AMDGPUOperand::CreateDummy(S)); // invalidates iterators
-        break;
-      }
-    }
-  }
-#endif
   return MatchOperand_Success;
 }
 
 AMDGPUAsmParser::OperandMatchResultTy
 AMDGPUAsmParser::parseNamedBit(const char *Name, OperandVector &Operands,
-                               enum AMDGPUOperand::ImmTy ImmTy,
-                                     bool AddDefault) {
+                               enum AMDGPUOperand::ImmTy ImmTy) {
   int64_t Bit = 0;
   SMLoc S = Parser.getTok().getLoc();
 
   // We are at the end of the statement, and this is a default argument, so
   // use a default value.
   if (getLexer().isNot(AsmToken::EndOfStatement)) {
     switch(getLexer().getKind()) {
       case AsmToken::Identifier: {
         StringRef Tok = Parser.getTok().getString();
         if (Tok == Name) {
           Bit = 1;
           Parser.Lex();
         } else if (Tok.startswith("no") && Tok.endswith(Name)) {
           Bit = 0;
           Parser.Lex();
         } else {
-          if (AddDefault) {
-            Bit = 0;
-          } else {
-            return MatchOperand_NoMatch;
-          }
-        }
         break;
       }
       default:
         return MatchOperand_NoMatch;
     }
   }
 
   Operands.push_back(AMDGPUOperand::CreateImm(Bit, S, ImmTy));
void addOptionalImmOperand(MCInst& Inst, const OperandVector& Operands,
   if (i != OptionalIdx.end()) {
     unsigned Idx = i->second;
     ((AMDGPUOperand &)*Operands[Idx]).addImmOperands(Inst, 1);
   } else {
     Inst.addOperand(MCOperand::createImm(Default));
   }
 }
 
-static bool operandsHasOptionalOp(const OperandVector &Operands,
-                                  const OptionalOperand &OOp) {
-  for (unsigned i = 0; i < Operands.size(); i++) {
-    const AMDGPUOperand &ParsedOp = ((const AMDGPUOperand &)*Operands[i]);
-    if ((ParsedOp.isImm() && ParsedOp.getImmTy() == OOp.Type) ||
-        (ParsedOp.isToken() && ParsedOp.getToken() == OOp.Name))
-      return true;
-
-  }
-  return false;
-}
-
-AMDGPUAsmParser::OperandMatchResultTy
-AMDGPUAsmParser::parseOptionalOps(const ArrayRef<OptionalOperand> &OptionalOps,
-                                   OperandVector &Operands) {
-  SMLoc S = Parser.getTok().getLoc();
-  for (const OptionalOperand &Op : OptionalOps) {
-    if (operandsHasOptionalOp(Operands, Op))
-      continue;
-    AMDGPUAsmParser::OperandMatchResultTy Res;
-    int64_t Value;
-    if (Op.IsBit) {
-      Res = parseNamedBit(Op.Name, Operands, Op.Type);
-      if (Res == MatchOperand_NoMatch)
-        continue;
-      return Res;
-    }
-
-    Res = parseIntWithPrefix(Op.Name, Value, Op.Default);
-
-    if (Res == MatchOperand_NoMatch)
-      continue;
-
-    if (Res != MatchOperand_Success)
-      return Res;
-
-    bool DefaultValue = (Value == Op.Default);
-
-    if (Op.ConvertResult && !Op.ConvertResult(Value)) {
-      return MatchOperand_ParseFail;
-    }
-
-    if (!DefaultValue) {
-      Operands.push_back(AMDGPUOperand::CreateImm(Value, S, Op.Type));
-    }
-    return MatchOperand_Success;
-  }
-  return MatchOperand_NoMatch;
-}
-
 AMDGPUAsmParser::OperandMatchResultTy 
 AMDGPUAsmParser::parseStringWithPrefix(const char *Prefix, StringRef &Value) {
   if (getLexer().isNot(AsmToken::Identifier)) {
     return MatchOperand_NoMatch;
   }
   StringRef Tok = Parser.getTok().getString();
   if (Tok != Prefix) {
     return MatchOperand_NoMatch;

 }
 
 AMDGPUAsmParser::OperandMatchResultTy
 AMDGPUAsmParser::parseHwreg(OperandVector &Operands) {
   int64_t Imm16Val = 0;
   SMLoc S = Parser.getTok().getLoc();
 
   switch(getLexer().getKind()) {
-    default: return MatchOperand_ParseFail;
+    default: return MatchOperand_NoMatch;
     case AsmToken::Integer:
       // The operand can be an integer value.
       if (getParser().parseAbsoluteExpression(Imm16Val))
         return MatchOperand_ParseFail;
       if (!isInt<16>(Imm16Val) && !isUInt<16>(Imm16Val)) {
         Error(S, "invalid immediate: only 16-bit values are legal");
         // Do not return error code, but create an imm operand anyway and proceed
         // to the next operand, if any. That avoids unneccessary error messages.
case AsmToken::Identifier:
           MCSymbolRefExpr::create(getContext().getOrCreateSymbol(
                                   Parser.getTok().getString()), getContext()), S));
       Parser.Lex();
       return MatchOperand_Success;
   }
 }
 
 //===----------------------------------------------------------------------===//
-// flat
-//===----------------------------------------------------------------------===//
-
-//===----------------------------------------------------------------------===//
 // mubuf
 //===----------------------------------------------------------------------===//
 
 bool AMDGPUOperand::isMubufOffset() const {
   return isImmTy(ImmTyOffset) && isUInt<12>(getImm());
 }
 
 AMDGPUOperand::Ptr AMDGPUAsmParser::defaultMubufOffset() const {
void AMDGPUAsmParser::cvtMubufImpl(MCInst &Inst,
   addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTySLC);
   addOptionalImmOperand(Inst, Operands, OptionalIdx, AMDGPUOperand::ImmTyTFE);
 }
 
 //===----------------------------------------------------------------------===//
 // mimg
 //===----------------------------------------------------------------------===//
 
-AMDGPUAsmParser::OperandMatchResultTy
-AMDGPUAsmParser::parseDMask(OperandVector &Operands) {
-  return parseIntWithPrefix("dmask", Operands, AMDGPUOperand::ImmTyDMask);
-}
-
-AMDGPUAsmParser::OperandMatchResultTy
-AMDGPUAsmParser::parseUNorm(OperandVector &Operands) {
-  return parseNamedBit("unorm", Operands, AMDGPUOperand::ImmTyUNorm);
-}
-
-AMDGPUAsmParser::OperandMatchResultTy
-AMDGPUAsmParser::parseDA(OperandVector &Operands) {
-  return parseNamedBit("da", Operands, AMDGPUOperand::ImmTyDA);
-}
-
-AMDGPUAsmParser::OperandMatchResultTy
-AMDGPUAsmParser::parseR128(OperandVector &Operands) {
-  return parseNamedBit("r128", Operands, AMDGPUOperand::ImmTyR128);
-}
-
-AMDGPUAsmParser::OperandMatchResultTy
-AMDGPUAsmParser::parseLWE(OperandVector &Operands) {
-  return parseNamedBit("lwe", Operands, AMDGPUOperand::ImmTyLWE);
-}
-
 void AMDGPUAsmParser::cvtMIMG(MCInst &Inst, const OperandVector &Operands) {
   unsigned I = 1;
   const MCInstrDesc &Desc = MII.get(Inst.getOpcode());
   for (unsigned J = 0; J < Desc.getNumDefs(); ++J) {
     ((AMDGPUOperand &)*Operands[I++]).addRegOperands(Inst, 1);
   }
 
   OptionalImmIndexMap OptionalIdx;
static bool ConvertBoundCtrl(int64_t &BoundCtrl) {
   } else if (BoundCtrl == -1) {
     BoundCtrl = 0;
     return true;
   }
   return false;
 }
 
 // Note: the order in this table matches the order of operands in AsmString.
-static const OptionalOperand AMDGPUOperandTable[] = {
-  {"offen",   AMDGPUOperand::ImmTyOffen, true, 0, nullptr},
-  {"offset0", AMDGPUOperand::ImmTyOffset0, false, 0, nullptr},
-  {"offset1", AMDGPUOperand::ImmTyOffset1, false, 0, nullptr},
-  {"gds",     AMDGPUOperand::ImmTyGDS, true, 0, nullptr},
-  {"offset",  AMDGPUOperand::ImmTyOffset, false, 0, nullptr},
-  {"glc",     AMDGPUOperand::ImmTyGLC, true, 0, nullptr},
-  {"slc",     AMDGPUOperand::ImmTySLC, true, 0, nullptr},
-  {"tfe",     AMDGPUOperand::ImmTyTFE, true, 0, nullptr},
-  {"clamp",   AMDGPUOperand::ImmTyClampSI, true, 0, nullptr},
-  {"omod",    AMDGPUOperand::ImmTyOModSI, false, 1, ConvertOmodMul},
-  {"unorm",   AMDGPUOperand::ImmTyUNorm, true, 0, nullptr},
-  {"da",      AMDGPUOperand::ImmTyDA,    true, 0, nullptr},
-  {"r128",    AMDGPUOperand::ImmTyR128,  true, 0, nullptr},
-  {"lwe",     AMDGPUOperand::ImmTyLWE,   true, 0, nullptr},
-  {"dmask",   AMDGPUOperand::ImmTyDMask, false, 0, nullptr},
-  {"dpp_ctrl", AMDGPUOperand::ImmTyDppCtrl, false, -1, nullptr},
-  {"row_mask", AMDGPUOperand::ImmTyDppRowMask, false, 0xf, nullptr},
-  {"bank_mask", AMDGPUOperand::ImmTyDppBankMask, false, 0xf, nullptr},
-  {"bound_ctrl", AMDGPUOperand::ImmTyDppBoundCtrl, false, -1, ConvertBoundCtrl},
+static const OptionalOperand AMDGPUOptionalOperandTable[] = {
+  {"offen",   AMDGPUOperand::ImmTyOffen, true, nullptr},
+  {"idxen",   AMDGPUOperand::ImmTyIdxen, true, nullptr},
+  {"addr64",  AMDGPUOperand::ImmTyAddr64, true, nullptr},
+  {"offset0", AMDGPUOperand::ImmTyOffset0, false, nullptr},
+  {"offset1", AMDGPUOperand::ImmTyOffset1, false, nullptr},
+  {"gds",     AMDGPUOperand::ImmTyGDS, true, nullptr},
+  {"offset",  AMDGPUOperand::ImmTyOffset, false, nullptr},
+  {"glc",     AMDGPUOperand::ImmTyGLC, true, nullptr},
+  {"slc",     AMDGPUOperand::ImmTySLC, true, nullptr},
+  {"tfe",     AMDGPUOperand::ImmTyTFE, true, nullptr},
+  {"clamp",   AMDGPUOperand::ImmTyClampSI, true, nullptr},
+  {"omod",    AMDGPUOperand::ImmTyOModSI, false, ConvertOmodMul},
+  {"unorm",   AMDGPUOperand::ImmTyUNorm, true, nullptr},
+  {"da",      AMDGPUOperand::ImmTyDA,    true, nullptr},
+  {"r128",    AMDGPUOperand::ImmTyR128,  true, nullptr},
+  {"lwe",     AMDGPUOperand::ImmTyLWE,   true, nullptr},
+  {"dmask",   AMDGPUOperand::ImmTyDMask, false, nullptr},
+  {"row_mask",   AMDGPUOperand::ImmTyDppRowMask, false, nullptr},
+  {"bank_mask",  AMDGPUOperand::ImmTyDppBankMask, false, nullptr},
+  {"bound_ctrl", AMDGPUOperand::ImmTyDppBoundCtrl, false, ConvertBoundCtrl},
+  {"sdwa_sel",   AMDGPUOperand::ImmTySdwaSel, false, nullptr},
+  {"dst_unused", AMDGPUOperand::ImmTySdwaDstUnused, false, nullptr},
 };
 
-AMDGPUAsmParser::OperandMatchResultTy AMDGPUAsmParser::parseOptionalOperand(OperandVector &Operands, const OptionalOperand& Op, bool AddDefault)
-{
+AMDGPUAsmParser::OperandMatchResultTy AMDGPUAsmParser::parseOptionalOperand(OperandVector &Operands) {
+  OperandMatchResultTy res;
+  for (const OptionalOperand &Op : AMDGPUOptionalOperandTable) {
+    // try to parse any optional operand here
     if (Op.IsBit) {
-    return parseNamedBit(Op.Name, Operands, Op.Type, AddDefault);
-  } else if (Op.Type == AMDGPUOperand::ImmTyDppCtrl) {
-    return parseDPPCtrlOps(Operands, AddDefault);
+      res = parseNamedBit(Op.Name, Operands, Op.Type);
     } else if (Op.Type == AMDGPUOperand::ImmTyOModSI) {
-    return parseOModOperand(Operands);
+      res = parseOModOperand(Operands);
+    } else if (Op.Type == AMDGPUOperand::ImmTySdwaSel) {
+      res = parseSDWASel(Operands);
+    } else if (Op.Type == AMDGPUOperand::ImmTySdwaDstUnused) {
+      res = parseSDWADstUnused(Operands);
     } else {
-    return parseIntWithPrefix(Op.Name, Operands, Op.Type, Op.Default, AddDefault, Op.ConvertResult);
-  }
-}
-
-AMDGPUAsmParser::OperandMatchResultTy AMDGPUAsmParser::parseAMDGPUOperand(OperandVector &Operands, StringRef Name)
-{
-  StringRef Tok;
-  if (getLexer().is(AsmToken::Identifier)) {
-    Tok = Parser.getTok().getString();
+      res = parseIntWithPrefix(Op.Name, Operands, Op.Type, Op.ConvertResult);
     }
-  bool optional = false;
-  if (Tok == "mul" || Tok == "div") { optional = true; }
-  for (const OptionalOperand &Op1 : AMDGPUOperandTable) {
-    if (Op1.Name == Tok) { optional = true; break; }
-  }
-  // Attemp to parse current optional operand.
-  for (const OptionalOperand &Op : AMDGPUOperandTable) {
-    // TODO: For now, omod is handled separately because
-    // token name does not match name in table.
-    bool parseThis =
-      Name == "" ||
-      (Op.Name == Name) ||
-      (Name == "omod" && Op.Type == AMDGPUOperand::ImmTyOModSI);
-    if (parseThis && Tok == Name) {
-      // Exactly the expected token for optional operand.
-      // Parse it and add operand normally.
-      return parseOptionalOperand(Operands, Op, true);
-    } else if (parseThis) {
-      // Token for optional operand which is later in the table
-      // than the one we expect. If needed, add default value
-      // for the operand we expect, do not consume anything
-      // and return MatchOperand_NoMatch. Parsing will continue.
-      return parseOptionalOperand(Operands, Op, optional);
-    } else if (Op.Name == Tok) {
-      // This looks like optional operand, but we do not expect it.
-      // This is the case when AsmString has token in it.
-      return MatchOperand_NoMatch;
+    if (res != MatchOperand_NoMatch) {
+      return res;
     }
   }
   return MatchOperand_NoMatch;
 }
 
 AMDGPUAsmParser::OperandMatchResultTy AMDGPUAsmParser::parseOModOperand(OperandVector &Operands)
 {
   StringRef Name = Parser.getTok().getString();
   if (Name == "mul") {
-    return parseIntWithPrefix("mul", Operands, AMDGPUOperand::ImmTyOModSI, 0, false, ConvertOmodMul);
+    return parseIntWithPrefix("mul", Operands, AMDGPUOperand::ImmTyOModSI, ConvertOmodMul);
   } else if (Name == "div") {
-    return parseIntWithPrefix("div", Operands, AMDGPUOperand::ImmTyOModSI, 0, false, ConvertOmodDiv);
+    return parseIntWithPrefix("div", Operands, AMDGPUOperand::ImmTyOModSI, ConvertOmodDiv);
   } else {
     return MatchOperand_NoMatch;
   }
 }
 
 AMDGPUOperand::Ptr AMDGPUAsmParser::defaultClampSI() const {
   return AMDGPUOperand::CreateImm(0, SMLoc(), AMDGPUOperand::ImmTyClampSI);
 }
return ((Imm >= 0x000) && (Imm <= 0x0ff)) ||
            (Imm == 0x141) ||
            (Imm == 0x142) ||
            (Imm == 0x143);
   }
   return false;
 }
 
 AMDGPUAsmParser::OperandMatchResultTy
-AMDGPUAsmParser::parseDPPCtrlOps(OperandVector &Operands, bool AddDefault) {
+AMDGPUAsmParser::parseDPPCtrl(OperandVector &Operands) {
   SMLoc S = Parser.getTok().getLoc();
   StringRef Prefix;
   int64_t Int;
 
   if (getLexer().getKind() == AsmToken::Identifier) {
     Prefix = Parser.getTok().getString();
   } else {
     return MatchOperand_NoMatch;
if (Prefix != "quad_perm"
         && Prefix != "row_shl"
         && Prefix != "row_shr"
         && Prefix != "row_ror"
         && Prefix != "wave_shl"
         && Prefix != "wave_rol"
         && Prefix != "wave_shr"
         && Prefix != "wave_ror"
         && Prefix != "row_bcast") {
-      if (AddDefault) {
-        Operands.push_back(AMDGPUOperand::CreateImm(0, S, AMDGPUOperand::ImmTyDppCtrl));
-        return MatchOperand_Success;
-      } else {
-        return MatchOperand_NoMatch;
-      }
-    }
 
     Parser.Lex();
     if (getLexer().isNot(AsmToken::Colon))
       return MatchOperand_ParseFail;
 
     if (Prefix == "quad_perm") {
       // quad_perm:[%d,%d,%d,%d]
       Parser.Lex();

 extern "C" void LLVMInitializeAMDGPUAsmParser() {
   RegisterMCAsmParser<AMDGPUAsmParser> A(TheAMDGPUTarget);
   RegisterMCAsmParser<AMDGPUAsmParser> B(TheGCNTarget);
 }
 
 #define GET_REGISTER_MATCHER
 #define GET_MATCHER_IMPLEMENTATION
 #include "AMDGPUGenAsmMatcher.inc"
+
+
+// This fuction should be defined after auto-generated include so that we have
+// MatchClassKind enum defined
+unsigned AMDGPUAsmParser::validateTargetOperandClass(MCParsedAsmOperand &Op,
+                                                     unsigned Kind) {
+  // Tokens like "glc" would be parsed as immediate operands in ParseOperand().
+  // But MatchInstructionImpl() expects to meet token and fails to validate 
+  // operand. This method checks if we are given immediate operand but expect to
+  // get corresponding token.
+  AMDGPUOperand &Operand = (AMDGPUOperand&)Op;
+  switch (Kind) {
+  case MCK_addr64:
+    return Operand.isAddr64() ? Match_Success : Match_InvalidOperand;
+  case MCK_gds:
+    return Operand.isGDS() ? Match_Success : Match_InvalidOperand;
+  case MCK_glc:
+    return Operand.isGLC() ? Match_Success : Match_InvalidOperand;
+  case MCK_idxen:
+    return Operand.isIdxen() ? Match_Success : Match_InvalidOperand;
+  case MCK_offen:
+    return Operand.isOffen() ? Match_Success : Match_InvalidOperand;
+  default: return Match_InvalidOperand;
+  }
+}

llvm/trunk/lib/Target/AMDGPU/SIInstrInfo.td

 }
 
 include "SIInstrFormats.td"
 include "VIInstrFormats.td"
 
 class NamedMatchClass<string CName, bit Optional = 1> : AsmOperandClass {
   let Name = "Imm"#CName;
   let PredicateMethod = "is"#CName;
-  let ParserMethod = "parse"#CName;
+  let ParserMethod = !if(Optional, "parseOptionalOperand", "parse"#CName);
   let RenderMethod = "addImmOperands";
   let IsOptional = Optional;
   let DefaultMethod = "default"#CName;
 }
 
 def sdwa_sel : NamedMatchClass<"SDWASel">;
 
 class NamedOperandBit<string Name, AsmOperandClass MatchClass> : Operand<i1> {

 def bank_mask : NamedOperandU32<"BankMask", NamedMatchClass<"BankMask">>;
 def bound_ctrl : NamedOperandBit<"BoundCtrl", NamedMatchClass<"BoundCtrl">>;
 
 def dst_sel : NamedOperandU32<"SDWADstSel", sdwa_sel>;
 def src0_sel : NamedOperandU32<"SDWASrc0Sel", sdwa_sel>;
 def src1_sel : NamedOperandU32<"SDWASrc1Sel", sdwa_sel>;
 def dst_unused : NamedOperandU32<"SDWADstUnused", NamedMatchClass<"SDWADstUnused">>;
 
-def hwreg : NamedOperandU16<"Hwreg", NamedMatchClass<"Hwreg">>;
+def hwreg : NamedOperandU16<"Hwreg", NamedMatchClass<"Hwreg", 0>>;
 
 } // End OperandType = "OPERAND_IMMEDIATE"
 
 
 def VOPDstS64 : VOPDstOperand <SReg_64>;
 
 //===----------------------------------------------------------------------===//
 // Complex patterns

llvm/trunk/test/MC/AMDGPU/ds-err.s

 // RUN: not llvm-mc -arch=amdgcn %s 2>&1 | FileCheck %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=SI %s 2>&1 | FileCheck %s
 
 // offset too big
 // CHECK: error: invalid operand for instruction
 ds_add_u32 v2, v4 offset:1000000000
 
 // offset0 twice
-// CHECK:  error: not a valid operand.
+// CHECK:  error: invalid operand for instruction
 ds_write2_b32 v2, v4, v6 offset0:4 offset0:8
 
 // offset1 twice
-// CHECK:  error: not a valid operand.
+// CHECK:  error: invalid operand for instruction
 ds_write2_b32 v2, v4, v6 offset1:4 offset1:8
 
 // offset0 too big
 // CHECK: invalid operand for instruction
 ds_write2_b32 v2, v4, v6 offset0:1000000000
 
 // offset1 too big
 // CHECK: invalid operand for instruction
 ds_write2_b32 v2, v4, v6 offset1:1000000000