[mlir][LLVM][NFC] Implement print/parse for LLVMStructType

[zero9178]

The printing and parsing logic for struct types was still using ad-hoc functions instead of the more conventional print and parse methods whose declarations are automatically generated by TableGen.

This PR effectively renames these functions and uses them directly as implementations for print and parse of LLVMStructType.

This additionally fixes linking errors when users or auto generated code may call print and parse directly.

Fixes #117927

[llvmbot]

@llvm/pr-subscribers-mlir-llvm

@llvm/pr-subscribers-mlir

Author: Markus Böck (zero9178)

Changes

The printing and parsing logic for types is still using ad-hoc functions instead of the more conventional print and parse methods whose declarations are automatically generated by TableGen.

This PR effectively renames these functions and uses them directly as implementations for print and parse of LLVMStructType.

This additionally fixes linking errors when users or auto generated code may call print and parse directly.

Fixes #117927

---

Full diff: https://github.com/llvm/llvm-project/pull/117930.diff

3 Files Affected:

• (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td (+1-1)
• (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMTypeSyntax.cpp (+11-14)
• (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp (+1-1)

diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
index e88139fa5b28da..77c8035ce3d71a 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
@@ -233,7 +233,7 @@ def LLVMStructType : LLVMType<"LLVMStruct", "struct", [
     bool isIdentified() const;
 
     /// Checks if a struct is opaque.
-    bool isOpaque();
+    bool isOpaque() const;
 
     /// Checks if a struct is initialized.
     bool isInitialized();
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypeSyntax.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypeSyntax.cpp
index 9537f7c40dd4be..d700dc52d42d20 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypeSyntax.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypeSyntax.cpp
@@ -53,14 +53,14 @@ static StringRef getTypeKeyword(Type type) {
 
 /// Prints a structure type. Keeps track of known struct names to handle self-
 /// or mutually-referring structs without falling into infinite recursion.
-static void printStructType(AsmPrinter &printer, LLVMStructType type) {
+void LLVMStructType::print(AsmPrinter &printer) const {
   FailureOr<AsmPrinter::CyclicPrintReset> cyclicPrint;
 
   printer << "<";
-  if (type.isIdentified()) {
-    cyclicPrint = printer.tryStartCyclicPrint(type);
+  if (isIdentified()) {
+    cyclicPrint = printer.tryStartCyclicPrint(*this);
 
-    printer << '"' << type.getName() << '"';
+    printer << '"' << getName() << '"';
     // If we are printing a reference to one of the enclosing structs, just
     // print the name and stop to avoid infinitely long output.
     if (failed(cyclicPrint)) {
@@ -70,17 +70,17 @@ static void printStructType(AsmPrinter &printer, LLVMStructType type) {
     printer << ", ";
   }
 
-  if (type.isIdentified() && type.isOpaque()) {
+  if (isIdentified() && isOpaque()) {
     printer << "opaque>";
     return;
   }
 
-  if (type.isPacked())
+  if (isPacked())
     printer << "packed ";
 
   // Put the current type on stack to avoid infinite recursion.
   printer << '(';
-  llvm::interleaveComma(type.getBody(), printer.getStream(),
+  llvm::interleaveComma(getBody(), printer.getStream(),
                         [&](Type subtype) { dispatchPrint(printer, subtype); });
   printer << ')';
   printer << '>';
@@ -105,11 +105,8 @@ void mlir::LLVM::detail::printType(Type type, AsmPrinter &printer) {
 
   llvm::TypeSwitch<Type>(type)
       .Case<LLVMPointerType, LLVMArrayType, LLVMFixedVectorType,
-            LLVMScalableVectorType, LLVMFunctionType, LLVMTargetExtType>(
-          [&](auto type) { type.print(printer); })
-      .Case([&](LLVMStructType structType) {
-        printStructType(printer, structType);
-      });
+            LLVMScalableVectorType, LLVMFunctionType, LLVMTargetExtType,
+            LLVMStructType>([&](auto type) { type.print(printer); });
 }
 
 //===----------------------------------------------------------------------===//
@@ -182,7 +179,7 @@ static LLVMStructType trySetStructBody(LLVMStructType type,
 ///                 `(` llvm-type-list `)` `>`
 ///               | `struct<` string-literal `>`
 ///               | `struct<` string-literal `, opaque>`
-static LLVMStructType parseStructType(AsmParser &parser) {
+Type LLVMStructType::parse(AsmParser &parser) {
   Location loc = parser.getEncodedSourceLoc(parser.getCurrentLocation());
 
   if (failed(parser.parseLess()))
@@ -316,7 +313,7 @@ static Type dispatchParse(AsmParser &parser, bool allowAny = true) {
       .Case("ptr", [&] { return LLVMPointerType::parse(parser); })
       .Case("vec", [&] { return parseVectorType(parser); })
       .Case("array", [&] { return LLVMArrayType::parse(parser); })
-      .Case("struct", [&] { return parseStructType(parser); })
+      .Case("struct", [&] { return LLVMStructType::parse(parser); })
       .Case("target", [&] { return LLVMTargetExtType::parse(parser); })
       .Case("x86_amx", [&] { return LLVMX86AMXType::get(ctx); })
       .Default([&] {
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
index 33c231e2d2045f..4e5600c715915c 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
@@ -480,7 +480,7 @@ LogicalResult LLVMStructType::setBody(ArrayRef<Type> types, bool isPacked) {
 
 bool LLVMStructType::isPacked() const { return getImpl()->isPacked(); }
 bool LLVMStructType::isIdentified() const { return getImpl()->isIdentified(); }
-bool LLVMStructType::isOpaque() {
+bool LLVMStructType::isOpaque() const {
   return getImpl()->isIdentified() &&
          (getImpl()->isOpaque() || !getImpl()->isInitialized());
 }

[gysit]

Thanks for the fix!

LGTM