[SOL] Remove now obsolete old syntax bits. (#72)

Patch #54 originally
reworked the SBF textual assembly syntax to match the rbpf-style syntax.

In order to allow some soak-time, the above patch temporarily supported
both the old and new syntax (selectable on the command line or via bespoke
assembler directives). A little over a year has passed since then,
and all is well with the new syntax, testing, and so forth.

The current patch now removes all existing remnants of the old syntax--
including some of the target-independent changes made in the original
patch to support both (e.g., a minor TableGen change for variants, some
additional command line selector flags, etc).

All related unit tests have been updated accordingly.

Author: nvjle

llvm/lib/Target/SBF/AsmParser/SBFAsmParser.cpp

@@ -33,12 +33,6 @@ class SBFAsmParser : public MCTargetAsmParser {
 
   SMLoc getLoc() const { return getParser().getTok().getLoc(); }
 
-  bool isNewSyntax() {
-    return getParser().getAssemblerDialect() == 0;
-  }
-
-  bool PreMatchCheck(OperandVector &Operands);
-
   bool MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
                                OperandVector &Operands, MCStreamer &Out,
                                uint64_t &ErrorInfo,
@@ -52,25 +46,14 @@ class SBFAsmParser : public MCTargetAsmParser {
   bool ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
                         SMLoc NameLoc, OperandVector &Operands) override;
 
-  bool parseOldInstruction(ParseInstructionInfo &Info, StringRef Name,
-                           SMLoc NameLoc, OperandVector &Operands);
-
   bool ParseDirective(AsmToken DirectiveID) override;
 
-  // "=" is used as assignment operator for assembly statement, so can't be used
-  // for symbol assignment (old syntax only).
-  bool equalIsAsmAssignment() override { return isNewSyntax(); }
-  // "*" is used for dereferencing memory that it will be the start of
-  // statement (old syntax only).
-  bool starIsStartOfStatement() override { return !isNewSyntax(); }
-
 #define GET_ASSEMBLER_HEADER
 #include "SBFGenAsmMatcher.inc"
 
   bool parseOperand(OperandVector &Operands, StringRef Mnemonic);
   OperandMatchResultTy parseImmediate(OperandVector &Operands);
   OperandMatchResultTy parseRegister(OperandVector &Operands);
-  OperandMatchResultTy parseOperandAsOperator(OperandVector &Operands);
   OperandMatchResultTy parseMemOperand(OperandVector &Operands);
 
 public:
@@ -280,45 +263,14 @@ struct SBFOperand : public MCParsedAsmOperand {
 #define GET_MATCHER_IMPLEMENTATION
 #include "SBFGenAsmMatcher.inc"
 
-bool SBFAsmParser::PreMatchCheck(OperandVector &Operands) {
-
-  // These checks not needed for the new syntax.
-  if (isNewSyntax())
-    return false;
-
-  if (Operands.size() == 4) {
-    // check "reg1 = -reg2" and "reg1 = be16/be32/be64/le16/le32/le64 reg2",
-    // reg1 must be the same as reg2
-    SBFOperand &Op0 = (SBFOperand &)*Operands[0];
-    SBFOperand &Op1 = (SBFOperand &)*Operands[1];
-    SBFOperand &Op2 = (SBFOperand &)*Operands[2];
-    SBFOperand &Op3 = (SBFOperand &)*Operands[3];
-    if (Op0.isReg() && Op1.isToken() && Op2.isToken() && Op3.isReg()
-        && Op1.getToken() == "="
-        && (Op2.getToken() == "-" || Op2.getToken() == "be16"
-            || Op2.getToken() == "be32" || Op2.getToken() == "be64"
-            || Op2.getToken() == "le16" || Op2.getToken() == "le32"
-            || Op2.getToken() == "le64")
-        && Op0.getReg() != Op3.getReg())
-      return true;
-  }
-
-  return false;
-}
-
 bool SBFAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
                                            OperandVector &Operands,
                                            MCStreamer &Out, uint64_t &ErrorInfo,
                                            bool MatchingInlineAsm) {
   MCInst Inst;
   SMLoc ErrorLoc;
 
-  if (PreMatchCheck(Operands))
-    return Error(IDLoc, "additional inst constraint not met");
-
-  unsigned Dialect = getParser().getAssemblerDialect();
-  switch (MatchInstructionImpl(Operands, Inst, ErrorInfo, MatchingInlineAsm,
-                               Dialect)) {
+  switch (MatchInstructionImpl(Operands, Inst, ErrorInfo, MatchingInlineAsm)) {
   default:
     break;
   case Match_Success:
@@ -372,75 +324,6 @@ OperandMatchResultTy SBFAsmParser::tryParseRegister(MCRegister &Reg,
   return MatchOperand_NoMatch;
 }
 
-OperandMatchResultTy
-SBFAsmParser::parseOperandAsOperator(OperandVector &Operands) {
-  if (isNewSyntax())
-    llvm_unreachable("parseOperandAsOperator called for new syntax");
-
-  SMLoc S = getLoc();
-
-  if (getLexer().getKind() == AsmToken::Identifier) {
-    StringRef Name = getLexer().getTok().getIdentifier();
-
-    if (SBFOperand::isValidIdInMiddle(Name)) {
-      getLexer().Lex();
-      Operands.push_back(SBFOperand::createToken(Name, S));
-      return MatchOperand_Success;
-    }
-
-    return MatchOperand_NoMatch;
-  }
-
-  switch (getLexer().getKind()) {
-  case AsmToken::Minus:
-  case AsmToken::Plus: {
-    if (getLexer().peekTok().is(AsmToken::Integer))
-      return MatchOperand_NoMatch;
-    [[fallthrough]];
-  }
-
-  case AsmToken::Equal:
-  case AsmToken::Greater:
-  case AsmToken::Less:
-  case AsmToken::Pipe:
-  case AsmToken::Star:
-  case AsmToken::LParen:
-  case AsmToken::RParen:
-  case AsmToken::LBrac:
-  case AsmToken::RBrac:
-  case AsmToken::Slash:
-  case AsmToken::Amp:
-  case AsmToken::Percent:
-  case AsmToken::Caret: {
-    StringRef Name = getLexer().getTok().getString();
-    getLexer().Lex();
-    Operands.push_back(SBFOperand::createToken(Name, S));
-
-    return MatchOperand_Success;
-  }
-
-  case AsmToken::EqualEqual:
-  case AsmToken::ExclaimEqual:
-  case AsmToken::GreaterEqual:
-  case AsmToken::GreaterGreater:
-  case AsmToken::LessEqual:
-  case AsmToken::LessLess: {
-    Operands.push_back(SBFOperand::createToken(
-        getLexer().getTok().getString().substr(0, 1), S));
-    Operands.push_back(SBFOperand::createToken(
-        getLexer().getTok().getString().substr(1, 1), S));
-    getLexer().Lex();
-
-    return MatchOperand_Success;
-  }
-
-  default:
-    break;
-  }
-
-  return MatchOperand_NoMatch;
-}
-
 OperandMatchResultTy SBFAsmParser::parseRegister(OperandVector &Operands) {
   SMLoc S = getLoc();
   SMLoc E = SMLoc::getFromPointer(S.getPointer() - 1);
@@ -519,9 +402,6 @@ OperandMatchResultTy SBFAsmParser::parseMemOperand(OperandVector &Operands) {
 /// information, adding to Operands. If operand was parsed, returns false, else
 /// true.
 bool SBFAsmParser::parseOperand(OperandVector &Operands, StringRef Mnemonic) {
-  if (!isNewSyntax())
-    llvm_unreachable("parseOperand called for old syntax");
-
   // Attempt to parse token as a register.
   if (parseRegister(Operands) == MatchOperand_Success)
     return false;
@@ -544,10 +424,6 @@ bool SBFAsmParser::parseOperand(OperandVector &Operands, StringRef Mnemonic) {
 /// Parse an SBF instruction.
 bool SBFAsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
                                     SMLoc NameLoc, OperandVector &Operands) {
-  if (!isNewSyntax()) {
-    return parseOldInstruction(Info, Name, NameLoc, Operands);
-  }
-
   // First operand is token for instruction mnemonic.
   Operands.push_back(SBFOperand::createToken(Name, NameLoc));
 
@@ -581,71 +457,7 @@ bool SBFAsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
   return false;
 }
 
-/// Parse an SBF instruction which is in SBF verifier format (old syntax).
-bool SBFAsmParser::parseOldInstruction(ParseInstructionInfo &Info,
-                                       StringRef Name, SMLoc NameLoc,
-                                       OperandVector &Operands) {
-  if (isNewSyntax())
-    llvm_unreachable("parseOldInstruction called for new syntax");
-
-  // The first operand could be either register or actually an operator.
-  unsigned RegNo = MatchRegisterName(Name);
-
-  if (RegNo != 0) {
-    SMLoc E = SMLoc::getFromPointer(NameLoc.getPointer() - 1);
-    Operands.push_back(SBFOperand::createReg(RegNo, NameLoc, E));
-  } else if (SBFOperand::isValidIdAtStart (Name))
-    Operands.push_back(SBFOperand::createToken(Name, NameLoc));
-  else
-    return Error(NameLoc, "invalid register/token name");
-
-  while (!getLexer().is(AsmToken::EndOfStatement)) {
-    // Attempt to parse token as operator
-    if (parseOperandAsOperator(Operands) == MatchOperand_Success)
-      continue;
-
-    // Attempt to parse token as register
-    if (parseRegister(Operands) == MatchOperand_Success)
-      continue;
-
-    // Attempt to parse token as an immediate
-    if (parseImmediate(Operands) != MatchOperand_Success) {
-      SMLoc Loc = getLexer().getLoc();
-      return Error(Loc, "unexpected token");
-    }
-  }
-
-  if (getLexer().isNot(AsmToken::EndOfStatement)) {
-    SMLoc Loc = getLexer().getLoc();
-
-    getParser().eatToEndOfStatement();
-
-    return Error(Loc, "unexpected token");
-  }
-
-  // Consume the EndOfStatement.
-  getParser().Lex();
-  return false;
-}
-
-bool SBFAsmParser::ParseDirective(AsmToken DirectiveID) {
-  // This returns false if this function recognizes the directive
-  // regardless of whether it is successfully handles or reports an
-  // error. Otherwise it returns true to give the generic parser a
-  // chance at recognizing it.
-  StringRef IDVal = DirectiveID.getString();
-
-  if (IDVal == ".syntax_old") {
-    getParser().setAssemblerDialect(1);
-    return false;
-  }
-  if (IDVal == ".syntax_new") {
-    getParser().setAssemblerDialect(0);
-    return false;
-  }
-
-  return true;
-}
+bool SBFAsmParser::ParseDirective(AsmToken DirectiveID) { return true; }
 
 extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeSBFAsmParser() {
   RegisterMCAsmParser<SBFAsmParser> XX(getTheSBFXTarget());

llvm/lib/Target/SBF/CMakeLists.txt

@@ -4,7 +4,6 @@ set(LLVM_TARGET_DEFINITIONS SBF.td)
 
 tablegen(LLVM SBFGenAsmMatcher.inc -gen-asm-matcher)
 tablegen(LLVM SBFGenAsmWriter.inc -gen-asm-writer)
-tablegen(LLVM SBFGenAsmWriter1.inc -gen-asm-writer -asmwriternum=1)
 tablegen(LLVM SBFGenCallingConv.inc -gen-callingconv)
 tablegen(LLVM SBFGenDAGISel.inc -gen-dag-isel)
 tablegen(LLVM SBFGenDisassemblerTables.inc -gen-disassembler)

llvm/lib/Target/SBF/MCTargetDesc/SBFInstPrinter.cpp

@@ -25,7 +25,6 @@ using namespace llvm;
 
 // Include the auto-generated portion of the assembly writer.
 #include "SBFGenAsmWriter.inc"
-#include "SBFGenAsmWriter1.inc"
 
 void SBFInstPrinter::printInst(const MCInst *MI, uint64_t Address,
                                StringRef Annot, const MCSubtargetInfo &STI,

llvm/lib/Target/SBF/MCTargetDesc/SBFInstPrinter.h

@@ -36,26 +36,6 @@ class SBFInstPrinter : public MCInstPrinter {
   void printInstruction(const MCInst *MI, uint64_t Address, raw_ostream &O);
   static const char *getRegisterName(MCRegister Reg);
 };
-class MachineInstr;
-
-class SBFLegacyInstPrinter : public SBFInstPrinter {
-public:
-  SBFLegacyInstPrinter(const MCAsmInfo &MAI, const MCInstrInfo &MII,
-                       const MCRegisterInfo &MRI)
-      : SBFInstPrinter(MAI, MII, MRI) {}
-
-  void printInst(const MCInst *MI, uint64_t Address, StringRef Annot,
-                 const MCSubtargetInfo &STI, raw_ostream &O) override {
-    printInstruction(MI, Address, O);
-    printAnnotation(O, Annot);
-  }
-
-  // Autogenerated by tblgen.
-  std::pair<const char *, uint64_t> getMnemonic(const MCInst *MI) override;
-  void printInstruction(const MCInst *MI, uint64_t Address, raw_ostream &O);
-  static const char *getRegisterName(MCRegister Reg);
-};
-
 }
 
 #endif

llvm/lib/Target/SBF/MCTargetDesc/SBFMCAsmInfo.cpp

@@ -17,12 +17,8 @@
 
 using namespace llvm;
 
-cl::opt<unsigned> SBFAsmWriterVariant(
-    "sbf-output-asm-variant", cl::Hidden, cl::init(0),
-    cl::desc("Choose output assembly variant (0 = sbf[default], 1 = legacy)"));
-
 SBFMCAsmInfo::SBFMCAsmInfo(const Triple &TT, const MCTargetOptions &Options) {
-  AssemblerDialect = SBFAsmWriterVariant;
+  assert(AssemblerDialect == 0);
 
   PrivateGlobalPrefix = ".L";
   WeakRefDirective = "\t.weak\t";

llvm/lib/Target/SBF/MCTargetDesc/SBFMCTargetDesc.cpp

@@ -80,8 +80,6 @@ static MCInstPrinter *createSBFMCInstPrinter(const Triple &T,
                                              const MCRegisterInfo &MRI) {
   if (SyntaxVariant == 0)
     return new SBFInstPrinter(MAI, MII, MRI);
-  if (SyntaxVariant == 1)
-    return new SBFLegacyInstPrinter(MAI, MII, MRI);
 
   return nullptr;
 }

llvm/lib/Target/SBF/SBF.td

@@ -58,40 +58,25 @@ def SBFAsmWriter : AsmWriter {
   bit isMCAsmWriter = 1;
 }
 
-def LegacyAsmWriter : AsmWriter {
-  let AsmWriterClassName = "LegacyInstPrinter";
-  int Variant = 1;
-  int isMCAsmWriter = 1;
-}
-
 //===----------------------------------------------------------------------===//
 // Assembly parser
 //===----------------------------------------------------------------------===//
 
-def SBFAsmParser : AsmParser {
-  bit HasMnemonicFirst = 0;
-}
+def SBFAsmParser : AsmParser;
 
 def SBFAsmParserVariant : AsmParserVariant {
   int Variant = 0;
   string Name = "sbf";
   string BreakCharacters = ".";
 }
 
-def LegacyAsmParserVariant : AsmParserVariant {
-  int Variant = 1;
-  string Name = "legacy";
-  string BreakCharacters = ".";
-  string TokenizingCharacters = "#()[]=:.<>!+*/";
-}
-
 //===----------------------------------------------------------------------===//
 // Target Declaration
 //===----------------------------------------------------------------------===//
 
 def SBF : Target {
   let InstructionSet = SBFInstrInfo;
-  let AssemblyWriters = [SBFAsmWriter, LegacyAsmWriter];
+  let AssemblyWriters = [SBFAsmWriter];
   let AssemblyParsers = [SBFAsmParser];
-  let AssemblyParserVariants = [SBFAsmParserVariant, LegacyAsmParserVariant];
+  let AssemblyParserVariants = [SBFAsmParserVariant];
 }

llvm/lib/Target/SBF/SBFAsmPrinter.cpp

@@ -34,7 +34,6 @@ using namespace llvm;
 
 #define DEBUG_TYPE "asm-printer"
 
-extern cl::opt<unsigned> SBFAsmWriterVariant;
 static cl::opt<bool> SBFEnableBTFEmission(
     "sbf-enable-btf-emission", cl::Hidden, cl::init(false),
     cl::desc("Enable BTF debuginfo sections to be emitted"));
@@ -138,15 +137,11 @@ bool SBFAsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI,
   if (ExtraCode)
     return true; // Unknown modifier.
 
-  if (SBFAsmWriterVariant == 1)
-    O << "(";
   O << SBFInstPrinter::getRegisterName(BaseMO.getReg());
   if (Offset < 0)
     O << " - " << -Offset;
   else
     O << " + " << Offset;
-  if (SBFAsmWriterVariant == 1)
-    O << ")";
 
   return false;
 }