[RTG][circt-tblgen] Improve whitespace handling (#9887)

Author: maerhart

test/Tools/circt-tblgen/rtg-instruction-methods-errors.td

@@ -209,7 +209,7 @@ def ErrorMissingSpaceOp : TestOp<"error_missing_space"> {
   let arguments = (ins TestRegType:$reg1, TestRegType:$reg2);
   let isaAssemblyFormat = "$reg1$reg2";
 }
-// MISSING_SPACE: [[@LINE-2]]:33: error: expected ' '
+// MISSING_SPACE: [[@LINE-2]]:33: error: expected whitespace
 #endif
 
 // RUN: not circt-tblgen -gen-rtg-instruction-methods -I %S/../../../include -I %S/../../../llvm/mlir/include %s -DTEST_EMPTY_OPERAND_NAME 2>&1 | FileCheck %s --check-prefix=EMPTY_OPERAND_NAME

test/Tools/circt-tblgen/rtg-instruction-methods.td

@@ -354,3 +354,65 @@ def Test22_AnyImmediateOp : TestOp<"any_immediate"> {
 //      CHECK:   ::llvm::SmallVector<char> strBuf;
 // CHECK-NEXT:   ::llvm::cast<::circt::rtg::ImmediateAttr>(adaptor.getImm()).getValue().toStringSigned(strBuf);
 // CHECK-NEXT:   os << strBuf;
+
+def Test23_MultipleSpacesOp : TestOp<"multiple_spaces"> {
+  let isaAssemblyFormat = "`  ` \t  `,  `";
+}
+// CHECK-LABEL:  void MultipleSpacesOp::printInstructionAssembly
+// CHECK-NEXT:    os << "  ";
+// CHECK-NEXT:    os << ",  ";
+
+def Test24_MultiLineFormatOp : TestOp<"multi_line_format"> {
+  let isaBinaryFormat = [{
+    0b001 
+    0b010
+    0b011
+  }];
+  let isaAssemblyFormat = "`a`\n`b`\n";
+}
+// CHECK-LABEL:  void MultiLineFormatOp::printInstructionAssembly
+// CHECK-NEXT:    os << "a";
+// CHECK-NEXT:    os << "b";
+
+// CHECK-LABEL:  void MultiLineFormatOp::printInstructionBinary
+//      CHECK:    binary = binary.concat(llvm::APInt(3, 1));
+// CHECK-NEXT:    binary = binary.concat(llvm::APInt(3, 2));
+// CHECK-NEXT:    binary = binary.concat(llvm::APInt(3, 3));
+
+// Test: Binary literal followed by tab
+def Test25_BinaryLiteralTabOp : TestOp<"binary_literal_tab"> {
+  let isaBinaryFormat = "0b111  0b000";
+  let isaAssemblyFormat = "` `";
+}
+// CHECK-LABEL:  void BinaryLiteralTabOp::printInstructionBinary
+// CHECK:         binary = binary.concat(llvm::APInt(3, 7));
+// CHECK-NEXT:    binary = binary.concat(llvm::APInt(3, 0));
+
+def Test26_IfThenElseNewlinesOp : TestOp<"if_then_else_newlines"> {
+  let arguments = (ins ImmediateOfWidth<1>:$cond);
+  let isaAssemblyFormat = "if\n$cond\nthen\n(`a`\n`b`)\nelse\n( `c` )";
+}
+// CHECK-LABEL:  void IfThenElseNewlinesOp::printInstructionAssembly
+// CHECK:         ::llvm::APInt cond = ::llvm::cast<::circt::rtg::ImmediateAttr>(adaptor.getCond()).getValue();
+// CHECK:         if (cond.isOne()) {
+// CHECK-NEXT:      os << "a";
+// CHECK-NEXT:      os << "b";
+// CHECK-NEXT:    } else {
+// CHECK-NEXT:      os << "c";
+
+def Test27_CustomDirectiveNewlinesOp : TestOp<"custom_directive_newlines"> {
+  let arguments = (ins Arg<TestRegType, "", [SourceReg]>:$rs1,
+                       Arg<TestRegType, "", [SourceReg]>:$rs2);
+  let isaAssemblyFormat = "custom<myFunc>($rs1\n$rs2)";
+}
+// CHECK-LABEL:  void CustomDirectiveNewlinesOp::printInstructionAssembly
+// CHECK:         myFunc(adaptor.getRs1(), adaptor.getRs2(), os);
+
+def Test28_SignednessNewlineOp : TestOp<"signedness_newline"> {
+  let arguments = (ins ImmediateOfWidth<12>:$imm);
+  let isaAssemblyFormat = "` `\nsigned($imm)";
+}
+// CHECK-LABEL:  void SignednessNewlineOp::printInstructionAssembly
+// CHECK-NEXT:    os << " ";
+// CHECK:         ::llvm::SmallVector<char> strBuf;
+// CHECK-NEXT:    ::llvm::cast<::circt::rtg::ImmediateAttr>(adaptor.getImm()).getValue().toStringSigned(strBuf);

tools/circt-tblgen/RTGInstructionFormat.cpp

@@ -40,25 +40,33 @@ SMLoc FormatParser::getLoc() {
   return SMLoc::getFromPointer(loc.getPointer() + pos);
 }
 
+bool FormatParser::isWhitespace(char c) {
+  return c == ' ' || c == '\t' || c == '\n' || c == '\r';
+}
+
 void FormatParser::skipWhitespace() {
-  while (pos < input.size() && (input[pos] == ' ' || input[pos] == '\t'))
+  while (pos < input.size() && isWhitespace(input[pos]))
     ++pos;
 }
 
+void FormatParser::consumeAtLeastOneWhitespace() {
+  if (pos >= input.size() || !isWhitespace(input[pos]))
+    PrintFatalError(getLoc(), "expected whitespace");
+  skipWhitespace();
+}
+
 void FormatParser::consume(StringRef str, bool skipWhitespaceFirst) {
   auto loc = getLoc();
   if (!tryConsume(str, skipWhitespaceFirst))
     PrintFatalError(loc, "expected '" + str + "'");
 }
 
 bool FormatParser::tryConsume(StringRef str, bool skipWhitespaceFirst) {
-  auto savedPos = pos;
+  assert(!str.empty() && "cannot consume empty string");
+  assert(llvm::none_of(str, [&](char c) { return isWhitespace(c); }) &&
+         "string to consume must not be whitespace");
 
-  // In case the user tries to consume whitespace.
-  if (input.substr(pos).starts_with(str)) {
-    pos += str.size();
-    return true;
-  }
+  auto savedPos = pos;
 
   if (skipWhitespaceFirst)
     skipWhitespace();
@@ -160,7 +168,7 @@ FormatNode *FormatParser::parseOptionalBinaryLiteral() {
     return nullptr;
 
   // Check for invalid characters after the binary digits
-  if (!isAtScopeEnd() && input[pos] != ' ')
+  if (!isAtScopeEnd() && !isWhitespace(input[pos]))
     PrintFatalError(getLoc(), "invalid character ('" +
                                   input.substr(pos, pos + 1) +
                                   "') after binary literal");
@@ -194,10 +202,10 @@ FormatNode *FormatParser::parseOptionalSignednessSpecifier() {
   bool isSigned = false;
   StringRef specifierName;
 
-  if (tryConsume("signed(", false)) {
+  if (tryConsume("signed(")) {
     isSigned = true;
     specifierName = "'signed'";
-  } else if (tryConsume("unsigned(", false)) {
+  } else if (tryConsume("unsigned(")) {
     isSigned = false;
     specifierName = "'unsigned'";
   } else {
@@ -254,10 +262,9 @@ FormatNode *FormatParser::parseNextNode() {
                     "unexpected character '" +
                         Twine(input[std::min(pos, input.size() - 1)]) + "'");
 
-  // Elements in the format are separated by a space.
+  // Elements in the format are separated by whitespace.
   if (!isAtScopeEnd())
-    consume(" ");
-  skipWhitespace();
+    consumeAtLeastOneWhitespace();
 
   return node;
 }
@@ -267,7 +274,7 @@ FormatNode *FormatParser::parseOptionalIfThenElse() {
   if (!tryConsume("if"))
     return nullptr;
 
-  consume(" ");
+  consumeAtLeastOneWhitespace();
 
   // Parse the condition operand (must be a 1-bit immediate)
   FormatNode *condNode = parseOptionalOperand();
@@ -276,7 +283,7 @@ FormatNode *FormatParser::parseOptionalIfThenElse() {
 
   auto *condition = cast<OperandNode>(condNode);
 
-  consume(" ");
+  consumeAtLeastOneWhitespace();
 
   SmallVector<FormatNode *> thenBranch;
   bool hasThen = tryConsume("then");
@@ -328,9 +335,10 @@ FormatNode *FormatParser::parseOptionalCustomDirective() {
     auto *operandNode = cast<OperandNode>(argNode);
     arguments.push_back(operandNode);
 
-    // Arguments are separated by spaces, but the last one is followed by ')'
+    // Arguments are separated by whitespace, but the last one is followed by
+    // ')'
     if (!tryConsume(")")) {
-      consume(" ");
+      consumeAtLeastOneWhitespace();
       continue;
     }
     break;

tools/circt-tblgen/RTGInstructionFormat.h

@@ -428,6 +428,9 @@ struct FormatParser {
   /// \return True if the string was consumed, false otherwise
   bool tryConsume(StringRef str, bool skipWhitespaceFirst = true);
 
+  /// Consume all but at least one whitespace character at the current position.
+  void consumeAtLeastOneWhitespace();
+
   /// Try to parse an unsigned integer literal at the current position.
   /// \return True if an unsigned integer was parsed, false otherwise
   bool tryParseUIntLiteral(uint64_t &value);
@@ -446,6 +449,9 @@ struct FormatParser {
   /// Check if the current position is at the end of a scope.
   bool isAtScopeEnd();
 
+  /// Check if a character is whitespace.
+  bool isWhitespace(char c);
+
   /// Get the current source location in the input string.
   /// \return The current source location for error reporting
   llvm::SMLoc getLoc();