[flang-commits] [flang] [mlir] [mlir][LLVM] Make struct types immutable (PR #116035)

Markus Böck via flang-commits flang-commits at lists.llvm.org
Thu Nov 14 13:27:03 PST 2024


https://github.com/zero9178 updated https://github.com/llvm/llvm-project/pull/116035

>From a547985b50c9a47d6ca7c81a8fd6501dab0f33bd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20B=C3=B6ck?= <markus.boeck02 at gmail.com>
Date: Wed, 13 Nov 2024 12:51:01 +0100
Subject: [PATCH 1/5] [mlir][LLVM] Make struct types immutable

This PR is the implementation of [RFC LINK].

Since the transition to opaque pointers, LLVMs type system is no longer recursive, making the immutability of LLVM's struct type no longer necessary.
Immutable types have much better support (via e.g. TableGen) than mutable types, greatly simplifying the entire codebase.

This PR therefore makes the LLVM dialects struct type an immutable type that is now defined and mostly implemented in TableGen.

Minor API and semantic changes had to be done to support this case, meaning there will likely be downstream breakage. Please see the RFC for more details as to how to mitigate these.
---
 mlir/docs/Dialects/LLVM.md                    |  23 +-
 mlir/include/mlir-c/Dialect/LLVM.h            |  14 -
 mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.h  | 142 +------
 mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td | 108 +++++
 mlir/include/mlir/IR/AttrTypeBase.td          |  17 +
 mlir/lib/CAPI/Dialect/LLVM.cpp                |  20 +-
 .../Conversion/LLVMCommon/TypeConverter.cpp   |  45 +-
 mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp    |   4 +-
 mlir/lib/Dialect/LLVMIR/IR/LLVMTypeSyntax.cpp | 179 +-------
 mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp      |  93 ++---
 mlir/lib/Dialect/LLVMIR/IR/TypeDetail.h       | 390 ------------------
 mlir/lib/Target/LLVMIR/TypeFromLLVM.cpp       |   2 +-
 mlir/lib/Target/LLVMIR/TypeToLLVM.cpp         |  17 +-
 mlir/test/CAPI/llvm.c                         |  92 -----
 mlir/test/Conversion/LLVMCommon/types.mlir    |  35 --
 mlir/test/Dialect/LLVMIR/types-invalid.mlir   |  57 +--
 .../Target/LLVMIR/Import/global-struct.ll     |   8 -
 mlir/test/Target/LLVMIR/llvmir-types.mlir     |   5 +
 mlir/unittests/Dialect/CMakeLists.txt         |   1 -
 mlir/unittests/Dialect/LLVMIR/CMakeLists.txt  |   7 -
 mlir/unittests/Dialect/LLVMIR/LLVMTestBase.h  |  27 --
 .../unittests/Dialect/LLVMIR/LLVMTypeTest.cpp |  19 -
 22 files changed, 195 insertions(+), 1110 deletions(-)
 delete mode 100644 mlir/lib/Dialect/LLVMIR/IR/TypeDetail.h
 delete mode 100644 mlir/test/Conversion/LLVMCommon/types.mlir
 delete mode 100644 mlir/test/Target/LLVMIR/Import/global-struct.ll
 delete mode 100644 mlir/unittests/Dialect/LLVMIR/CMakeLists.txt
 delete mode 100644 mlir/unittests/Dialect/LLVMIR/LLVMTestBase.h
 delete mode 100644 mlir/unittests/Dialect/LLVMIR/LLVMTypeTest.cpp

diff --git a/mlir/docs/Dialects/LLVM.md b/mlir/docs/Dialects/LLVM.md
index fadc81b567b4e4..dc2929b6d786bb 100644
--- a/mlir/docs/Dialects/LLVM.md
+++ b/mlir/docs/Dialects/LLVM.md
@@ -375,33 +375,18 @@ memory. The elements of a structure may be any type that has a size.
 Structure types are represented in a single dedicated class
 mlir::LLVM::LLVMStructType. Internally, the struct type stores a (potentially
 empty) name, a (potentially empty) list of contained types and a bitmask
-indicating whether the struct is named, opaque, packed or uninitialized.
+indicating whether the struct is opaque or packed.
 Structure types that don't have a name are referred to as _literal_ structs.
 Such structures are uniquely identified by their contents. _Identified_ structs
-on the other hand are uniquely identified by the name.
+on the other hand are uniquely identified by the name and contents.
 
 #### Identified Structure Types
 
-Identified structure types are uniqued using their name in a given context.
-Attempting to construct an identified structure with the same name a structure
-that already exists in the context *will result in the existing structure being
-returned*. **MLIR does not auto-rename identified structs in case of name
+Identified structure types are uniqued using their name and contents in a given
+context. **MLIR does not auto-rename identified structs in case of name
 conflicts** because there is no naming scope equivalent to a module in LLVM IR
 since MLIR modules can be arbitrarily nested.
 
-Programmatically, identified structures can be constructed in an _uninitialized_
-state. In this case, they are given a name but the body must be set up by a
-later call, using MLIR's type mutation mechanism. Such uninitialized types can
-be used in type construction, but must be eventually initialized for IR to be
-valid. This mechanism allows for constructing _recursive_ or mutually referring
-structure types: an uninitialized type can be used in its own initialization.
-
-Once the type is initialized, its body cannot be changed anymore. Any further
-attempts to modify the body will fail and return failure to the caller _unless
-the type is initialized with the exact same body_. Type initialization is
-thread-safe; however, if a concurrent thread initializes the type before the
-current thread, the initialization may return failure.
-
 The syntax for identified structure types is as follows.
 
 ```
diff --git a/mlir/include/mlir-c/Dialect/LLVM.h b/mlir/include/mlir-c/Dialect/LLVM.h
index 0e6434073437a5..b23c91c46cdb8c 100644
--- a/mlir/include/mlir-c/Dialect/LLVM.h
+++ b/mlir/include/mlir-c/Dialect/LLVM.h
@@ -81,14 +81,6 @@ MLIR_CAPI_EXPORTED MlirType
 mlirLLVMStructTypeLiteralGetChecked(MlirLocation loc, intptr_t nFieldTypes,
                                     MlirType const *fieldTypes, bool isPacked);
 
-/// Creates an LLVM identified struct type with no body. If a struct type with
-/// this name already exists in the context, returns that type. Use
-/// mlirLLVMStructTypeIdentifiedNewGet to create a fresh struct type,
-/// potentially renaming it. The body should be set separatelty by calling
-/// mlirLLVMStructTypeSetBody, if it isn't set already.
-MLIR_CAPI_EXPORTED MlirType mlirLLVMStructTypeIdentifiedGet(MlirContext ctx,
-                                                            MlirStringRef name);
-
 /// Creates an LLVM identified struct type with no body and a name starting with
 /// the given prefix. If a struct with the exact name as the given prefix
 /// already exists, appends an unspecified suffix to the name so that the name
@@ -100,12 +92,6 @@ MLIR_CAPI_EXPORTED MlirType mlirLLVMStructTypeIdentifiedNewGet(
 MLIR_CAPI_EXPORTED MlirType mlirLLVMStructTypeOpaqueGet(MlirContext ctx,
                                                         MlirStringRef name);
 
-/// Sets the body of the identified struct if it hasn't been set yet. Returns
-/// whether the operation was successful.
-MLIR_CAPI_EXPORTED MlirLogicalResult
-mlirLLVMStructTypeSetBody(MlirType structType, intptr_t nFieldTypes,
-                          MlirType const *fieldTypes, bool isPacked);
-
 enum MlirLLVMCConv {
   MlirLLVMCConvC = 0,
   MlirLLVMCConvFast = 8,
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.h b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.h
index 2ea589a7c4c3bd..a7d53beea3cfa1 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.h
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.h
@@ -73,145 +73,6 @@ DEFINE_TRIVIAL_LLVM_TYPE(LLVMMetadataType, "llvm.metadata");
 
 #undef DEFINE_TRIVIAL_LLVM_TYPE
 
-//===----------------------------------------------------------------------===//
-// LLVMStructType.
-//===----------------------------------------------------------------------===//
-
-/// LLVM dialect structure type representing a collection of different-typed
-/// elements manipulated together. Structured can optionally be packed, meaning
-/// that their elements immediately follow each other in memory without
-/// accounting for potential alignment.
-///
-/// Structure types can be identified (named) or literal. Literal structures
-/// are uniquely represented by the list of types they contain and packedness.
-/// Literal structure types are immutable after construction.
-///
-/// Identified structures are uniquely represented by their name, a string. They
-/// have a mutable component, consisting of the list of types they contain,
-/// the packedness and the opacity bits. Identified structs can be created
-/// without providing the lists of element types, making them suitable to
-/// represent recursive, i.e. self-referring, structures. Identified structs
-/// without body are considered opaque. For such structs, one can set the body.
-/// Identified structs can be created as intentionally-opaque, implying that the
-/// caller does not intend to ever set the body (e.g. forward-declarations of
-/// structs from another module) and wants to disallow further modification of
-/// the body. For intentionally-opaque structs or non-opaque structs with the
-/// body, one is not allowed to set another body (however, one can set exactly
-/// the same body).
-///
-/// Note that the packedness of the struct takes place in uniquing of literal
-/// structs, but does not in uniquing of identified structs.
-class LLVMStructType
-    : public Type::TypeBase<LLVMStructType, Type, detail::LLVMStructTypeStorage,
-                            DataLayoutTypeInterface::Trait,
-                            DestructurableTypeInterface::Trait,
-                            TypeTrait::IsMutable> {
-public:
-  /// Inherit base constructors.
-  using Base::Base;
-
-  static constexpr StringLiteral name = "llvm.struct";
-
-  /// Checks if the given type can be contained in a structure type.
-  static bool isValidElementType(Type type);
-
-  /// Gets or creates an identified struct with the given name in the provided
-  /// context. Note that unlike llvm::StructType::create, this function will
-  /// _NOT_ rename a struct in case a struct with the same name already exists
-  /// in the context. Instead, it will just return the existing struct,
-  /// similarly to the rest of MLIR type ::get methods.
-  static LLVMStructType getIdentified(MLIRContext *context, StringRef name);
-  static LLVMStructType
-  getIdentifiedChecked(function_ref<InFlightDiagnostic()> emitError,
-                       MLIRContext *context, StringRef name);
-
-  /// Gets a new identified struct with the given body. The body _cannot_ be
-  /// changed later. If a struct with the given name already exists, renames
-  /// the struct by appending a `.` followed by a number to the name. Renaming
-  /// happens even if the existing struct has the same body.
-  static LLVMStructType getNewIdentified(MLIRContext *context, StringRef name,
-                                         ArrayRef<Type> elements,
-                                         bool isPacked = false);
-
-  /// Gets or creates a literal struct with the given body in the provided
-  /// context.
-  static LLVMStructType getLiteral(MLIRContext *context, ArrayRef<Type> types,
-                                   bool isPacked = false);
-  static LLVMStructType
-  getLiteralChecked(function_ref<InFlightDiagnostic()> emitError,
-                    MLIRContext *context, ArrayRef<Type> types,
-                    bool isPacked = false);
-
-  /// Gets or creates an intentionally-opaque identified struct. Such a struct
-  /// cannot have its body set. To create an opaque struct with a mutable body,
-  /// use `getIdentified`. Note that unlike llvm::StructType::create, this
-  /// function will _NOT_ rename a struct in case a struct with the same name
-  /// already exists in the context. Instead, it will just return the existing
-  /// struct, similarly to the rest of MLIR type ::get methods.
-  static LLVMStructType getOpaque(StringRef name, MLIRContext *context);
-  static LLVMStructType
-  getOpaqueChecked(function_ref<InFlightDiagnostic()> emitError,
-                   MLIRContext *context, StringRef name);
-
-  /// Set the body of an identified struct. Returns failure if the body could
-  /// not be set, e.g. if the struct already has a body or if it was marked as
-  /// intentionally opaque. This might happen in a multi-threaded context when a
-  /// different thread modified the struct after it was created. Most callers
-  /// are likely to assert this always succeeds, but it is possible to implement
-  /// a local renaming scheme based on the result of this call.
-  LogicalResult setBody(ArrayRef<Type> types, bool isPacked);
-
-  /// Checks if a struct is packed.
-  bool isPacked() const;
-
-  /// Checks if a struct is identified.
-  bool isIdentified() const;
-
-  /// Checks if a struct is opaque.
-  bool isOpaque();
-
-  /// Checks if a struct is initialized.
-  bool isInitialized();
-
-  /// Returns the name of an identified struct.
-  StringRef getName();
-
-  /// Returns the list of element types contained in a non-opaque struct.
-  ArrayRef<Type> getBody() const;
-
-  /// Verifies that the type about to be constructed is well-formed.
-  static LogicalResult
-  verifyInvariants(function_ref<InFlightDiagnostic()> emitError, StringRef,
-                   bool);
-  static LogicalResult
-  verifyInvariants(function_ref<InFlightDiagnostic()> emitError,
-                   ArrayRef<Type> types, bool);
-  using Base::verifyInvariants;
-
-  /// Hooks for DataLayoutTypeInterface. Should not be called directly. Obtain a
-  /// DataLayout instance and query it instead.
-  llvm::TypeSize getTypeSizeInBits(const DataLayout &dataLayout,
-                                   DataLayoutEntryListRef params) const;
-
-  uint64_t getABIAlignment(const DataLayout &dataLayout,
-                           DataLayoutEntryListRef params) const;
-
-  uint64_t getPreferredAlignment(const DataLayout &dataLayout,
-                                 DataLayoutEntryListRef params) const;
-
-  bool areCompatible(DataLayoutEntryListRef oldLayout,
-                     DataLayoutEntryListRef newLayout) const;
-
-  LogicalResult verifyEntries(DataLayoutEntryListRef entries,
-                              Location loc) const;
-
-  /// Destructs the struct into its indexed field types.
-  std::optional<DenseMap<Attribute, Type>> getSubelementIndexMap();
-
-  /// Returns which type is stored at a given integer index within the struct.
-  Type getTypeAtIndex(Attribute index);
-};
-
 //===----------------------------------------------------------------------===//
 // Printing and parsing.
 //===----------------------------------------------------------------------===//
@@ -226,8 +87,9 @@ void printType(Type type, AsmPrinter &printer);
 
 /// Parse any MLIR type or a concise syntax for LLVM types.
 ParseResult parsePrettyLLVMType(AsmParser &p, Type &type);
+ParseResult parsePrettyLLVMType(AsmParser &p, SmallVectorImpl<Type> &types);
 /// Print any MLIR type or a concise syntax for LLVM types.
-void printPrettyLLVMType(AsmPrinter &p, Type type);
+void printPrettyLLVMType(AsmPrinter &p, TypeRange type);
 
 //===----------------------------------------------------------------------===//
 // Utility functions.
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
index 09dd0919c318fb..cab1aa45efea67 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
@@ -117,6 +117,114 @@ def LLVMFunctionType : LLVMType<"LLVMFunction", "func"> {
   }];
 }
 
+//===----------------------------------------------------------------------===//
+// LLVMStructType
+//===----------------------------------------------------------------------===//
+
+def LLVMStructType : LLVMType<"LLVMStruct", "struct", [
+  DeclareTypeInterfaceMethods<DataLayoutTypeInterface,
+    ["areCompatible", "verifyEntries"]>,
+  DeclareTypeInterfaceMethods<DestructurableTypeInterface>
+]> {
+  let summary = "LLVM struct type";
+
+  let description = [{
+    The `!llvm.struct` type is used to represent a sequential collection of
+    types within memory.
+    By default, all elements within the struct are aligned according to their
+    type unless the `$packed` parameter is set to true.
+    In that case, all elements are tightly packed with no padding inbetween
+    elements.
+
+    A struct may additionally contain a name.
+    Two struct types are considered equal if their body matches and they have
+    the same name.
+    Note that this differs from LLVM IR where LLVM would rename structs instead
+    to avoid name clashes, meaning two structs with the same name are also
+    equal.
+
+    Instead of a body, a struct may also be marked as "opaque".
+    This signals that the struct definition is complete and the body of the
+    struct unknown.
+    An opaque struct must have a name.
+  }];
+
+  let parameters = (ins
+    StringRefParameter<"struct name", [{""}]>:$name,
+    OptionalArrayRefParameter<"Type">:$body,
+    ImpliedBoolParameter<>:$packed,
+    ImpliedBoolParameter<>:$opaque
+  );
+
+  // We do not want a builder that may set opaque to true and supply a body
+  // at the same time.
+  let skipDefaultBuilders = 1;
+
+   let builders = [
+     TypeBuilder<(ins
+       "llvm::StringRef":$name,
+       "llvm::ArrayRef<mlir::Type>":$body,
+       CArg<"bool", "false">:$isPacked), [{
+       return Base::get($_ctxt, name, body, isPacked, /*opaque=*/false);
+     }]>
+   ];
+
+  let extraClassDeclaration = [{
+    /// Checks if the given type can be contained in a structure type.
+    static bool isValidElementType(Type type);
+
+    /// Returns true if the struct has a name.
+    bool isIdentified() const {
+      return !getName().empty();
+    }
+
+    bool isPacked() const {
+      return getPacked();
+    }
+
+    bool isOpaque() const {
+      return getOpaque();
+    }
+
+    /// Destructs the struct into its indexed field types.
+    std::optional<DenseMap<Attribute, Type>> getSubelementIndexMap();
+
+    /// Returns which type is stored at a given integer index within the struct.
+    Type getTypeAtIndex(Attribute index);
+
+    /// Gets or creates a literal struct with the given body in the provided
+    /// context.
+    static LLVMStructType getLiteral(MLIRContext *context, ArrayRef<Type> types,
+                                     bool isPacked = false);
+
+    static LLVMStructType
+    getLiteralChecked(function_ref<InFlightDiagnostic()> emitError,
+                      MLIRContext *context, ArrayRef<Type> types,
+                      bool isPacked = false);
+
+    /// Gets or creates an intentionally-opaque identified struct. Such a struct
+    /// cannot have its body set.
+    /// Note that unlike llvm::StructType::create, this function will _NOT_
+    /// rename a struct in case a struct with the same name
+    /// already exists in the context. Instead, it will just return the existing
+    /// struct, similarly to the rest of MLIR type ::get methods.
+    static LLVMStructType getOpaque(StringRef name, MLIRContext *context);
+
+    static LLVMStructType
+    getOpaqueChecked(function_ref<InFlightDiagnostic()> emitError,
+                     MLIRContext *context, StringRef name);
+  }];
+
+  let assemblyFormat = [{
+    `<` (custom<OptionalName>($name)^ `,` ` `)?
+        (`opaque` `` $opaque^) :
+          ( (`packed` $packed^)? `(` (`)`)
+            : (custom<PrettyLLVMType>($body)^ `)`)? )? `>`
+  }];
+
+  let genVerifyDecl = 1;
+}
+
 //===----------------------------------------------------------------------===//
 // LLVMPointerType
 //===----------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/IR/AttrTypeBase.td b/mlir/include/mlir/IR/AttrTypeBase.td
index cbe4f0d67574b3..1a4061fbeac555 100644
--- a/mlir/include/mlir/IR/AttrTypeBase.td
+++ b/mlir/include/mlir/IR/AttrTypeBase.td
@@ -371,6 +371,23 @@ class DefaultValuedParameter<string type, string value, string desc = ""> :
   let defaultValue = value;
 }
 
+// Bool parameter that is true or false depending on whether an optional group
+// is present in the assembly syntax.
+// Example:
+// `<` (`opaque` `` $opaque^)? `>`
+//
+// The 'opaque' parameter will be set to true if the `opaque` keyword is
+// present, false otherwise. This is analogous as to how 'UnitAttr' can be used
+// in op definition syntax.
+class ImpliedBoolParameter<string desc = "">
+  : DefaultValuedParameter<"bool", "false", desc> {
+
+  // Always set to true if the parser is invocated.
+  let parser = "true";
+  // Printing should do nothing.
+  let printer = "";
+}
+
 // For StringRefs, which require allocation.
 class StringRefParameter<string desc = "", string value = ""> :
     AttrOrTypeParameter<"::llvm::StringRef", desc> {
diff --git a/mlir/lib/CAPI/Dialect/LLVM.cpp b/mlir/lib/CAPI/Dialect/LLVM.cpp
index c7082445dd9c27..44139ced24ce84 100644
--- a/mlir/lib/CAPI/Dialect/LLVM.cpp
+++ b/mlir/lib/CAPI/Dialect/LLVM.cpp
@@ -102,28 +102,14 @@ MlirType mlirLLVMStructTypeOpaqueGet(MlirContext ctx, MlirStringRef name) {
   return wrap(LLVMStructType::getOpaque(unwrap(name), unwrap(ctx)));
 }
 
-MlirType mlirLLVMStructTypeIdentifiedGet(MlirContext ctx, MlirStringRef name) {
-  return wrap(LLVMStructType::getIdentified(unwrap(ctx), unwrap(name)));
-}
-
 MlirType mlirLLVMStructTypeIdentifiedNewGet(MlirContext ctx, MlirStringRef name,
                                             intptr_t nFieldTypes,
                                             MlirType const *fieldTypes,
                                             bool isPacked) {
   SmallVector<Type> fields;
-  return wrap(LLVMStructType::getNewIdentified(
-      unwrap(ctx), unwrap(name), unwrapList(nFieldTypes, fieldTypes, fields),
-      isPacked));
-}
-
-MlirLogicalResult mlirLLVMStructTypeSetBody(MlirType structType,
-                                            intptr_t nFieldTypes,
-                                            MlirType const *fieldTypes,
-                                            bool isPacked) {
-  SmallVector<Type> fields;
-  return wrap(
-      cast<LLVM::LLVMStructType>(unwrap(structType))
-          .setBody(unwrapList(nFieldTypes, fieldTypes, fields), isPacked));
+  return wrap(LLVMStructType::get(unwrap(ctx), unwrap(name),
+                                  unwrapList(nFieldTypes, fieldTypes, fields),
+                                  isPacked));
 }
 
 MlirAttribute mlirLLVMDIExpressionElemAttrGet(MlirContext ctx,
diff --git a/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp b/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
index ce91424e7a577e..80d8220715cbb0 100644
--- a/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
+++ b/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
@@ -84,54 +84,13 @@ LLVMTypeConverter::LLVMTypeConverter(MLIRContext *ctx,
       return success();
     }
 
-    if (type.isIdentified()) {
-      auto convertedType = LLVM::LLVMStructType::getIdentified(
-          type.getContext(), ("_Converted." + type.getName()).str());
-
-      SmallVectorImpl<Type> &recursiveStack = getCurrentThreadRecursiveStack();
-      if (llvm::count(recursiveStack, type)) {
-        results.push_back(convertedType);
-        return success();
-      }
-      recursiveStack.push_back(type);
-      auto popConversionCallStack = llvm::make_scope_exit(
-          [&recursiveStack]() { recursiveStack.pop_back(); });
-
-      SmallVector<Type> convertedElemTypes;
-      convertedElemTypes.reserve(type.getBody().size());
-      if (failed(convertTypes(type.getBody(), convertedElemTypes)))
-        return std::nullopt;
-
-      // If the converted type has not been initialized yet, just set its body
-      // to be the converted arguments and return.
-      if (!convertedType.isInitialized()) {
-        if (failed(
-                convertedType.setBody(convertedElemTypes, type.isPacked()))) {
-          return failure();
-        }
-        results.push_back(convertedType);
-        return success();
-      }
-
-      // If it has been initialized, has the same body and packed bit, just use
-      // it. This ensures that recursive structs keep being recursive rather
-      // than including a non-updated name.
-      if (TypeRange(convertedType.getBody()) == TypeRange(convertedElemTypes) &&
-          convertedType.isPacked() == type.isPacked()) {
-        results.push_back(convertedType);
-        return success();
-      }
-
-      return failure();
-    }
-
     SmallVector<Type> convertedSubtypes;
     convertedSubtypes.reserve(type.getBody().size());
     if (failed(convertTypes(type.getBody(), convertedSubtypes)))
       return std::nullopt;
 
-    results.push_back(LLVM::LLVMStructType::getLiteral(
-        type.getContext(), convertedSubtypes, type.isPacked()));
+    results.push_back(LLVM::LLVMStructType::get(
+        type.getContext(), type.getName(), convertedSubtypes, type.isPacked()));
     return success();
   });
   addConversion([&](LLVM::LLVMArrayType type) -> std::optional<Type> {
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index d4f8c4c1faf956..444e9a461eb03c 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -12,7 +12,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "mlir/Dialect/LLVMIR/LLVMDialect.h"
-#include "TypeDetail.h"
 #include "mlir/Dialect/LLVMIR/LLVMAttrs.h"
 #include "mlir/Dialect/LLVMIR/LLVMInterfaces.h"
 #include "mlir/Dialect/LLVMIR/LLVMTypes.h"
@@ -3521,8 +3520,7 @@ void LLVMDialect::initialize() {
            LLVMPPCFP128Type,
            LLVMTokenType,
            LLVMLabelType,
-           LLVMMetadataType,
-           LLVMStructType>();
+           LLVMMetadataType>();
   // clang-format on
   registerTypes();
 
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypeSyntax.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypeSyntax.cpp
index 9537f7c40dd4be..6bcf952a0fb39b 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypeSyntax.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypeSyntax.cpp
@@ -51,41 +51,6 @@ 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) {
-  FailureOr<AsmPrinter::CyclicPrintReset> cyclicPrint;
-
-  printer << "<";
-  if (type.isIdentified()) {
-    cyclicPrint = printer.tryStartCyclicPrint(type);
-
-    printer << '"' << type.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)) {
-      printer << '>';
-      return;
-    }
-    printer << ", ";
-  }
-
-  if (type.isIdentified() && type.isOpaque()) {
-    printer << "opaque>";
-    return;
-  }
-
-  if (type.isPacked())
-    printer << "packed ";
-
-  // Put the current type on stack to avoid infinite recursion.
-  printer << '(';
-  llvm::interleaveComma(type.getBody(), printer.getStream(),
-                        [&](Type subtype) { dispatchPrint(printer, subtype); });
-  printer << ')';
-  printer << '>';
-}
-
 /// Prints the given LLVM dialect type recursively. This leverages closedness of
 /// the LLVM dialect type system to avoid printing the dialect prefix
 /// repeatedly. For recursive structures, only prints the name of the structure
@@ -105,11 +70,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); });
 }
 
 //===----------------------------------------------------------------------===//
@@ -156,130 +118,6 @@ static Type parseVectorType(AsmParser &parser) {
   return parser.getChecked<LLVMFixedVectorType>(loc, elementType, dims[0]);
 }
 
-/// Attempts to set the body of an identified structure type. Reports a parsing
-/// error at `subtypesLoc` in case of failure.
-static LLVMStructType trySetStructBody(LLVMStructType type,
-                                       ArrayRef<Type> subtypes, bool isPacked,
-                                       AsmParser &parser, SMLoc subtypesLoc) {
-  for (Type t : subtypes) {
-    if (!LLVMStructType::isValidElementType(t)) {
-      parser.emitError(subtypesLoc)
-          << "invalid LLVM structure element type: " << t;
-      return LLVMStructType();
-    }
-  }
-
-  if (succeeded(type.setBody(subtypes, isPacked)))
-    return type;
-
-  parser.emitError(subtypesLoc)
-      << "identified type already used with a different body";
-  return LLVMStructType();
-}
-
-/// Parses an LLVM dialect structure type.
-///   llvm-type ::= `struct<` (string-literal `,`)? `packed`?
-///                 `(` llvm-type-list `)` `>`
-///               | `struct<` string-literal `>`
-///               | `struct<` string-literal `, opaque>`
-static LLVMStructType parseStructType(AsmParser &parser) {
-  Location loc = parser.getEncodedSourceLoc(parser.getCurrentLocation());
-
-  if (failed(parser.parseLess()))
-    return LLVMStructType();
-
-  // If we are parsing a self-reference to a recursive struct, i.e. the parsing
-  // stack already contains a struct with the same identifier, bail out after
-  // the name.
-  std::string name;
-  bool isIdentified = succeeded(parser.parseOptionalString(&name));
-  if (isIdentified) {
-    SMLoc greaterLoc = parser.getCurrentLocation();
-    if (succeeded(parser.parseOptionalGreater())) {
-      auto type = LLVMStructType::getIdentifiedChecked(
-          [loc] { return emitError(loc); }, loc.getContext(), name);
-      if (succeeded(parser.tryStartCyclicParse(type))) {
-        parser.emitError(
-            greaterLoc,
-            "struct without a body only allowed in a recursive struct");
-        return nullptr;
-      }
-
-      return type;
-    }
-    if (failed(parser.parseComma()))
-      return LLVMStructType();
-  }
-
-  // Handle intentionally opaque structs.
-  SMLoc kwLoc = parser.getCurrentLocation();
-  if (succeeded(parser.parseOptionalKeyword("opaque"))) {
-    if (!isIdentified)
-      return parser.emitError(kwLoc, "only identified structs can be opaque"),
-             LLVMStructType();
-    if (failed(parser.parseGreater()))
-      return LLVMStructType();
-    auto type = LLVMStructType::getOpaqueChecked(
-        [loc] { return emitError(loc); }, loc.getContext(), name);
-    if (!type.isOpaque()) {
-      parser.emitError(kwLoc, "redeclaring defined struct as opaque");
-      return LLVMStructType();
-    }
-    return type;
-  }
-
-  FailureOr<AsmParser::CyclicParseReset> cyclicParse;
-  if (isIdentified) {
-    cyclicParse =
-        parser.tryStartCyclicParse(LLVMStructType::getIdentifiedChecked(
-            [loc] { return emitError(loc); }, loc.getContext(), name));
-    if (failed(cyclicParse)) {
-      parser.emitError(kwLoc,
-                       "identifier already used for an enclosing struct");
-      return nullptr;
-    }
-  }
-
-  // Check for packedness.
-  bool isPacked = succeeded(parser.parseOptionalKeyword("packed"));
-  if (failed(parser.parseLParen()))
-    return LLVMStructType();
-
-  // Fast pass for structs with zero subtypes.
-  if (succeeded(parser.parseOptionalRParen())) {
-    if (failed(parser.parseGreater()))
-      return LLVMStructType();
-    if (!isIdentified)
-      return LLVMStructType::getLiteralChecked([loc] { return emitError(loc); },
-                                               loc.getContext(), {}, isPacked);
-    auto type = LLVMStructType::getIdentifiedChecked(
-        [loc] { return emitError(loc); }, loc.getContext(), name);
-    return trySetStructBody(type, {}, isPacked, parser, kwLoc);
-  }
-
-  // Parse subtypes. For identified structs, put the identifier of the struct on
-  // the stack to support self-references in the recursive calls.
-  SmallVector<Type, 4> subtypes;
-  SMLoc subtypesLoc = parser.getCurrentLocation();
-  do {
-    Type type;
-    if (dispatchParse(parser, type))
-      return LLVMStructType();
-    subtypes.push_back(type);
-  } while (succeeded(parser.parseOptionalComma()));
-
-  if (parser.parseRParen() || parser.parseGreater())
-    return LLVMStructType();
-
-  // Construct the struct with body.
-  if (!isIdentified)
-    return LLVMStructType::getLiteralChecked(
-        [loc] { return emitError(loc); }, loc.getContext(), subtypes, isPacked);
-  auto type = LLVMStructType::getIdentifiedChecked(
-      [loc] { return emitError(loc); }, loc.getContext(), name);
-  return trySetStructBody(type, subtypes, isPacked, parser, subtypesLoc);
-}
-
 /// Parses a type appearing inside another LLVM dialect-compatible type. This
 /// will try to parse any type in full form (including types with the `!llvm`
 /// prefix), and on failure fall back to parsing the short-hand version of the
@@ -316,7 +154,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([&] {
@@ -348,6 +186,13 @@ ParseResult LLVM::parsePrettyLLVMType(AsmParser &p, Type &type) {
   return dispatchParse(p, type);
 }
 
-void LLVM::printPrettyLLVMType(AsmPrinter &p, Type type) {
-  return dispatchPrint(p, type);
+ParseResult LLVM::parsePrettyLLVMType(AsmParser &p,
+                                      SmallVectorImpl<Type> &types) {
+  return p.parseCommaSeparatedList(
+      [&] { return dispatchParse(p, types.emplace_back()); });
+}
+
+void LLVM::printPrettyLLVMType(AsmPrinter &p, TypeRange types) {
+  interleaveComma(types, p.getStream(),
+                  [&](Type type) { return dispatchPrint(p, type); });
 }
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
index 1bed3fa48b30d7..d4cf2baa06d123 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
@@ -11,8 +11,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "TypeDetail.h"
-
 #include "mlir/Dialect/LLVMIR/LLVMDialect.h"
 #include "mlir/Dialect/LLVMIR/LLVMTypes.h"
 #include "mlir/IR/BuiltinTypes.h"
@@ -29,6 +27,17 @@ using namespace mlir::LLVM;
 
 constexpr const static uint64_t kBitsInByte = 8;
 
+static OptionalParseResult parseOptionalName(AsmParser &p, std::string &name) {
+  if (succeeded(p.parseOptionalString(&name)))
+    return std::nullopt;
+
+  return success();
+}
+
+static void printOptionalName(AsmPrinter &p, StringRef name) {
+  p.printString(name);
+}
+
 //===----------------------------------------------------------------------===//
 // custom<FunctionTypes>
 //===----------------------------------------------------------------------===//
@@ -421,85 +430,46 @@ bool LLVMStructType::isValidElementType(Type type) {
                     LLVMFunctionType, LLVMTokenType>(type);
 }
 
-LLVMStructType LLVMStructType::getIdentified(MLIRContext *context,
-                                             StringRef name) {
-  return Base::get(context, name, /*opaque=*/false);
-}
-
-LLVMStructType LLVMStructType::getIdentifiedChecked(
-    function_ref<InFlightDiagnostic()> emitError, MLIRContext *context,
-    StringRef name) {
-  return Base::getChecked(emitError, context, name, /*opaque=*/false);
-}
-
-LLVMStructType LLVMStructType::getNewIdentified(MLIRContext *context,
-                                                StringRef name,
-                                                ArrayRef<Type> elements,
-                                                bool isPacked) {
-  std::string stringName = name.str();
-  unsigned counter = 0;
-  do {
-    auto type = LLVMStructType::getIdentified(context, stringName);
-    if (type.isInitialized() || failed(type.setBody(elements, isPacked))) {
-      counter += 1;
-      stringName = (Twine(name) + "." + std::to_string(counter)).str();
-      continue;
-    }
-    return type;
-  } while (true);
-}
-
 LLVMStructType LLVMStructType::getLiteral(MLIRContext *context,
                                           ArrayRef<Type> types, bool isPacked) {
-  return Base::get(context, types, isPacked);
+  return Base::get(context, "", types, isPacked, /*opaque=*/false);
 }
 
 LLVMStructType
 LLVMStructType::getLiteralChecked(function_ref<InFlightDiagnostic()> emitError,
                                   MLIRContext *context, ArrayRef<Type> types,
                                   bool isPacked) {
-  return Base::getChecked(emitError, context, types, isPacked);
+  return Base::getChecked(emitError, context, "", types, isPacked,
+                          /*opaque=*/false);
 }
 
 LLVMStructType LLVMStructType::getOpaque(StringRef name, MLIRContext *context) {
-  return Base::get(context, name, /*opaque=*/true);
+  return Base::get(context, name, /*types=*/std::nullopt, /*packed=*/false,
+                   /*opaque=*/true);
 }
 
 LLVMStructType
 LLVMStructType::getOpaqueChecked(function_ref<InFlightDiagnostic()> emitError,
                                  MLIRContext *context, StringRef name) {
-  return Base::getChecked(emitError, context, name, /*opaque=*/true);
-}
-
-LogicalResult LLVMStructType::setBody(ArrayRef<Type> types, bool isPacked) {
-  assert(isIdentified() && "can only set bodies of identified structs");
-  assert(llvm::all_of(types, LLVMStructType::isValidElementType) &&
-         "expected valid body types");
-  return Base::mutate(types, isPacked);
-}
-
-bool LLVMStructType::isPacked() const { return getImpl()->isPacked(); }
-bool LLVMStructType::isIdentified() const { return getImpl()->isIdentified(); }
-bool LLVMStructType::isOpaque() {
-  return getImpl()->isIdentified() &&
-         (getImpl()->isOpaque() || !getImpl()->isInitialized());
-}
-bool LLVMStructType::isInitialized() { return getImpl()->isInitialized(); }
-StringRef LLVMStructType::getName() { return getImpl()->getIdentifier(); }
-ArrayRef<Type> LLVMStructType::getBody() const {
-  return isIdentified() ? getImpl()->getIdentifiedStructBody()
-                        : getImpl()->getTypeList();
+  return Base::getChecked(emitError, context, name, /*types=*/std::nullopt,
+                          /*packed=*/false, /*opaque=*/true);
 }
 
 LogicalResult
-LLVMStructType::verifyInvariants(function_ref<InFlightDiagnostic()>, StringRef,
-                                 bool) {
-  return success();
-}
+LLVMStructType::verify(function_ref<InFlightDiagnostic()> emitError,
+                       StringRef name, ArrayRef<Type> types, bool packed,
+                       bool opaque) {
+  if (opaque) {
+    if (packed)
+      return emitError() << "opaque struct cannot be packed";
+
+    if (name.empty())
+      return emitError() << "only identified structs can be opaque";
+
+    if (!types.empty())
+      return emitError() << "opaque struct must not have a body";
+  }
 
-LogicalResult
-LLVMStructType::verifyInvariants(function_ref<InFlightDiagnostic()> emitError,
-                                 ArrayRef<Type> types, bool) {
   for (Type t : types)
     if (!isValidElementType(t))
       return emitError() << "invalid LLVM structure element type: " << t;
@@ -1026,6 +996,7 @@ void LLVMDialect::registerTypes() {
   addTypes<
 #define GET_TYPEDEF_LIST
 #include "mlir/Dialect/LLVMIR/LLVMTypes.cpp.inc"
+
       >();
 }
 
diff --git a/mlir/lib/Dialect/LLVMIR/IR/TypeDetail.h b/mlir/lib/Dialect/LLVMIR/IR/TypeDetail.h
deleted file mode 100644
index 8767b1c3ffc5bd..00000000000000
--- a/mlir/lib/Dialect/LLVMIR/IR/TypeDetail.h
+++ /dev/null
@@ -1,390 +0,0 @@
-//===- TypeDetail.h - Details of MLIR LLVM dialect types --------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-//
-// This file contains implementation details, such as storage structures, of
-// MLIR LLVM dialect types.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef DIALECT_LLVMIR_IR_TYPEDETAIL_H
-#define DIALECT_LLVMIR_IR_TYPEDETAIL_H
-
-#include "mlir/Dialect/LLVMIR/LLVMTypes.h"
-#include "mlir/IR/TypeSupport.h"
-#include "mlir/IR/Types.h"
-
-#include "llvm/ADT/Bitfields.h"
-#include "llvm/ADT/PointerIntPair.h"
-
-namespace mlir {
-namespace LLVM {
-namespace detail {
-
-//===----------------------------------------------------------------------===//
-// LLVMStructTypeStorage.
-//===----------------------------------------------------------------------===//
-
-/// Type storage for LLVM structure types.
-///
-/// Structures are uniqued using:
-/// - a bit indicating whether a struct is literal or identified;
-/// - for identified structs, in addition to the bit:
-///   - a string identifier;
-/// - for literal structs, in addition to the bit:
-///   - a list of contained types;
-///   - a bit indicating whether the literal struct is packed.
-///
-/// Identified structures only have a mutable component consisting of:
-///   - a list of contained types;
-///   - a bit indicating whether the identified struct is packed;
-///   - a bit indicating whether the identified struct is intentionally opaque;
-///   - a bit indicating whether the identified struct has been initialized.
-/// Uninitialized structs are considered opaque by the user, and can be mutated.
-/// Initialized and still opaque structs cannot be mutated.
-///
-/// The struct storage consists of:
-///   - immutable part:
-///     - a pointer to the first element of the key (character for identified
-///       structs, type for literal structs);
-///     - the number of elements in the key packed together with bits indicating
-///       whether a type is literal or identified, and the packedness bit for
-///       literal structs only;
-///   - mutable part:
-///     - a pointer to the first contained type for identified structs only;
-///     - the number of contained types packed together with bits of the mutable
-///       component, for identified structs only.
-struct LLVMStructTypeStorage : public TypeStorage {
-public:
-  /// Construction/uniquing key class for LLVM dialect structure storage. Note
-  /// that this is a transient helper data structure that is NOT stored.
-  /// Therefore, it intentionally avoids bit manipulation and type erasure in
-  /// pointers to make manipulation more straightforward. Not all elements of
-  /// the key participate in uniquing, but all elements participate in
-  /// construction.
-  class Key {
-  public:
-    /// Constructs a key for an identified struct.
-    Key(StringRef name, bool opaque, ArrayRef<Type> types = std::nullopt)
-        : types(types), name(name), identified(true), packed(false),
-          opaque(opaque) {}
-    /// Constructs a key for a literal struct.
-    Key(ArrayRef<Type> types, bool packed)
-        : types(types), identified(false), packed(packed), opaque(false) {}
-
-    /// Checks a specific property of the struct.
-    bool isIdentified() const { return identified; }
-    bool isPacked() const {
-      assert(!isIdentified() &&
-             "'packed' bit is not part of the key for identified structs");
-      return packed;
-    }
-    bool isOpaque() const {
-      assert(isIdentified() &&
-             "'opaque' bit is meaningless on literal structs");
-      return opaque;
-    }
-
-    /// Returns the identifier of a key for identified structs.
-    StringRef getIdentifier() const {
-      assert(isIdentified() &&
-             "non-identified struct key cannot have an identifier");
-      return name;
-    }
-
-    /// Returns the list of type contained in the key of a literal struct.
-    ArrayRef<Type> getTypeList() const {
-      assert(!isIdentified() &&
-             "identified struct key cannot have a type list");
-      return types;
-    }
-
-    /// Returns the list of type contained in an identified struct.
-    ArrayRef<Type> getIdentifiedStructBody() const {
-      assert(isIdentified() &&
-             "requested struct body on a non-identified struct");
-      return types;
-    }
-
-    /// Returns the hash value of the key. This combines various flags into a
-    /// single value: the identified flag sets the first bit, and the packedness
-    /// flag sets the second bit. Opacity bit is only used for construction and
-    /// does not participate in uniquing.
-    llvm::hash_code hashValue() const {
-      constexpr static unsigned kIdentifiedHashFlag = 1;
-      constexpr static unsigned kPackedHashFlag = 2;
-
-      unsigned flags = 0;
-      if (isIdentified()) {
-        flags |= kIdentifiedHashFlag;
-        return llvm::hash_combine(flags, getIdentifier());
-      }
-      if (isPacked())
-        flags |= kPackedHashFlag;
-      return llvm::hash_combine(flags, getTypeList());
-    }
-
-    /// Compares two keys.
-    bool operator==(const Key &other) const {
-      if (isIdentified())
-        return other.isIdentified() && other.getIdentifier() == getIdentifier();
-
-      return !other.isIdentified() && other.isPacked() == isPacked() &&
-             other.getTypeList() == getTypeList();
-    }
-
-    /// Copies dynamically-sized components of the key into the given allocator.
-    Key copyIntoAllocator(TypeStorageAllocator &allocator) const {
-      if (isIdentified())
-        return Key(allocator.copyInto(name), opaque);
-      return Key(allocator.copyInto(types), packed);
-    }
-
-  private:
-    ArrayRef<Type> types;
-    StringRef name;
-    bool identified;
-    bool packed;
-    bool opaque;
-  };
-  using KeyTy = Key;
-
-  /// Returns the string identifier of an identified struct.
-  StringRef getIdentifier() const {
-    assert(isIdentified() && "requested identifier on a non-identified struct");
-    return StringRef(static_cast<const char *>(keyPtr), keySize());
-  }
-
-  /// Returns the list of types (partially) identifying a literal struct.
-  ArrayRef<Type> getTypeList() const {
-    // If this triggers, use getIdentifiedStructBody() instead.
-    assert(!isIdentified() && "requested typelist on an identified struct");
-    return ArrayRef<Type>(static_cast<const Type *>(keyPtr), keySize());
-  }
-
-  /// Returns the list of types contained in an identified struct.
-  ArrayRef<Type> getIdentifiedStructBody() const {
-    // If this triggers, use getTypeList() instead.
-    assert(isIdentified() &&
-           "requested struct body on a non-identified struct");
-    return ArrayRef<Type>(identifiedBodyArray, identifiedBodySize());
-  }
-
-  /// Checks whether the struct is identified.
-  bool isIdentified() const {
-    return llvm::Bitfield::get<KeyFlagIdentified>(keySizeAndFlags);
-  }
-
-  /// Checks whether the struct is packed (both literal and identified structs).
-  bool isPacked() const {
-    return isIdentified() ? llvm::Bitfield::get<MutableFlagPacked>(
-                                identifiedBodySizeAndFlags)
-                          : llvm::Bitfield::get<KeyFlagPacked>(keySizeAndFlags);
-  }
-
-  /// Checks whether a struct is marked as intentionally opaque (an
-  /// uninitialized struct is also considered opaque by the user, call
-  /// isInitialized to check that).
-  bool isOpaque() const {
-    return llvm::Bitfield::get<MutableFlagOpaque>(identifiedBodySizeAndFlags);
-  }
-
-  /// Checks whether an identified struct has been explicitly initialized either
-  /// by setting its body or by marking it as intentionally opaque.
-  bool isInitialized() const {
-    return llvm::Bitfield::get<MutableFlagInitialized>(
-        identifiedBodySizeAndFlags);
-  }
-
-  /// Constructs the storage from the given key. This sets up the uniquing key
-  /// components and optionally the mutable component if they construction key
-  /// has the relevant information. In the latter case, the struct is considered
-  /// as initialized and can no longer be mutated.
-  LLVMStructTypeStorage(const KeyTy &key) {
-    if (!key.isIdentified()) {
-      ArrayRef<Type> types = key.getTypeList();
-      keyPtr = static_cast<const void *>(types.data());
-      setKeySize(types.size());
-      llvm::Bitfield::set<KeyFlagPacked>(keySizeAndFlags, key.isPacked());
-      return;
-    }
-
-    StringRef name = key.getIdentifier();
-    keyPtr = static_cast<const void *>(name.data());
-    setKeySize(name.size());
-    llvm::Bitfield::set<KeyFlagIdentified>(keySizeAndFlags, true);
-
-    // If the struct is being constructed directly as opaque, mark it as
-    // initialized.
-    llvm::Bitfield::set<MutableFlagInitialized>(identifiedBodySizeAndFlags,
-                                                key.isOpaque());
-    llvm::Bitfield::set<MutableFlagOpaque>(identifiedBodySizeAndFlags,
-                                           key.isOpaque());
-  }
-
-  /// Hook into the type uniquing infrastructure.
-  bool operator==(const KeyTy &other) const { return getAsKey() == other; };
-  static llvm::hash_code hashKey(const KeyTy &key) { return key.hashValue(); }
-  static LLVMStructTypeStorage *construct(TypeStorageAllocator &allocator,
-                                          const KeyTy &key) {
-    return new (allocator.allocate<LLVMStructTypeStorage>())
-        LLVMStructTypeStorage(key.copyIntoAllocator(allocator));
-  }
-
-  /// Sets the body of an identified struct. If the struct is already
-  /// initialized, succeeds only if the body is equal to the current body. Fails
-  /// if the struct is marked as intentionally opaque. The struct will be marked
-  /// as initialized as a result of this operation and can no longer be changed.
-  LogicalResult mutate(TypeStorageAllocator &allocator, ArrayRef<Type> body,
-                       bool packed) {
-    if (!isIdentified())
-      return failure();
-    if (isInitialized())
-      return success(!isOpaque() && body == getIdentifiedStructBody() &&
-                     packed == isPacked());
-
-    llvm::Bitfield::set<MutableFlagInitialized>(identifiedBodySizeAndFlags,
-                                                true);
-    llvm::Bitfield::set<MutableFlagPacked>(identifiedBodySizeAndFlags, packed);
-
-    ArrayRef<Type> typesInAllocator = allocator.copyInto(body);
-    identifiedBodyArray = typesInAllocator.data();
-    setIdentifiedBodySize(typesInAllocator.size());
-
-    return success();
-  }
-
-  /// Returns the key for the current storage.
-  Key getAsKey() const {
-    if (isIdentified())
-      return Key(getIdentifier(), isOpaque(), getIdentifiedStructBody());
-    return Key(getTypeList(), isPacked());
-  }
-
-private:
-  /// Returns the number of elements in the key.
-  unsigned keySize() const {
-    return llvm::Bitfield::get<KeySize>(keySizeAndFlags);
-  }
-
-  /// Sets the number of elements in the key.
-  void setKeySize(unsigned value) {
-    llvm::Bitfield::set<KeySize>(keySizeAndFlags, value);
-  }
-
-  /// Returns the number of types contained in an identified struct.
-  unsigned identifiedBodySize() const {
-    return llvm::Bitfield::get<MutableSize>(identifiedBodySizeAndFlags);
-  }
-  /// Sets the number of types contained in an identified struct.
-  void setIdentifiedBodySize(unsigned value) {
-    llvm::Bitfield::set<MutableSize>(identifiedBodySizeAndFlags, value);
-  }
-
-  /// Bitfield elements for `keyAndSizeFlags`:
-  ///   - bit 0: identified key flag;
-  ///   - bit 1: packed key flag;
-  ///   - bits 2..bitwidth(unsigned): size of the key.
-  using KeyFlagIdentified =
-      llvm::Bitfield::Element<bool, /*Offset=*/0, /*Size=*/1>;
-  using KeyFlagPacked = llvm::Bitfield::Element<bool, /*Offset=*/1, /*Size=*/1>;
-  using KeySize =
-      llvm::Bitfield::Element<unsigned, /*Offset=*/2,
-                              std::numeric_limits<unsigned>::digits - 2>;
-
-  /// Bitfield elements for `identifiedBodySizeAndFlags`:
-  ///   - bit 0: opaque flag;
-  ///   - bit 1: packed mutable flag;
-  ///   - bit 2: initialized flag;
-  ///   - bits 3..bitwidth(unsigned): size of the identified body.
-  using MutableFlagOpaque =
-      llvm::Bitfield::Element<bool, /*Offset=*/0, /*Size=*/1>;
-  using MutableFlagPacked =
-      llvm::Bitfield::Element<bool, /*Offset=*/1, /*Size=*/1>;
-  using MutableFlagInitialized =
-      llvm::Bitfield::Element<bool, /*Offset=*/2, /*Size=*/1>;
-  using MutableSize =
-      llvm::Bitfield::Element<unsigned, /*Offset=*/3,
-                              std::numeric_limits<unsigned>::digits - 3>;
-
-  /// Pointer to the first element of the uniquing key.
-  // Note: cannot use PointerUnion because bump-ptr allocator does not guarantee
-  // address alignment.
-  const void *keyPtr = nullptr;
-
-  /// Pointer to the first type contained in an identified struct.
-  const Type *identifiedBodyArray = nullptr;
-
-  /// Size of the uniquing key combined with identified/literal and
-  /// packedness bits. Must only be used through the Key* bitfields.
-  unsigned keySizeAndFlags = 0;
-
-  /// Number of the types contained in an identified struct combined with
-  /// mutable flags. Must only be used through the Mutable* bitfields.
-  unsigned identifiedBodySizeAndFlags = 0;
-};
-} // end namespace detail
-} // end namespace LLVM
-
-/// Allow walking and replacing the subelements of a LLVMStructTypeStorage key.
-template <>
-struct AttrTypeSubElementHandler<LLVM::detail::LLVMStructTypeStorage::Key> {
-  static void walk(const LLVM::detail::LLVMStructTypeStorage::Key &param,
-                   AttrTypeImmediateSubElementWalker &walker) {
-    if (param.isIdentified())
-      walker.walkRange(param.getIdentifiedStructBody());
-    else
-      walker.walkRange(param.getTypeList());
-  }
-  static FailureOr<LLVM::detail::LLVMStructTypeStorage::Key>
-  replace(const LLVM::detail::LLVMStructTypeStorage::Key &param,
-          AttrSubElementReplacements &attrRepls,
-          TypeSubElementReplacements &typeRepls) {
-    // TODO: It's not clear how we support replacing sub-elements of mutable
-    // types.
-    if (param.isIdentified())
-      return failure();
-
-    return LLVM::detail::LLVMStructTypeStorage::Key(
-        typeRepls.take_front(param.getTypeList().size()), param.isPacked());
-  }
-};
-
-namespace LLVM {
-namespace detail {
-//===----------------------------------------------------------------------===//
-// LLVMTypeAndSizeStorage.
-//===----------------------------------------------------------------------===//
-
-/// Common storage used for LLVM dialect types that need an element type and a
-/// number: arrays, fixed and scalable vectors. The actual semantics of the
-/// type is defined by its kind.
-struct LLVMTypeAndSizeStorage : public TypeStorage {
-  using KeyTy = std::tuple<Type, unsigned>;
-
-  LLVMTypeAndSizeStorage(const KeyTy &key)
-      : elementType(std::get<0>(key)), numElements(std::get<1>(key)) {}
-
-  static LLVMTypeAndSizeStorage *construct(TypeStorageAllocator &allocator,
-                                           const KeyTy &key) {
-    return new (allocator.allocate<LLVMTypeAndSizeStorage>())
-        LLVMTypeAndSizeStorage(key);
-  }
-
-  bool operator==(const KeyTy &key) const {
-    return std::make_tuple(elementType, numElements) == key;
-  }
-
-  Type elementType;
-  unsigned numElements;
-};
-
-} // namespace detail
-} // namespace LLVM
-} // namespace mlir
-
-#endif // DIALECT_LLVMIR_IR_TYPEDETAIL_H
diff --git a/mlir/lib/Target/LLVMIR/TypeFromLLVM.cpp b/mlir/lib/Target/LLVMIR/TypeFromLLVM.cpp
index ea990ca7aefbe0..f028dc80d707df 100644
--- a/mlir/lib/Target/LLVMIR/TypeFromLLVM.cpp
+++ b/mlir/lib/Target/LLVMIR/TypeFromLLVM.cpp
@@ -116,7 +116,7 @@ class TypeFromLLVMIRTranslatorImpl {
     // using getIdentified is not possible, as type names in LLVM are not
     // guaranteed to be unique.
     translateTypes(type->subtypes(), subtypes);
-    LLVM::LLVMStructType translated = LLVM::LLVMStructType::getNewIdentified(
+    LLVM::LLVMStructType translated = LLVM::LLVMStructType::get(
         &context, type->getName(), subtypes, type->isPacked());
     knownTranslations.try_emplace(type, translated);
     return translated;
diff --git a/mlir/lib/Target/LLVMIR/TypeToLLVM.cpp b/mlir/lib/Target/LLVMIR/TypeToLLVM.cpp
index c7a533eddce84b..cc9128b8218282 100644
--- a/mlir/lib/Target/LLVMIR/TypeToLLVM.cpp
+++ b/mlir/lib/Target/LLVMIR/TypeToLLVM.cpp
@@ -113,22 +113,19 @@ class TypeToLLVMIRTranslatorImpl {
   /// structs. This will _create_ a new identified structure every time, use
   /// `convertType` if a structure with the same name must be looked up instead.
   llvm::Type *translate(LLVM::LLVMStructType type) {
+    if (type.isOpaque())
+      return llvm::StructType::create(context, type.getName());
+
     SmallVector<llvm::Type *, 8> subtypes;
-    if (!type.isIdentified()) {
-      translateTypes(type.getBody(), subtypes);
+    translateTypes(type.getBody(), subtypes);
+    if (!type.isIdentified())
       return llvm::StructType::get(context, subtypes, type.isPacked());
-    }
 
-    llvm::StructType *structType =
-        llvm::StructType::create(context, type.getName());
+    llvm::StructType *structType = llvm::StructType::create(
+        context, subtypes, type.getName(), type.isPacked());
     // Mark the type we just created as known so that recursive calls can pick
     // it up and use directly.
     knownTranslations.try_emplace(type, structType);
-    if (type.isOpaque())
-      return structType;
-
-    translateTypes(type.getBody(), subtypes);
-    structType->setBody(subtypes, type.isPacked());
     return structType;
   }
 
diff --git a/mlir/test/CAPI/llvm.c b/mlir/test/CAPI/llvm.c
index 12a436ad12fc4c..b955cde3a25fe9 100644
--- a/mlir/test/CAPI/llvm.c
+++ b/mlir/test/CAPI/llvm.c
@@ -126,98 +126,6 @@ static int testStructTypeCreation(MlirContext ctx) {
     return 2;
   }
 
-  // CHECK: !llvm.struct<"foo", opaque>
-  // CHECK: !llvm.struct<"bar", opaque>
-  mlirTypeDump(mlirLLVMStructTypeIdentifiedGet(
-      ctx, mlirStringRefCreateFromCString("foo")));
-  mlirTypeDump(mlirLLVMStructTypeIdentifiedGet(
-      ctx, mlirStringRefCreateFromCString("bar")));
-
-  if (!mlirTypeEqual(mlirLLVMStructTypeIdentifiedGet(
-                         ctx, mlirStringRefCreateFromCString("foo")),
-                     mlirLLVMStructTypeIdentifiedGet(
-                         ctx, mlirStringRefCreateFromCString("foo")))) {
-    return 3;
-  }
-  if (mlirTypeEqual(mlirLLVMStructTypeIdentifiedGet(
-                        ctx, mlirStringRefCreateFromCString("foo")),
-                    mlirLLVMStructTypeIdentifiedGet(
-                        ctx, mlirStringRefCreateFromCString("bar")))) {
-    return 4;
-  }
-
-  MlirType fooStruct = mlirLLVMStructTypeIdentifiedGet(
-      ctx, mlirStringRefCreateFromCString("foo"));
-  MlirStringRef name = mlirLLVMStructTypeGetIdentifier(fooStruct);
-  if (memcmp(name.data, "foo", name.length))
-    return 5;
-  if (!mlirLLVMStructTypeIsOpaque(fooStruct))
-    return 6;
-
-  MlirType i32_i64[] = {i32, i64};
-  MlirLogicalResult result =
-      mlirLLVMStructTypeSetBody(fooStruct, sizeof(i32_i64) / sizeof(MlirType),
-                                i32_i64, /*isPacked=*/false);
-  if (!mlirLogicalResultIsSuccess(result))
-    return 7;
-
-  // CHECK: !llvm.struct<"foo", (i32, i64)>
-  mlirTypeDump(fooStruct);
-  if (mlirLLVMStructTypeIsOpaque(fooStruct))
-    return 8;
-  if (mlirLLVMStructTypeIsPacked(fooStruct))
-    return 9;
-  if (!mlirTypeEqual(mlirLLVMStructTypeIdentifiedGet(
-                         ctx, mlirStringRefCreateFromCString("foo")),
-                     fooStruct)) {
-    return 10;
-  }
-
-  MlirType barStruct = mlirLLVMStructTypeIdentifiedGet(
-      ctx, mlirStringRefCreateFromCString("bar"));
-  result = mlirLLVMStructTypeSetBody(barStruct, 1, &i32, /*isPacked=*/true);
-  if (!mlirLogicalResultIsSuccess(result))
-    return 11;
-
-  // CHECK: !llvm.struct<"bar", packed (i32)>
-  mlirTypeDump(barStruct);
-  if (!mlirLLVMStructTypeIsPacked(barStruct))
-    return 12;
-
-  // Same body, should succeed.
-  result =
-      mlirLLVMStructTypeSetBody(fooStruct, sizeof(i32_i64) / sizeof(MlirType),
-                                i32_i64, /*isPacked=*/false);
-  if (!mlirLogicalResultIsSuccess(result))
-    return 13;
-
-  // Different body, should fail.
-  result = mlirLLVMStructTypeSetBody(fooStruct, 1, &i32, /*isPacked=*/false);
-  if (mlirLogicalResultIsSuccess(result))
-    return 14;
-
-  // Packed flag differs, should fail.
-  result = mlirLLVMStructTypeSetBody(barStruct, 1, &i32, /*isPacked=*/false);
-  if (mlirLogicalResultIsSuccess(result))
-    return 15;
-
-  // Should have a different name.
-  // CHECK: !llvm.struct<"foo{{[^"]+}}
-  mlirTypeDump(mlirLLVMStructTypeIdentifiedNewGet(
-      ctx, mlirStringRefCreateFromCString("foo"), /*nFieldTypes=*/0,
-      /*fieldTypes=*/NULL, /*isPacked=*/false));
-
-  // Two freshly created "new" types must differ.
-  if (mlirTypeEqual(
-          mlirLLVMStructTypeIdentifiedNewGet(
-              ctx, mlirStringRefCreateFromCString("foo"), /*nFieldTypes=*/0,
-              /*fieldTypes=*/NULL, /*isPacked=*/false),
-          mlirLLVMStructTypeIdentifiedNewGet(
-              ctx, mlirStringRefCreateFromCString("foo"), /*nFieldTypes=*/0,
-              /*fieldTypes=*/NULL, /*isPacked=*/false))) {
-    return 16;
-  }
-
   MlirType opaque = mlirLLVMStructTypeOpaqueGet(
       ctx, mlirStringRefCreateFromCString("opaque"));
   // CHECK: !llvm.struct<"opaque", opaque>
diff --git a/mlir/test/Conversion/LLVMCommon/types.mlir b/mlir/test/Conversion/LLVMCommon/types.mlir
deleted file mode 100644
index 6d5cf562abe024..00000000000000
--- a/mlir/test/Conversion/LLVMCommon/types.mlir
+++ /dev/null
@@ -1,35 +0,0 @@
-// RUN: mlir-opt %s --convert-func-to-llvm --split-input-file | FileCheck %s
-
-// CHECK: @create_clashes_on_conversion(!llvm.struct<"foo", (index)>)
-func.func private @clashing_struct_name(!llvm.struct<"_Converted.foo", (f32)>)
-func.func private @create_clashes_on_conversion(!llvm.struct<"foo", (index)>)
-
-// -----
-
-// CHECK: @merge_on_conversion(!llvm.struct<"_Converted.foo", (i64)>) attributes {sym_visibility = "private"}
-func.func private @clashing_struct_name(!llvm.struct<"_Converted.foo", (i64)>)
-func.func private @merge_on_conversion(!llvm.struct<"foo", (index)>)
-
-// -----
-
-// CHECK: @create_clashes_on_conversion_recursive(!llvm.struct<"foo", (!llvm.struct<"foo">, index)>)
-func.func private @clashing_struct_name(!llvm.struct<"_Converted.foo", (struct<"_Converted.foo">, f32)>)
-func.func private @create_clashes_on_conversion_recursive(!llvm.struct<"foo", (struct<"foo">, index)>)
-
-// -----
-
-// CHECK: @merge_on_conversion_recursive(!llvm.struct<"_Converted.foo", (struct<"_Converted.foo">, i64)>)
-func.func private @clashing_struct_name(!llvm.struct<"_Converted.foo", (struct<"_Converted.foo">, i64)>)
-func.func private @merge_on_conversion_recursive(!llvm.struct<"foo", (struct<"foo">, index)>)
-
-// -----
-
-// CHECK: @create_clashing_pack(!llvm.struct<"foo", packed (!llvm.struct<"foo">, index)>)
-func.func private @clashing_struct_name(!llvm.struct<"_Converted.foo", (struct<"_Converted.foo">, i64)>)
-func.func private @create_clashing_pack(!llvm.struct<"foo", packed (struct<"foo">, index)>)
-
-// -----
-
-// CHECK: @merge_on_conversion_pack(!llvm.struct<"_Converted.foo", packed (struct<"_Converted.foo">, i64)>)
-func.func private @clashing_struct_name(!llvm.struct<"_Converted.foo", packed (struct<"_Converted.foo">, i64)>)
-func.func private @merge_on_conversion_pack(!llvm.struct<"foo", packed (struct<"foo">, index)>)
diff --git a/mlir/test/Dialect/LLVMIR/types-invalid.mlir b/mlir/test/Dialect/LLVMIR/types-invalid.mlir
index 76fb6780d8668f..3433cf8a30723a 100644
--- a/mlir/test/Dialect/LLVMIR/types-invalid.mlir
+++ b/mlir/test/Dialect/LLVMIR/types-invalid.mlir
@@ -21,46 +21,6 @@ func.func @function_taking_function() {
 
 // -----
 
-func.func @repeated_struct_name() {
-  "some.op"() : () -> !llvm.struct<"a", (ptr)>
-  // expected-error @+1 {{identified type already used with a different body}}
-  "some.op"() : () -> !llvm.struct<"a", (i32)>
-}
-
-// -----
-
-func.func @repeated_struct_name_packed() {
-  "some.op"() : () -> !llvm.struct<"a", packed (i32)>
-  // expected-error @+1 {{identified type already used with a different body}}
-  "some.op"() : () -> !llvm.struct<"a", (i32)>
-}
-
-// -----
-
-func.func @repeated_struct_opaque() {
-  "some.op"() : () -> !llvm.struct<"a", opaque>
-  // expected-error @+1 {{identified type already used with a different body}}
-  "some.op"() : () -> !llvm.struct<"a", ()>
-}
-
-// -----
-
-func.func @repeated_struct_opaque_non_empty() {
-  "some.op"() : () -> !llvm.struct<"a", opaque>
-  // expected-error @+1 {{identified type already used with a different body}}
-  "some.op"() : () -> !llvm.struct<"a", (i32, i32)>
-}
-
-// -----
-
-func.func @repeated_struct_opaque_redefinition() {
-  "some.op"() : () -> !llvm.struct<"a", ()>
-  // expected-error @+1 {{redeclaring defined struct as opaque}}
-  "some.op"() : () -> !llvm.struct<"a", opaque>
-}
-
-// -----
-
 func.func @struct_literal_opaque() {
   // expected-error @+1 {{only identified structs can be opaque}}
   "some.op"() : () -> !llvm.struct<opaque>
@@ -69,19 +29,12 @@ func.func @struct_literal_opaque() {
 // -----
 
 func.func @top_level_struct_no_body() {
-  // expected-error @below {{struct without a body only allowed in a recursive struct}}
+  // expected-error @below {{expected ','}}
   "some.op"() : () -> !llvm.struct<"a">
 }
 
 // -----
 
-func.func @nested_redefine_attempt() {
-  // expected-error @below {{identifier already used for an enclosing struct}}
-  "some.op"() : () -> !llvm.struct<"a", (struct<"a", ()>)>
-}
-
-// -----
-
 func.func @unexpected_type() {
   // expected-error @+1 {{unexpected type, expected keyword}}
   "some.op"() : () -> !llvm.tensor<*xf32>
@@ -96,14 +49,6 @@ func.func @unexpected_type() {
 
 // -----
 
-func.func @explicitly_opaque_struct() {
-  "some.op"() : () -> !llvm.struct<"a", opaque>
-  // expected-error @+1 {{identified type already used with a different body}}
-  "some.op"() : () -> !llvm.struct<"a", ()>
-}
-
-// -----
-
 func.func @literal_struct_with_void() {
   // expected-error @+1 {{invalid LLVM structure element type}}
   "some.op"() : () -> !llvm.struct<(void)>
diff --git a/mlir/test/Target/LLVMIR/Import/global-struct.ll b/mlir/test/Target/LLVMIR/Import/global-struct.ll
deleted file mode 100644
index 5e72b380537d66..00000000000000
--- a/mlir/test/Target/LLVMIR/Import/global-struct.ll
+++ /dev/null
@@ -1,8 +0,0 @@
-; RUN: mlir-translate --import-llvm %s | FileCheck %s
-
-; Ensure both structs have different names.
-; CHECK: llvm.func @fn(!llvm.struct<"[[NAME:[^"]*]]",
-; CHECK-NOT: struct<"[[NAME]]",
-%0 = type { %1 }
-%1 = type { i8 }
-declare void @fn(%0)
diff --git a/mlir/test/Target/LLVMIR/llvmir-types.mlir b/mlir/test/Target/LLVMIR/llvmir-types.mlir
index 6e54bb022c077e..221dfd010c78b5 100644
--- a/mlir/test/Target/LLVMIR/llvmir-types.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-types.mlir
@@ -175,3 +175,8 @@ llvm.func @return_s_symbols() -> !llvm.struct<"name with spaces and !^$@$#", pac
 llvm.func @return_s_struct_of_arrays() -> !llvm.struct<"struct-of-arrays", (array<10 x i32>)>
 // CHECK: declare [10 x %array-of-structs]
 llvm.func @return_s_array_of_structs() -> !llvm.array<10 x struct<"array-of-structs", (i32)>>
+
+// CHECK: declare %[[RET:foo.*]] @structs_with_same_name(
+// CHECK-NOT: %[[RET]]
+// CHECK-SAME: )
+llvm.func @structs_with_same_name(!llvm.struct<"foo", (i32)>) -> !llvm.struct<"foo", (i64)>
diff --git a/mlir/unittests/Dialect/CMakeLists.txt b/mlir/unittests/Dialect/CMakeLists.txt
index a5d4c48546e650..6b52de44ac5ce1 100644
--- a/mlir/unittests/Dialect/CMakeLists.txt
+++ b/mlir/unittests/Dialect/CMakeLists.txt
@@ -9,7 +9,6 @@ target_link_libraries(MLIRDialectTests
 add_subdirectory(AMDGPU)
 add_subdirectory(ArmSME)
 add_subdirectory(Index)
-add_subdirectory(LLVMIR)
 add_subdirectory(MemRef)
 add_subdirectory(OpenACC)
 add_subdirectory(Polynomial)
diff --git a/mlir/unittests/Dialect/LLVMIR/CMakeLists.txt b/mlir/unittests/Dialect/LLVMIR/CMakeLists.txt
deleted file mode 100644
index 92af1856c68e01..00000000000000
--- a/mlir/unittests/Dialect/LLVMIR/CMakeLists.txt
+++ /dev/null
@@ -1,7 +0,0 @@
-add_mlir_unittest(MLIRLLVMIRTests
-  LLVMTypeTest.cpp
-)
-target_link_libraries(MLIRLLVMIRTests
-  PRIVATE
-  MLIRLLVMDialect
-  )
diff --git a/mlir/unittests/Dialect/LLVMIR/LLVMTestBase.h b/mlir/unittests/Dialect/LLVMIR/LLVMTestBase.h
deleted file mode 100644
index 1badc44ce2132c..00000000000000
--- a/mlir/unittests/Dialect/LLVMIR/LLVMTestBase.h
+++ /dev/null
@@ -1,27 +0,0 @@
-//===- LLVMTestBase.h - Test fixure for LLVM dialect tests ------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-//
-// Test fixure for LLVM dialect tests.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef MLIR_UNITTEST_DIALECT_LLVMIR_LLVMTESTBASE_H
-#define MLIR_UNITTEST_DIALECT_LLVMIR_LLVMTESTBASE_H
-
-#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
-#include "mlir/IR/MLIRContext.h"
-#include "gtest/gtest.h"
-
-class LLVMIRTest : public ::testing::Test {
-protected:
-  LLVMIRTest() { context.getOrLoadDialect<mlir::LLVM::LLVMDialect>(); }
-
-  mlir::MLIRContext context;
-};
-
-#endif
diff --git a/mlir/unittests/Dialect/LLVMIR/LLVMTypeTest.cpp b/mlir/unittests/Dialect/LLVMIR/LLVMTypeTest.cpp
deleted file mode 100644
index 083dec819a0e05..00000000000000
--- a/mlir/unittests/Dialect/LLVMIR/LLVMTypeTest.cpp
+++ /dev/null
@@ -1,19 +0,0 @@
-//===- LLVMTypeTest.cpp - Tests for LLVM types ----------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "LLVMTestBase.h"
-#include "mlir/Dialect/LLVMIR/LLVMTypes.h"
-
-using namespace mlir;
-using namespace mlir::LLVM;
-
-TEST_F(LLVMIRTest, IsStructTypeMutable) {
-  auto structTy = LLVMStructType::getIdentified(&context, "foo");
-  ASSERT_TRUE(bool(structTy));
-  ASSERT_TRUE(structTy.hasTrait<TypeTrait::IsMutable>());
-}

>From 89444327a4c0397c0312ff4840d95ea898930328 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20B=C3=B6ck?= <markus.boeck02 at gmail.com>
Date: Wed, 13 Nov 2024 13:39:45 +0100
Subject: [PATCH 2/5] update Python bindings as well

---
 mlir/lib/Bindings/Python/DialectLLVM.cpp | 22 +----------
 mlir/test/python/dialects/llvm.py        | 50 +++++-------------------
 2 files changed, 11 insertions(+), 61 deletions(-)

diff --git a/mlir/lib/Bindings/Python/DialectLLVM.cpp b/mlir/lib/Bindings/Python/DialectLLVM.cpp
index 42a4c8c0793ba8..1777425ad23f0e 100644
--- a/mlir/lib/Bindings/Python/DialectLLVM.cpp
+++ b/mlir/lib/Bindings/Python/DialectLLVM.cpp
@@ -43,14 +43,6 @@ void populateDialectLLVMSubmodule(const pybind11::module &m) {
       "cls"_a, "elements"_a, py::kw_only(), "packed"_a = false,
       "loc"_a = py::none());
 
-  llvmStructType.def_classmethod(
-      "get_identified",
-      [](py::object cls, const std::string &name, MlirContext context) {
-        return cls(mlirLLVMStructTypeIdentifiedGet(
-            context, mlirStringRefCreate(name.data(), name.size())));
-      },
-      "cls"_a, "name"_a, py::kw_only(), "context"_a = py::none());
-
   llvmStructType.def_classmethod(
       "get_opaque",
       [](py::object cls, const std::string &name, MlirContext context) {
@@ -59,20 +51,8 @@ void populateDialectLLVMSubmodule(const pybind11::module &m) {
       },
       "cls"_a, "name"_a, "context"_a = py::none());
 
-  llvmStructType.def(
-      "set_body",
-      [](MlirType self, const std::vector<MlirType> &elements, bool packed) {
-        MlirLogicalResult result = mlirLLVMStructTypeSetBody(
-            self, elements.size(), elements.data(), packed);
-        if (!mlirLogicalResultIsSuccess(result)) {
-          throw py::value_error(
-              "Struct body already set to different content.");
-        }
-      },
-      "elements"_a, py::kw_only(), "packed"_a = false);
-
   llvmStructType.def_classmethod(
-      "new_identified",
+      "get",
       [](py::object cls, const std::string &name,
          const std::vector<MlirType> &elements, bool packed, MlirContext ctx) {
         return cls(mlirLLVMStructTypeIdentifiedNewGet(
diff --git a/mlir/test/python/dialects/llvm.py b/mlir/test/python/dialects/llvm.py
index d9ffdeb65bfd40..ce16f03f25c1f5 100644
--- a/mlir/test/python/dialects/llvm.py
+++ b/mlir/test/python/dialects/llvm.py
@@ -37,68 +37,38 @@ def testStructType():
     assert llvm.StructType.get_literal([i32]) == llvm.StructType.get_literal([i32])
     assert llvm.StructType.get_literal([i32]) != llvm.StructType.get_literal([i64])
 
-    print(llvm.StructType.get_identified("foo"))
-    print(llvm.StructType.get_identified("bar"))
+    print(llvm.StructType.get_opaque("foo"))
+    print(llvm.StructType.get_opaque("bar"))
     # CHECK: !llvm.struct<"foo", opaque>
     # CHECK: !llvm.struct<"bar", opaque>
 
-    assert llvm.StructType.get_identified("foo") == llvm.StructType.get_identified(
-        "foo"
+    assert llvm.StructType.get("foo", []) == llvm.StructType.get(
+        "foo", []
     )
-    assert llvm.StructType.get_identified("foo") != llvm.StructType.get_identified(
-        "bar"
+    assert llvm.StructType.get("foo", []) != llvm.StructType.get(
+        "bar", []
     )
 
-    foo_struct = llvm.StructType.get_identified("foo")
+    foo_struct = llvm.StructType.get_opaque("foo")
     print(foo_struct.name)
     print(foo_struct.body)
     assert foo_struct.opaque
-    foo_struct.set_body([i32, i64])
+    foo_struct = llvm.StructType.get("foo", [i32, i64])
     print(*tuple(foo_struct.body))
     print(foo_struct)
     assert not foo_struct.packed
     assert not foo_struct.opaque
-    assert llvm.StructType.get_identified("foo") == foo_struct
+    assert llvm.StructType.get_opaque("foo") != foo_struct
     # CHECK: foo
     # CHECK: None
     # CHECK: i32 i64
     # CHECK: !llvm.struct<"foo", (i32, i64)>
 
-    bar_struct = llvm.StructType.get_identified("bar")
-    bar_struct.set_body([i32], packed=True)
+    bar_struct = llvm.StructType.get("bar", [i32], packed=True)
     print(bar_struct)
     assert bar_struct.packed
     # CHECK: !llvm.struct<"bar", packed (i32)>
 
-    # Same body, should not raise.
-    foo_struct.set_body([i32, i64])
-
-    try:
-        foo_struct.set_body([])
-    except ValueError as e:
-        pass
-    else:
-        assert False, "expected exception not raised"
-
-    try:
-        bar_struct.set_body([i32])
-    except ValueError as e:
-        pass
-    else:
-        assert False, "expected exception not raised"
-
-    print(llvm.StructType.new_identified("foo", []))
-    assert llvm.StructType.new_identified("foo", []) != llvm.StructType.new_identified(
-        "foo", []
-    )
-    # CHECK: !llvm.struct<"foo{{[^"]+}}
-
-    opaque = llvm.StructType.get_opaque("opaque")
-    print(opaque)
-    assert opaque.opaque
-    # CHECK: !llvm.struct<"opaque", opaque>
-
-
 # CHECK-LABEL: testSmoke
 @constructAndPrintInModule
 def testSmoke():

>From 12edbe620f60a2d7acb7c13d6fa6320e07f4431d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20B=C3=B6ck?= <markus.boeck02 at gmail.com>
Date: Wed, 13 Nov 2024 13:45:45 +0100
Subject: [PATCH 3/5] apply darker formatting diff

---
 mlir/test/python/dialects/llvm.py | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/mlir/test/python/dialects/llvm.py b/mlir/test/python/dialects/llvm.py
index ce16f03f25c1f5..25637015e5adbe 100644
--- a/mlir/test/python/dialects/llvm.py
+++ b/mlir/test/python/dialects/llvm.py
@@ -42,12 +42,8 @@ def testStructType():
     # CHECK: !llvm.struct<"foo", opaque>
     # CHECK: !llvm.struct<"bar", opaque>
 
-    assert llvm.StructType.get("foo", []) == llvm.StructType.get(
-        "foo", []
-    )
-    assert llvm.StructType.get("foo", []) != llvm.StructType.get(
-        "bar", []
-    )
+    assert llvm.StructType.get("foo", []) == llvm.StructType.get("foo", [])
+    assert llvm.StructType.get("foo", []) != llvm.StructType.get("bar", [])
 
     foo_struct = llvm.StructType.get_opaque("foo")
     print(foo_struct.name)

>From cae69a0e1110fb697cfaf0009a8d7c1dc04fb58c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20B=C3=B6ck?= <markus.boeck02 at gmail.com>
Date: Thu, 14 Nov 2024 22:11:46 +0100
Subject: [PATCH 4/5] try fix flang

---
 flang/lib/Optimizer/CodeGen/TypeConverter.cpp | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/flang/lib/Optimizer/CodeGen/TypeConverter.cpp b/flang/lib/Optimizer/CodeGen/TypeConverter.cpp
index c23203efcd3df2..4f95147fae411f 100644
--- a/flang/lib/Optimizer/CodeGen/TypeConverter.cpp
+++ b/flang/lib/Optimizer/CodeGen/TypeConverter.cpp
@@ -136,16 +136,6 @@ mlir::Type LLVMTypeConverter::indexType() const {
 std::optional<llvm::LogicalResult> LLVMTypeConverter::convertRecordType(
     fir::RecordType derived, llvm::SmallVectorImpl<mlir::Type> &results) {
   auto name = fir::NameUniquer::dropTypeConversionMarkers(derived.getName());
-  auto st = mlir::LLVM::LLVMStructType::getIdentified(&getContext(), name);
-
-  auto &callStack = getCurrentThreadRecursiveStack();
-  if (llvm::count(callStack, derived)) {
-    results.push_back(st);
-    return mlir::success();
-  }
-  callStack.push_back(derived);
-  auto popConversionCallStack =
-      llvm::make_scope_exit([&callStack]() { callStack.pop_back(); });
 
   llvm::SmallVector<mlir::Type> members;
   for (auto mem : derived.getTypeList()) {
@@ -156,8 +146,8 @@ std::optional<llvm::LogicalResult> LLVMTypeConverter::convertRecordType(
     else
       members.push_back(mlir::cast<mlir::Type>(convertType(mem.second)));
   }
-  if (mlir::failed(st.setBody(members, /*isPacked=*/false)))
-    return mlir::failure();
+  auto st = mlir::LLVM::LLVMStructType::get(&getContext(), name, members,
+                                            /*isPacked=*/false);
   results.push_back(st);
   return mlir::success();
 }

>From 684067ce0411016a35d8c0b74b3158197f03002c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20B=C3=B6ck?= <markus.boeck02 at gmail.com>
Date: Thu, 14 Nov 2024 22:26:47 +0100
Subject: [PATCH 5/5] attempt to fix endless recursion

---
 flang/lib/Optimizer/CodeGen/TypeConverter.cpp | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/flang/lib/Optimizer/CodeGen/TypeConverter.cpp b/flang/lib/Optimizer/CodeGen/TypeConverter.cpp
index 4f95147fae411f..b690cf51b968d0 100644
--- a/flang/lib/Optimizer/CodeGen/TypeConverter.cpp
+++ b/flang/lib/Optimizer/CodeGen/TypeConverter.cpp
@@ -169,14 +169,10 @@ mlir::Type LLVMTypeConverter::convertBoxTypeAsStruct(BaseBoxType box,
   // remove fir.heap/fir.ref/fir.ptr
   if (auto removeIndirection = fir::dyn_cast_ptrEleTy(ele))
     ele = removeIndirection;
-  auto eleTy = convertType(ele);
+
   // base_addr*
-  if (mlir::isa<SequenceType>(ele) &&
-      mlir::isa<mlir::LLVM::LLVMPointerType>(eleTy))
-    dataDescFields.push_back(eleTy);
-  else
-    dataDescFields.push_back(
-        mlir::LLVM::LLVMPointerType::get(eleTy.getContext()));
+  dataDescFields.push_back(mlir::LLVM::LLVMPointerType::get(ele.getContext()));
+
   // elem_len
   dataDescFields.push_back(
       getDescFieldTypeModel<kElemLenPosInBox>()(&getContext()));



More information about the flang-commits mailing list