[Mlir-commits] [mlir] [mlir][DLTI] Handle builtin types when combining data layouts (PR #222874)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Fri Sep 11 00:58:51 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-dlti
Author: Oscar Priego (Opriego)
<details>
<summary>Changes</summary>
While investigating #<!-- -->222356, two independent crashes were found when combining nested data layouts.
PR #<!-- -->222855 addresses the separate LLVM pointer data-layout crash. This PR fixes the builtin type combination path.
`DataLayoutSpecInterface::bucketEntriesByType()` groups type entries by `TypeID`. This means parametric builtin types such as `i32` and `i64` are placed in the same `IntegerType` bucket.
`combineOneSpec()` previously assumed that an already-existing type bucket could not belong to the builtin dialect. Nested layouts containing supported builtin entries therefore hit an assertion instead of being combined.
Handle the builtin types that DLTI already supports explicitly:
- `index`
- integer types
- floating-point types
For integer and floating-point entries, compatibility is checked only when the concrete type key matches. Different keys in the same `TypeID` bucket, such as `i32` and `i64`, coexist.
For matching integer/float entries, preserve the existing ABI nesting rule: the nested ABI alignment may remain equal or become less strict when it divides the enclosing ABI alignment.
For `index`, nested entries must preserve the index bitwidth.
The compatibility check also handles zero ABI alignment without performing modulo by zero. Zero-valued alignment is currently accepted by the existing verifier, so this keeps combination safe without changing verifier semantics.
Regression coverage includes:
- identical nested integer entries
- distinct integer keys sharing one `IntegerType` bucket
- compatible ABI relaxation from 64 to 32 bits
- incompatible ABI strengthening from 32 to 64 bits
- compatible and incompatible `index` bitwidths
- nested floating-point entries
- zero ABI alignment without division by zero
Validation:
- built `mlir-opt`
- DLTI test suite passes, 4/4
- all focused reproducers complete successfully
- incompatible builtin layouts produce the normal data-layout incompatibility diagnostic instead of asserting
- `git diff --check` passes
Related to #<!-- -->222356.
---
Full diff: https://github.com/llvm/llvm-project/pull/222874.diff
3 Files Affected:
- (modified) mlir/lib/Dialect/DLTI/DLTI.cpp (+52-10)
- (modified) mlir/test/Dialect/DLTI/invalid.mlir (+26)
- (modified) mlir/test/Dialect/DLTI/valid.mlir (+88)
``````````diff
diff --git a/mlir/lib/Dialect/DLTI/DLTI.cpp b/mlir/lib/Dialect/DLTI/DLTI.cpp
index da572f123fdc1..9b95250a7b047 100644
--- a/mlir/lib/Dialect/DLTI/DLTI.cpp
+++ b/mlir/lib/Dialect/DLTI/DLTI.cpp
@@ -288,6 +288,44 @@ overwriteDuplicateEntries(SmallVectorImpl<DataLayoutEntryInterface> &oldEntries,
}
}
+static bool
+areBuiltinDataLayoutEntriesCompatible(DataLayoutEntryListRef oldEntries,
+ DataLayoutEntryListRef newEntries) {
+ for (DataLayoutEntryInterface newEntry : newEntries) {
+ const auto *oldEntry =
+ llvm::find_if(oldEntries, [&](DataLayoutEntryInterface entry) {
+ return entry.getKey() == newEntry.getKey();
+ });
+ if (oldEntry == oldEntries.end())
+ continue;
+
+ Type type = cast<Type>(newEntry.getKey());
+ if (isa<IndexType>(type)) {
+ uint64_t oldBitwidth =
+ cast<IntegerAttr>(oldEntry->getValue()).getValue().getZExtValue();
+ uint64_t newBitwidth =
+ cast<IntegerAttr>(newEntry.getValue()).getValue().getZExtValue();
+ if (oldBitwidth != newBitwidth)
+ return false;
+ continue;
+ }
+
+ assert(type.isIntOrFloat() &&
+ "unexpected data layout entry for unsupported built-in type");
+ auto oldValues =
+ cast<DenseIntElementsAttr>(oldEntry->getValue()).getValues<uint64_t>();
+ auto newValues =
+ cast<DenseIntElementsAttr>(newEntry.getValue()).getValues<uint64_t>();
+ uint64_t oldAbi = *oldValues.begin();
+ uint64_t newAbi = *newValues.begin();
+ if (oldAbi == newAbi)
+ continue;
+ if (oldAbi < newAbi || newAbi == 0 || oldAbi % newAbi != 0)
+ return false;
+ }
+ return true;
+}
+
/// Combines a data layout spec into the given lists of entries organized by
/// type class and identifier, overwriting them if necessary. Fails to combine
/// if the two entries with identical keys are not compatible.
@@ -333,16 +371,20 @@ static LogicalResult combineOneSpec(
}
Type typeSample = cast<Type>(kvp.second.front().getKey());
- assert(&typeSample.getDialect() !=
- typeSample.getContext()->getLoadedDialect<BuiltinDialect>() &&
- "unexpected data layout entry for built-in type");
-
- auto interface = cast<DataLayoutTypeInterface>(typeSample);
- // TODO: Revisit this method and call once
- // https://github.com/llvm/llvm-project/issues/130321 gets resolved.
- if (!interface.areCompatible(entriesForType.lookup(kvp.first), kvp.second,
- spec, entriesForID))
- return failure();
+ if (isa<BuiltinDialect>(&typeSample.getDialect())) {
+ assert((isa<IndexType>(typeSample) || typeSample.isIntOrFloat()) &&
+ "unexpected data layout entry for unsupported built-in type");
+ if (!areBuiltinDataLayoutEntriesCompatible(
+ entriesForType.lookup(kvp.first), kvp.second))
+ return failure();
+ } else {
+ auto interface = cast<DataLayoutTypeInterface>(typeSample);
+ // TODO: Revisit this method and call once
+ // https://github.com/llvm/llvm-project/issues/130321 gets resolved.
+ if (!interface.areCompatible(entriesForType.lookup(kvp.first), kvp.second,
+ spec, entriesForID))
+ return failure();
+ }
overwriteDuplicateEntries(entriesForType[kvp.first], kvp.second);
}
diff --git a/mlir/test/Dialect/DLTI/invalid.mlir b/mlir/test/Dialect/DLTI/invalid.mlir
index 2436e4f7484f5..0eb531fed5d7f 100644
--- a/mlir/test/Dialect/DLTI/invalid.mlir
+++ b/mlir/test/Dialect/DLTI/invalid.mlir
@@ -111,6 +111,32 @@ module attributes { dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"unknown.unknown
// -----
+// expected-note at below {{enclosing op with data layout}}
+module attributes { dlti.dl_spec = #dlti.dl_spec<
+ i32 = dense<32> : vector<2xi64>
+>} {
+ // expected-error at below {{data layout does not combine with layouts of enclosing ops}}
+ module attributes { dlti.dl_spec = #dlti.dl_spec<
+ i32 = dense<64> : vector<2xi64>
+ >} {
+ }
+}
+
+// -----
+
+// expected-note at below {{enclosing op with data layout}}
+module attributes { dlti.dl_spec = #dlti.dl_spec<
+ index = 32
+>} {
+ // expected-error at below {{data layout does not combine with layouts of enclosing ops}}
+ module attributes { dlti.dl_spec = #dlti.dl_spec<
+ index = 64
+ >} {
+ }
+}
+
+// -----
+
// expected-error at below {{'dlti.target_system_spec' is expected to be a #dlti.target_system_spec attribute}}
"test.unknown_op"() { dlti.target_system_spec = 42 } : () -> ()
diff --git a/mlir/test/Dialect/DLTI/valid.mlir b/mlir/test/Dialect/DLTI/valid.mlir
index 4c24e80041003..648e93a9e6643 100644
--- a/mlir/test/Dialect/DLTI/valid.mlir
+++ b/mlir/test/Dialect/DLTI/valid.mlir
@@ -1,4 +1,5 @@
// RUN: mlir-opt -split-input-file %s | FileCheck %s
+// RUN: mlir-opt -split-input-file --test-data-layout-query %s | FileCheck %s --check-prefix=BUILTIN
// CHECK: module attributes {
@@ -252,3 +253,90 @@ module attributes {
// CHECK: }) {dlti.map = #dlti.map<i32 = 42 : i64>}
"test.op_with_dlti_map"() ({
}) { dlti.map = #dlti.map<#dlti.dl_entry<i32, 42>> } : () -> ()
+
+// -----
+
+// BUILTIN-LABEL: module @builtin_integer_entries
+module @builtin_integer_entries attributes { dlti.dl_spec = #dlti.dl_spec<
+ i32 = dense<32> : vector<2xi64>
+>} {
+ func.func @query() {
+ "test.op_with_data_layout"() ({
+ // BUILTIN: alignment = 4
+ "test.data_layout_query"() : () -> i32
+ // BUILTIN: alignment = 8
+ "test.data_layout_query"() : () -> i64
+ "test.maybe_terminator"() : () -> ()
+ }) { dlti.dl_spec = #dlti.dl_spec<
+ i32 = dense<32> : vector<2xi64>,
+ i64 = dense<64> : vector<2xi64>
+ >} : () -> ()
+ return
+ }
+}
+
+// -----
+
+// BUILTIN-LABEL: module @builtin_integer_compatible_abi
+module @builtin_integer_compatible_abi attributes { dlti.dl_spec = #dlti.dl_spec<
+ i32 = dense<64> : vector<2xi64>
+>} {
+ func.func @query() {
+ "test.op_with_data_layout"() ({
+ // BUILTIN: alignment = 4
+ "test.data_layout_query"() : () -> i32
+ "test.maybe_terminator"() : () -> ()
+ }) { dlti.dl_spec = #dlti.dl_spec<
+ i32 = dense<32> : vector<2xi64>
+ >} : () -> ()
+ return
+ }
+}
+
+// -----
+
+// BUILTIN-LABEL: module @builtin_index_entry
+module @builtin_index_entry attributes { dlti.dl_spec = #dlti.dl_spec<
+ index = 32
+>} {
+ func.func @query() {
+ "test.op_with_data_layout"() ({
+ // BUILTIN: bitsize = 32
+ // BUILTIN: index = 32
+ "test.data_layout_query"() : () -> index
+ "test.maybe_terminator"() : () -> ()
+ }) { dlti.dl_spec = #dlti.dl_spec<
+ index = 32
+ >} : () -> ()
+ return
+ }
+}
+
+// -----
+
+module attributes { dlti.dl_spec = #dlti.dl_spec<
+ i32 = dense<0> : vector<2xi64>
+>} {
+ module attributes { dlti.dl_spec = #dlti.dl_spec<
+ i32 = dense<0> : vector<2xi64>
+ >} {
+ }
+}
+
+// -----
+
+// BUILTIN-LABEL: module @builtin_float_entry
+module @builtin_float_entry attributes { dlti.dl_spec = #dlti.dl_spec<
+ f32 = dense<32> : vector<2xi64>
+>} {
+ func.func @query() {
+ "test.op_with_data_layout"() ({
+ // BUILTIN: alignment = 4
+ "test.data_layout_query"() : () -> f32
+ "test.maybe_terminator"() : () -> ()
+ }) { dlti.dl_spec = #dlti.dl_spec<
+ f32 = dense<32> : vector<2xi64>
+ >} : () -> ()
+ return
+ }
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/222874
More information about the Mlir-commits
mailing list