[Mlir-commits] [mlir] [mlir][LLVM][NFC] Move `LLVMStructType` to ODS (PR #117485)

llvmlistbot at llvm.org llvmlistbot at llvm.org
Sun Nov 24 06:10:25 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-llvm

Author: Markus Böck (zero9178)

<details>
<summary>Changes</summary>

This PR extracts NFC changes out of https://github.com/llvm/llvm-project/pull/116035 to reap as many of the same benefits without any of the semantic changes.

More concretely, moving `LLVMStructType` to ODS has the benefits of being able to generate much of the required boilerplate, such as interface definitions, documentation and more, automatically. Furthermore, `LLVMStructType` is then treated less special and its definition can be found at the same place where all other complex type definitions are found in the LLVM dialect.

Future changes could leverage more automatically generated code from TableGen such as `assemblyFormat`. As these are not as trivial, they have been left for future PRs.

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


7 Files Affected:

- (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.h (-139) 
- (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td (+134) 
- (modified) mlir/include/mlir/IR/AttrTypeBase.td (+3) 
- (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp (+1-2) 
- (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMMemorySlot.cpp (+2-2) 
- (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp (+1-1) 
- (modified) mlir/test/lib/Dialect/Test/TestTypeDefs.td (+1-1) 


``````````diff
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.h b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.h
index 2ea589a7c4c3bd..8b380751c2f9d6 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.
 //===----------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
index 09dd0919c318fb..5268f1ec311512 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
@@ -117,6 +117,140 @@ def LLVMFunctionType : LLVMType<"LLVMFunction", "func"> {
   }];
 }
 
+//===----------------------------------------------------------------------===//
+// LLVMStructType
+//===----------------------------------------------------------------------===//
+
+def LLVMStructType : LLVMType<"LLVMStruct", "struct", [
+  MutableType,
+  DeclareTypeInterfaceMethods<DataLayoutTypeInterface,
+    ["areCompatible", "verifyEntries"]>,
+  DeclareTypeInterfaceMethods<DestructurableTypeInterface,
+    ["getSubelementIndexMap", "getTypeAtIndex"]>
+]> {
+  let summary = "LLVM struct type";
+
+  let description = [{
+    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.
+  }];
+
+  // Specify parameters for which TableGen can generate convenient getters for
+  // us.
+  // TODO: Other parameters such as 'packed' or 'opaque' could be added in the
+  //       future iff they generate getters prefixed with 'is', instead of
+  //       'get'. Until then there are no advantages in doing so.
+  let parameters = (ins
+    StringRefParameter<"struct name", [{""}]>:$name,
+    OptionalArrayRefParameter<"mlir::Type">:$body
+  );
+
+  // A custom storage class defined in C++ is required to implement mutability.
+  let storageClass = "LLVMStructTypeStorage";
+  let genStorageClass = 0;
+
+  // We want users to use the more aptly named custom builders below.
+  let skipDefaultBuilders = 1;
+
+  let extraClassDeclaration = [{
+    /// 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.
+    /// 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();
+
+    /// 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;
+  }];
+
+  let hasCustomAssemblyFormat = 1;
+}
+
 //===----------------------------------------------------------------------===//
 // LLVMPointerType
 //===----------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/IR/AttrTypeBase.td b/mlir/include/mlir/IR/AttrTypeBase.td
index cbe4f0d67574b3..38d38cf098df3e 100644
--- a/mlir/include/mlir/IR/AttrTypeBase.td
+++ b/mlir/include/mlir/IR/AttrTypeBase.td
@@ -56,6 +56,9 @@ class ParamNativeTypeTrait<string prop, string params>
 class GenInternalTypeTrait<string prop> : GenInternalTrait<prop, "Type">;
 class PredTypeTrait<string descr, Pred pred> : PredTrait<descr, pred>;
 
+// Trait required to be added to any type which is mutable.
+def MutableType : NativeTypeTrait<"IsMutable">;
+
 //===----------------------------------------------------------------------===//
 // Builders
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 9bb0c80749a5f5..d30a6b8398f064 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -3510,8 +3510,7 @@ void LLVMDialect::initialize() {
            LLVMPPCFP128Type,
            LLVMTokenType,
            LLVMLabelType,
-           LLVMMetadataType,
-           LLVMStructType>();
+           LLVMMetadataType>();
   // clang-format on
   registerTypes();
 
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMMemorySlot.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMMemorySlot.cpp
index 903035a3ec2296..655316cc5d66d6 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMMemorySlot.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMMemorySlot.cpp
@@ -1566,7 +1566,7 @@ DeletionKind LLVM::MemmoveOp::rewire(const DestructurableMemorySlot &slot,
 //===----------------------------------------------------------------------===//
 
 std::optional<DenseMap<Attribute, Type>>
-LLVM::LLVMStructType::getSubelementIndexMap() {
+LLVM::LLVMStructType::getSubelementIndexMap() const {
   Type i32 = IntegerType::get(getContext(), 32);
   DenseMap<Attribute, Type> destructured;
   for (const auto &[index, elemType] : llvm::enumerate(getBody()))
@@ -1574,7 +1574,7 @@ LLVM::LLVMStructType::getSubelementIndexMap() {
   return destructured;
 }
 
-Type LLVM::LLVMStructType::getTypeAtIndex(Attribute index) {
+Type LLVM::LLVMStructType::getTypeAtIndex(Attribute index) const {
   auto indexAttr = llvm::dyn_cast<IntegerAttr>(index);
   if (!indexAttr || !indexAttr.getType().isInteger(32))
     return {};
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
index 1bed3fa48b30d7..33c231e2d2045f 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
@@ -485,7 +485,7 @@ bool LLVMStructType::isOpaque() {
          (getImpl()->isOpaque() || !getImpl()->isInitialized());
 }
 bool LLVMStructType::isInitialized() { return getImpl()->isInitialized(); }
-StringRef LLVMStructType::getName() { return getImpl()->getIdentifier(); }
+StringRef LLVMStructType::getName() const { return getImpl()->getIdentifier(); }
 ArrayRef<Type> LLVMStructType::getBody() const {
   return isIdentified() ? getImpl()->getIdentifiedStructBody()
                         : getImpl()->getTypeList();
diff --git a/mlir/test/lib/Dialect/Test/TestTypeDefs.td b/mlir/test/lib/Dialect/Test/TestTypeDefs.td
index 830475bed4e444..60108ac86d1edd 100644
--- a/mlir/test/lib/Dialect/Test/TestTypeDefs.td
+++ b/mlir/test/lib/Dialect/Test/TestTypeDefs.td
@@ -375,7 +375,7 @@ def TestI32 : Test_Type<"TestI32"> {
 }
 
 def TestRecursiveAlias
-    : Test_Type<"TestRecursiveAlias", [NativeTypeTrait<"IsMutable">]> {
+    : Test_Type<"TestRecursiveAlias", [MutableType]> {
   let mnemonic = "test_rec_alias";
   let storageClass = "TestRecursiveTypeStorage";
   let storageNamespace = "test";

``````````

</details>


https://github.com/llvm/llvm-project/pull/117485


More information about the Mlir-commits mailing list