[Mlir-commits] [mlir] [mlir][LLVM] Make struct types immutable (PR #116035)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Wed Nov 13 04:04:10 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-ods
Author: Markus Böck (zero9178)
<details>
<summary>Changes</summary>
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.
---
Patch is 66.78 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/116035.diff
22 Files Affected:
- (modified) mlir/docs/Dialects/LLVM.md (+4-19)
- (modified) mlir/include/mlir-c/Dialect/LLVM.h (-14)
- (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.h (+2-140)
- (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td (+108)
- (modified) mlir/include/mlir/IR/AttrTypeBase.td (+17)
- (modified) mlir/lib/CAPI/Dialect/LLVM.cpp (+3-17)
- (modified) mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp (+2-43)
- (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp (+1-3)
- (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMTypeSyntax.cpp (+12-167)
- (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp (+32-61)
- (removed) mlir/lib/Dialect/LLVMIR/IR/TypeDetail.h (-390)
- (modified) mlir/lib/Target/LLVMIR/TypeFromLLVM.cpp (+1-1)
- (modified) mlir/lib/Target/LLVMIR/TypeToLLVM.cpp (+7-10)
- (modified) mlir/test/CAPI/llvm.c (-92)
- (removed) mlir/test/Conversion/LLVMCommon/types.mlir (-35)
- (modified) mlir/test/Dialect/LLVMIR/types-invalid.mlir (+1-56)
- (removed) mlir/test/Target/LLVMIR/Import/global-struct.ll (-8)
- (modified) mlir/test/Target/LLVMIR/llvmir-types.mlir (+5)
- (modified) mlir/unittests/Dialect/CMakeLists.txt (-1)
- (removed) mlir/unittests/Dialect/LLVMIR/CMakeLists.txt (-7)
- (removed) mlir/unittests/Dialect/LLVMIR/LLVMTestBase.h (-27)
- (removed) mlir/unittests/Dialect/LLVMIR/LLVMTypeTest.cpp (-19)
``````````diff
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;
- convert...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/116035
More information about the Mlir-commits
mailing list