[Mlir-commits] [mlir] [mlir][LLVM] handle ArrayAttr for constant array of structs (PR #139724)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Thu May 15 05:20:30 PDT 2025
https://github.com/jeanPerier updated https://github.com/llvm/llvm-project/pull/139724
>From 9b5f1d654266afa921bdeebd6ec065965cf0bad5 Mon Sep 17 00:00:00 2001
From: Jean Perier <jperier at nvidia.com>
Date: Tue, 13 May 2025 05:24:45 -0700
Subject: [PATCH 1/6] [mlir][LLVM] handle ArrayAttr for constant array of
structs
---
mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp | 53 ++++++++++++++++----
mlir/lib/Target/LLVMIR/ModuleTranslation.cpp | 27 ++++++++++
mlir/test/Dialect/LLVMIR/invalid.mlir | 22 ++++++++
mlir/test/Target/LLVMIR/llvmir-invalid.mlir | 5 --
mlir/test/Target/LLVMIR/llvmir.mlir | 17 +++++++
5 files changed, 110 insertions(+), 14 deletions(-)
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index c757f3ceb90e3..1868995e3f5ed 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -3221,15 +3221,50 @@ LogicalResult LLVM::ConstantOp::verify() {
if (!isa<VectorType, LLVM::LLVMArrayType>(getType()))
return emitOpError() << "expected vector or array type";
// The number of elements of the attribute and the type must match.
- int64_t attrNumElements;
- if (auto elementsAttr = dyn_cast<ElementsAttr>(getValue()))
- attrNumElements = elementsAttr.getNumElements();
- else
- attrNumElements = cast<ArrayAttr>(getValue()).size();
- if (getNumElements(getType()) != attrNumElements)
- return emitOpError()
- << "type and attribute have a different number of elements: "
- << getNumElements(getType()) << " vs. " << attrNumElements;
+ if (auto elementsAttr = dyn_cast<ElementsAttr>(getValue())) {
+ int64_t attrNumElements = elementsAttr.getNumElements();
+ if (getNumElements(getType()) != attrNumElements)
+ return emitOpError()
+ << "type and attribute have a different number of elements: "
+ << getNumElements(getType()) << " vs. " << attrNumElements;
+ } else {
+ // When the attribute is an ArrayAttr, check that its nesting matches the
+ // corresponding ArrayType or VectorType nesting.
+ Type dimType = getType();
+ Attribute dimVal = getValue();
+ int dim = 0;
+ while (true) {
+ int64_t dimSize =
+ llvm::TypeSwitch<Type, int64_t>(dimType)
+ .Case<VectorType, LLVMArrayType>([&dimType](auto t) -> int64_t {
+ dimType = t.getElementType();
+ return t.getNumElements();
+ })
+ .Default([](auto) -> int64_t { return -1; });
+ if (dimSize < 0)
+ break;
+ auto arrayAttr = dyn_cast<ArrayAttr>(dimVal);
+ if (!arrayAttr)
+ return emitOpError()
+ << "array attribute nesting must match array type nesting";
+ if (dimSize != static_cast<int64_t>(arrayAttr.size()))
+ return emitOpError()
+ << "array attribute size does not match array type size in "
+ "dimension "
+ << dim << ": " << arrayAttr.size() << " vs. " << dimSize;
+ if (arrayAttr.size() == 0)
+ break;
+ dimVal = arrayAttr.getValue()[0];
+ ++dim;
+ }
+ if (auto structType = dyn_cast<LLVMStructType>(dimType)) {
+ auto arrayAttr = dyn_cast<ArrayAttr>(dimVal);
+ if (!arrayAttr || arrayAttr.size() != structType.getBody().size())
+ return emitOpError()
+ << "nested attribute must be an array attribute with the same "
+ "number of elements as the struct type";
+ }
+ }
} else {
return emitOpError()
<< "only supports integer, float, string or elements attributes";
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 1168b9f339904..1d4509ccb044e 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -713,6 +713,33 @@ llvm::Constant *mlir::LLVM::detail::getLLVMConstant(
ArrayRef<char>{stringAttr.getValue().data(),
stringAttr.getValue().size()});
}
+
+ // Handle arrays of structs that cannot be represented as DenseElementsAttr
+ // in MLIR.
+ if (auto arrayAttr = dyn_cast<ArrayAttr>(attr)) {
+ if (auto *arrayTy = dyn_cast<llvm::ArrayType>(llvmType)) {
+ llvm::Type *elementType = arrayTy->getElementType();
+ Attribute previousElementAttr;
+ llvm::Constant *elementCst = nullptr;
+ SmallVector<llvm::Constant *> constants;
+ constants.reserve(arrayTy->getNumElements());
+ for (auto elementAttr : arrayAttr) {
+ // Arrays with a single value or with repeating values are quite common.
+ // short-circuit the translation when the element value is the same as
+ // the previous one.
+ if (!previousElementAttr || previousElementAttr != elementAttr) {
+ previousElementAttr = elementAttr;
+ elementCst =
+ getLLVMConstant(elementType, elementAttr, loc, moduleTranslation);
+ if (!elementCst)
+ return nullptr;
+ }
+ constants.push_back(elementCst);
+ }
+ return llvm::ConstantArray::get(arrayTy, constants);
+ }
+ }
+
emitError(loc, "unsupported constant value");
return nullptr;
}
diff --git a/mlir/test/Dialect/LLVMIR/invalid.mlir b/mlir/test/Dialect/LLVMIR/invalid.mlir
index f9ea066a63624..4c82e586b8a3c 100644
--- a/mlir/test/Dialect/LLVMIR/invalid.mlir
+++ b/mlir/test/Dialect/LLVMIR/invalid.mlir
@@ -1850,3 +1850,25 @@ llvm.func @gep_inbounds_flag_usage(%ptr: !llvm.ptr, %idx: i64) {
llvm.getelementptr inbounds_flag %ptr[%idx, 0, %idx] : (!llvm.ptr, i64, i64) -> !llvm.ptr, !llvm.struct<(array<10 x f32>)>
llvm.return
}
+
+// -----
+
+llvm.mlir.global @x1() : !llvm.array<2x!llvm.struct<(i32, f32)>> {
+ // expected-error at +1{{'llvm.mlir.constant' op array attribute size does not match array type size in dimension 0: 1 vs. 2}}
+ %0 = llvm.mlir.constant([[42 : i32, 1.000000e+00 : f32]]) : !llvm.array<2x!llvm.struct<(i32, f32)>>
+ llvm.return %0 : !llvm.array<2x!llvm.struct<(i32, f32)>>
+}
+
+// -----
+llvm.mlir.global @x2() : !llvm.array<1x!llvm.array<1x!llvm.array<1x!llvm.struct<(i32)>>>> {
+ // expected-error at +1{{'llvm.mlir.constant' op array attribute nesting must match array type nesting}}
+ %0 = llvm.mlir.constant([[1 : i32]]) : !llvm.array<1x!llvm.array<1x!llvm.array<1x!llvm.struct<(i32)>>>>
+ llvm.return %0 : !llvm.array<1x!llvm.array<1x!llvm.array<1x!llvm.struct<(i32)>>>>
+}
+
+// -----
+llvm.mlir.global @x3() : !llvm.array<1x!llvm.struct<(i32, f32)>> {
+ // expected-error at +1{{'llvm.mlir.constant' op nested attribute must be an array attribute with the same number of elements as the struct type}}
+ %0 = llvm.mlir.constant([[1 : i32]]) : !llvm.array<1x!llvm.struct<(i32, f32)>>
+ llvm.return %0 : !llvm.array<1x!llvm.struct<(i32, f32)>>
+}
diff --git a/mlir/test/Target/LLVMIR/llvmir-invalid.mlir b/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
index 90c0f5ac55cb1..24a7b42557278 100644
--- a/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
@@ -79,11 +79,6 @@ llvm.func @incompatible_integer_type_for_float_attr() -> i32 {
// -----
-// expected-error @below{{unsupported constant value}}
-llvm.mlir.global internal constant @test([2.5, 7.4]) : !llvm.array<2 x f64>
-
-// -----
-
// expected-error @below{{LLVM attribute 'readonly' does not expect a value}}
llvm.func @passthrough_unexpected_value() attributes {passthrough = [["readonly", "42"]]}
diff --git a/mlir/test/Target/LLVMIR/llvmir.mlir b/mlir/test/Target/LLVMIR/llvmir.mlir
index 4ef68fa83a70d..242a151116fb3 100644
--- a/mlir/test/Target/LLVMIR/llvmir.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir.mlir
@@ -3000,3 +3000,20 @@ llvm.func internal @i(%arg0: i32) attributes {dso_local} {
llvm.call @testfn3(%arg0) : (i32 {llvm.alignstack = 8 : i64}) -> ()
llvm.return
}
+
+// -----
+
+// CHECK: @test_array_attr_1 = internal constant [2 x double] [double 2.500000e+00, double 7.400000e+00]
+llvm.mlir.global internal constant @test_array_attr_1([2.5, 7.4]) : !llvm.array<2 x f64>
+
+// CHECK: @test_array_attr_2 = global [2 x { i32, float }] [{ i32, float } { i32 42, float 1.000000e+00 }, { i32, float } { i32 42, float 1.000000e+00 }]
+llvm.mlir.global @test_array_attr_2() : !llvm.array<2x!llvm.struct<(i32, f32)>> {
+ %0 = llvm.mlir.constant([[42 : i32, 1.000000e+00 : f32],[42 : i32, 1.000000e+00 : f32]]) : !llvm.array<2x!llvm.struct<(i32, f32)>>
+ llvm.return %0 : !llvm.array<2x!llvm.struct<(i32, f32)>>
+}
+
+// CHECK: @test_array_attr_3 = global [2 x [3 x { i32, float }]{{.*}}[3 x { i32, float }] [{ i32, float } { i32 1, float 1.000000e+00 }, { i32, float } { i32 2, float 1.000000e+00 }, { i32, float } { i32 3, float 1.000000e+00 }], [3 x { i32, float }] [{ i32, float } { i32 4, float 1.000000e+00 }, { i32, float } { i32 5, float 1.000000e+00 }, { i32, float } { i32 6, float 1.000000e+00 }
+llvm.mlir.global @test_array_attr_3() : !llvm.array<2x!llvm.array<3x!llvm.struct<(i32, f32)>>> {
+ %0 = llvm.mlir.constant([[[1 : i32, 1.000000e+00 : f32],[2 : i32, 1.000000e+00 : f32],[3 : i32, 1.000000e+00 : f32]],[[4 : i32, 1.000000e+00 : f32],[5 : i32, 1.000000e+00 : f32],[6 : i32, 1.000000e+00 : f32]]]) : !llvm.array<2x!llvm.array<3x!llvm.struct<(i32, f32)>>>
+ llvm.return %0 : !llvm.array<2x!llvm.array<3x!llvm.struct<(i32, f32)>>>
+}
>From 4c9f1825e00557f0cd09e42dcfeff87c23e94772 Mon Sep 17 00:00:00 2001
From: Jean Perier <jperier at nvidia.com>
Date: Wed, 14 May 2025 04:13:56 -0700
Subject: [PATCH 2/6] support ZeroAttr and UndefAttr translation to
llvm::Constant
---
mlir/lib/Target/LLVMIR/ModuleTranslation.cpp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 1d4509ccb044e..8053367a10936 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -553,8 +553,10 @@ static llvm::Constant *convertDenseResourceElementsAttr(
llvm::Constant *mlir::LLVM::detail::getLLVMConstant(
llvm::Type *llvmType, Attribute attr, Location loc,
const ModuleTranslation &moduleTranslation) {
- if (!attr)
+ if (!attr || isa<UndefAttr>(attr))
return llvm::UndefValue::get(llvmType);
+ if (isa<ZeroAttr>(attr))
+ return llvm::Constant::getNullValue(llvmType);
if (auto *structType = dyn_cast<::llvm::StructType>(llvmType)) {
auto arrayAttr = dyn_cast<ArrayAttr>(attr);
if (!arrayAttr) {
@@ -723,7 +725,7 @@ llvm::Constant *mlir::LLVM::detail::getLLVMConstant(
llvm::Constant *elementCst = nullptr;
SmallVector<llvm::Constant *> constants;
constants.reserve(arrayTy->getNumElements());
- for (auto elementAttr : arrayAttr) {
+ for (Attribute elementAttr : arrayAttr) {
// Arrays with a single value or with repeating values are quite common.
// short-circuit the translation when the element value is the same as
// the previous one.
>From 63b199b64d3a2134cfd757cec99056c1c4a55d7b Mon Sep 17 00:00:00 2001
From: Jean Perier <jperier at nvidia.com>
Date: Wed, 14 May 2025 04:35:27 -0700
Subject: [PATCH 3/6] address test formatting comments
---
mlir/test/Dialect/LLVMIR/invalid.mlir | 12 ++++++------
mlir/test/Target/LLVMIR/llvmir.mlir | 12 ++++++------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/mlir/test/Dialect/LLVMIR/invalid.mlir b/mlir/test/Dialect/LLVMIR/invalid.mlir
index 4c82e586b8a3c..d66569761cf2c 100644
--- a/mlir/test/Dialect/LLVMIR/invalid.mlir
+++ b/mlir/test/Dialect/LLVMIR/invalid.mlir
@@ -1853,22 +1853,22 @@ llvm.func @gep_inbounds_flag_usage(%ptr: !llvm.ptr, %idx: i64) {
// -----
-llvm.mlir.global @x1() : !llvm.array<2x!llvm.struct<(i32, f32)>> {
- // expected-error at +1{{'llvm.mlir.constant' op array attribute size does not match array type size in dimension 0: 1 vs. 2}}
+llvm.mlir.global @bad_struct_array_init_size() : !llvm.array<2x!llvm.struct<(i32, f32)>> {
+ // expected-error at below{{'llvm.mlir.constant' op array attribute size does not match array type size in dimension 0: 1 vs. 2}}
%0 = llvm.mlir.constant([[42 : i32, 1.000000e+00 : f32]]) : !llvm.array<2x!llvm.struct<(i32, f32)>>
llvm.return %0 : !llvm.array<2x!llvm.struct<(i32, f32)>>
}
// -----
-llvm.mlir.global @x2() : !llvm.array<1x!llvm.array<1x!llvm.array<1x!llvm.struct<(i32)>>>> {
- // expected-error at +1{{'llvm.mlir.constant' op array attribute nesting must match array type nesting}}
+llvm.mlir.global @bad_struct_array_init_nesting() : !llvm.array<1x!llvm.array<1x!llvm.array<1x!llvm.struct<(i32)>>>> {
+ // expected-error at below{{'llvm.mlir.constant' op array attribute nesting must match array type nesting}}
%0 = llvm.mlir.constant([[1 : i32]]) : !llvm.array<1x!llvm.array<1x!llvm.array<1x!llvm.struct<(i32)>>>>
llvm.return %0 : !llvm.array<1x!llvm.array<1x!llvm.array<1x!llvm.struct<(i32)>>>>
}
// -----
-llvm.mlir.global @x3() : !llvm.array<1x!llvm.struct<(i32, f32)>> {
- // expected-error at +1{{'llvm.mlir.constant' op nested attribute must be an array attribute with the same number of elements as the struct type}}
+llvm.mlir.global @bad_struct_array_init_elements() : !llvm.array<1x!llvm.struct<(i32, f32)>> {
+ // expected-error at below{{'llvm.mlir.constant' op nested attribute must be an array attribute with the same number of elements as the struct type}}
%0 = llvm.mlir.constant([[1 : i32]]) : !llvm.array<1x!llvm.struct<(i32, f32)>>
llvm.return %0 : !llvm.array<1x!llvm.struct<(i32, f32)>>
}
diff --git a/mlir/test/Target/LLVMIR/llvmir.mlir b/mlir/test/Target/LLVMIR/llvmir.mlir
index 242a151116fb3..d8d8425538028 100644
--- a/mlir/test/Target/LLVMIR/llvmir.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir.mlir
@@ -3007,13 +3007,13 @@ llvm.func internal @i(%arg0: i32) attributes {dso_local} {
llvm.mlir.global internal constant @test_array_attr_1([2.5, 7.4]) : !llvm.array<2 x f64>
// CHECK: @test_array_attr_2 = global [2 x { i32, float }] [{ i32, float } { i32 42, float 1.000000e+00 }, { i32, float } { i32 42, float 1.000000e+00 }]
-llvm.mlir.global @test_array_attr_2() : !llvm.array<2x!llvm.struct<(i32, f32)>> {
- %0 = llvm.mlir.constant([[42 : i32, 1.000000e+00 : f32],[42 : i32, 1.000000e+00 : f32]]) : !llvm.array<2x!llvm.struct<(i32, f32)>>
- llvm.return %0 : !llvm.array<2x!llvm.struct<(i32, f32)>>
+llvm.mlir.global @test_array_attr_2() : !llvm.array<2 x !llvm.struct<(i32, f32)>> {
+ %0 = llvm.mlir.constant([[42 : i32, 1.000000e+00 : f32],[42 : i32, 1.000000e+00 : f32]]) : !llvm.array<2 x !llvm.struct<(i32, f32)>>
+ llvm.return %0 : !llvm.array<2 x !llvm.struct<(i32, f32)>>
}
// CHECK: @test_array_attr_3 = global [2 x [3 x { i32, float }]{{.*}}[3 x { i32, float }] [{ i32, float } { i32 1, float 1.000000e+00 }, { i32, float } { i32 2, float 1.000000e+00 }, { i32, float } { i32 3, float 1.000000e+00 }], [3 x { i32, float }] [{ i32, float } { i32 4, float 1.000000e+00 }, { i32, float } { i32 5, float 1.000000e+00 }, { i32, float } { i32 6, float 1.000000e+00 }
-llvm.mlir.global @test_array_attr_3() : !llvm.array<2x!llvm.array<3x!llvm.struct<(i32, f32)>>> {
- %0 = llvm.mlir.constant([[[1 : i32, 1.000000e+00 : f32],[2 : i32, 1.000000e+00 : f32],[3 : i32, 1.000000e+00 : f32]],[[4 : i32, 1.000000e+00 : f32],[5 : i32, 1.000000e+00 : f32],[6 : i32, 1.000000e+00 : f32]]]) : !llvm.array<2x!llvm.array<3x!llvm.struct<(i32, f32)>>>
- llvm.return %0 : !llvm.array<2x!llvm.array<3x!llvm.struct<(i32, f32)>>>
+llvm.mlir.global @test_array_attr_3() : !llvm.array<2 x !llvm.array<3 x !llvm.struct<(i32, f32)>>> {
+ %0 = llvm.mlir.constant([[[1 : i32, 1.000000e+00 : f32], [2 : i32, 1.000000e+00 : f32], [3 : i32, 1.000000e+00 : f32]], [[4 : i32, 1.000000e+00 : f32], [5 : i32, 1.000000e+00 : f32], [6 : i32, 1.000000e+00 : f32]]]) : !llvm.array<2 x !llvm.array<3 x !llvm.struct<(i32, f32)>>>
+ llvm.return %0 : !llvm.array<2 x !llvm.array<3 x !llvm.struct<(i32, f32)>>>
}
>From c04ff07ae2b3e2e7312a5b9ee7656f1ed749b081 Mon Sep 17 00:00:00 2001
From: Jean Perier <jperier at nvidia.com>
Date: Wed, 14 May 2025 06:47:36 -0700
Subject: [PATCH 4/6] refactor and restrict array of struct constant
verification
---
mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp | 112 ++++++++++++++-------
mlir/test/Dialect/LLVMIR/invalid.mlir | 16 ++-
mlir/test/Target/LLVMIR/llvmir.mlir | 15 ++-
3 files changed, 99 insertions(+), 44 deletions(-)
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 1868995e3f5ed..f495c3b642984 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -3142,6 +3142,72 @@ static bool hasScalableVectorType(Type t) {
return false;
}
+/// Verified helper for array of struct constants.
+static LogicalResult verifyStructArrayConstant(LLVM::ConstantOp op,
+ LLVM::LLVMArrayType arrayType,
+ ArrayAttr arrayAttr, int dim) {
+ if (arrayType.getNumElements() != arrayAttr.size())
+ return op.emitOpError()
+ << "array attribute size does not match array type size in "
+ "dimension "
+ << dim << ": " << arrayAttr.size() << " vs. "
+ << arrayType.getNumElements();
+
+ llvm::DenseSet<Attribute> elementsVerified;
+
+ // Recursively verify sub-dimensions for multidimensional arrays.
+ if (auto subArrayType =
+ dyn_cast<LLVM::LLVMArrayType>(arrayType.getElementType())) {
+ for (auto [idx, elementAttr] : llvm::enumerate(arrayAttr))
+ if (elementsVerified.insert(elementAttr).second) {
+ if (isa<LLVM::ZeroAttr, LLVM::UndefAttr>(elementAttr))
+ continue;
+ auto subArrayAttr = dyn_cast<ArrayAttr>(elementAttr);
+ if (!subArrayAttr)
+ return op.emitOpError()
+ << "nested attribute for sub-array in dimension " << dim
+ << " at index " << idx
+ << " must be a zero, or undef, or array attribute";
+ if (failed(verifyStructArrayConstant(op, subArrayType, subArrayAttr,
+ dim + 1)))
+ return failure();
+ }
+ return success();
+ }
+
+ // Forbid usages of ArrayAttr for simple array types that should use
+ // DenseElementsAttr instead. Note that there would be a use case for such
+ // array types when one element value is obtained via a ptr-to-int conversion
+ // from a symbol and cannot be represented in a DenseElementsAttr, but no MLIR
+ // user needs this so far, and it seems better to avoid people misusing the
+ // ArrayAttr for simple types.
+ auto structType = dyn_cast<LLVM::LLVMStructType>(arrayType.getElementType());
+ if (!structType)
+ return op.emitOpError() << "for array with an array attribute must have a "
+ "struct element type";
+
+ // Shallow verification that leaf attributes are appropriate as struct initial
+ // value.
+ size_t numStructElements = structType.getBody().size();
+ for (auto [idx, elementAttr] : llvm::enumerate(arrayAttr))
+ if (elementsVerified.insert(elementAttr).second) {
+ if (isa<LLVM::ZeroAttr, LLVM::UndefAttr>(elementAttr))
+ continue;
+ auto subArrayAttr = dyn_cast<ArrayAttr>(elementAttr);
+ if (!subArrayAttr)
+ return op.emitOpError()
+ << "nested attribute for struct element at index " << idx
+ << " must be a zero, or undef, or array attribute";
+ if (subArrayAttr.size() != numStructElements)
+ return op.emitOpError()
+ << "nested array attribute size for struct element at index "
+ << idx << " must match struct size: " << subArrayAttr.size()
+ << " vs. " << numStructElements;
+ }
+
+ return success();
+}
+
LogicalResult LLVM::ConstantOp::verify() {
if (StringAttr sAttr = llvm::dyn_cast<StringAttr>(getValue())) {
auto arrayType = llvm::dyn_cast<LLVMArrayType>(getType());
@@ -3208,7 +3274,7 @@ LogicalResult LLVM::ConstantOp::verify() {
if (isa<IntegerType>(getType()) && !getType().isInteger(floatWidth)) {
return emitOpError() << "expected integer type of width " << floatWidth;
}
- } else if (isa<ElementsAttr, ArrayAttr>(getValue())) {
+ } else if (isa<ElementsAttr>(getValue())) {
if (hasScalableVectorType(getType())) {
// The exact number of elements of a scalable vector is unknown, so we
// allow only splat attributes.
@@ -3227,44 +3293,14 @@ LogicalResult LLVM::ConstantOp::verify() {
return emitOpError()
<< "type and attribute have a different number of elements: "
<< getNumElements(getType()) << " vs. " << attrNumElements;
- } else {
- // When the attribute is an ArrayAttr, check that its nesting matches the
- // corresponding ArrayType or VectorType nesting.
- Type dimType = getType();
- Attribute dimVal = getValue();
- int dim = 0;
- while (true) {
- int64_t dimSize =
- llvm::TypeSwitch<Type, int64_t>(dimType)
- .Case<VectorType, LLVMArrayType>([&dimType](auto t) -> int64_t {
- dimType = t.getElementType();
- return t.getNumElements();
- })
- .Default([](auto) -> int64_t { return -1; });
- if (dimSize < 0)
- break;
- auto arrayAttr = dyn_cast<ArrayAttr>(dimVal);
- if (!arrayAttr)
- return emitOpError()
- << "array attribute nesting must match array type nesting";
- if (dimSize != static_cast<int64_t>(arrayAttr.size()))
- return emitOpError()
- << "array attribute size does not match array type size in "
- "dimension "
- << dim << ": " << arrayAttr.size() << " vs. " << dimSize;
- if (arrayAttr.size() == 0)
- break;
- dimVal = arrayAttr.getValue()[0];
- ++dim;
- }
- if (auto structType = dyn_cast<LLVMStructType>(dimType)) {
- auto arrayAttr = dyn_cast<ArrayAttr>(dimVal);
- if (!arrayAttr || arrayAttr.size() != structType.getBody().size())
- return emitOpError()
- << "nested attribute must be an array attribute with the same "
- "number of elements as the struct type";
- }
}
+ } else if (auto arrayAttr = dyn_cast<ArrayAttr>(getValue())) {
+ auto arrayType = dyn_cast<LLVM::LLVMArrayType>(getType());
+ if (!arrayType)
+ return emitOpError() << "expected array type";
+ // When the attribute is an ArrayAttr, check that its nesting matches the
+ // corresponding ArrayType or VectorType nesting.
+ return verifyStructArrayConstant(*this, arrayType, arrayAttr, /*dim=*/0);
} else {
return emitOpError()
<< "only supports integer, float, string or elements attributes";
diff --git a/mlir/test/Dialect/LLVMIR/invalid.mlir b/mlir/test/Dialect/LLVMIR/invalid.mlir
index d66569761cf2c..f5adf4b3bf33d 100644
--- a/mlir/test/Dialect/LLVMIR/invalid.mlir
+++ b/mlir/test/Dialect/LLVMIR/invalid.mlir
@@ -1854,21 +1854,31 @@ llvm.func @gep_inbounds_flag_usage(%ptr: !llvm.ptr, %idx: i64) {
// -----
llvm.mlir.global @bad_struct_array_init_size() : !llvm.array<2x!llvm.struct<(i32, f32)>> {
- // expected-error at below{{'llvm.mlir.constant' op array attribute size does not match array type size in dimension 0: 1 vs. 2}}
+ // expected-error at below {{'llvm.mlir.constant' op array attribute size does not match array type size in dimension 0: 1 vs. 2}}
%0 = llvm.mlir.constant([[42 : i32, 1.000000e+00 : f32]]) : !llvm.array<2x!llvm.struct<(i32, f32)>>
llvm.return %0 : !llvm.array<2x!llvm.struct<(i32, f32)>>
}
// -----
+
llvm.mlir.global @bad_struct_array_init_nesting() : !llvm.array<1x!llvm.array<1x!llvm.array<1x!llvm.struct<(i32)>>>> {
- // expected-error at below{{'llvm.mlir.constant' op array attribute nesting must match array type nesting}}
+ // expected-error at below {{'llvm.mlir.constant' op nested attribute for sub-array in dimension 1 at index 0 must be a zero, or undef, or array attribute}}
%0 = llvm.mlir.constant([[1 : i32]]) : !llvm.array<1x!llvm.array<1x!llvm.array<1x!llvm.struct<(i32)>>>>
llvm.return %0 : !llvm.array<1x!llvm.array<1x!llvm.array<1x!llvm.struct<(i32)>>>>
}
// -----
+
llvm.mlir.global @bad_struct_array_init_elements() : !llvm.array<1x!llvm.struct<(i32, f32)>> {
- // expected-error at below{{'llvm.mlir.constant' op nested attribute must be an array attribute with the same number of elements as the struct type}}
+ // expected-error at below {{'llvm.mlir.constant' op nested array attribute size for struct element at index 0 must match struct size: 1 vs. 2}}
%0 = llvm.mlir.constant([[1 : i32]]) : !llvm.array<1x!llvm.struct<(i32, f32)>>
llvm.return %0 : !llvm.array<1x!llvm.struct<(i32, f32)>>
}
+
+// ----
+
+llvm.mlir.global internal constant @bad_array_attr_simple_type() : !llvm.array<2 x f64> {
+ // expected-error at below {{'llvm.mlir.constant' op for array with an array attribute must have a struct element type}}
+ %0 = llvm.mlir.constant([2.5, 7.4]) : !llvm.array<2 x f64>
+ llvm.return %0 : !llvm.array<2 x f64>
+}
diff --git a/mlir/test/Target/LLVMIR/llvmir.mlir b/mlir/test/Target/LLVMIR/llvmir.mlir
index d8d8425538028..ccc2bec223113 100644
--- a/mlir/test/Target/LLVMIR/llvmir.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir.mlir
@@ -3003,9 +3003,6 @@ llvm.func internal @i(%arg0: i32) attributes {dso_local} {
// -----
-// CHECK: @test_array_attr_1 = internal constant [2 x double] [double 2.500000e+00, double 7.400000e+00]
-llvm.mlir.global internal constant @test_array_attr_1([2.5, 7.4]) : !llvm.array<2 x f64>
-
// CHECK: @test_array_attr_2 = global [2 x { i32, float }] [{ i32, float } { i32 42, float 1.000000e+00 }, { i32, float } { i32 42, float 1.000000e+00 }]
llvm.mlir.global @test_array_attr_2() : !llvm.array<2 x !llvm.struct<(i32, f32)>> {
%0 = llvm.mlir.constant([[42 : i32, 1.000000e+00 : f32],[42 : i32, 1.000000e+00 : f32]]) : !llvm.array<2 x !llvm.struct<(i32, f32)>>
@@ -3017,3 +3014,15 @@ llvm.mlir.global @test_array_attr_3() : !llvm.array<2 x !llvm.array<3 x !llvm.st
%0 = llvm.mlir.constant([[[1 : i32, 1.000000e+00 : f32], [2 : i32, 1.000000e+00 : f32], [3 : i32, 1.000000e+00 : f32]], [[4 : i32, 1.000000e+00 : f32], [5 : i32, 1.000000e+00 : f32], [6 : i32, 1.000000e+00 : f32]]]) : !llvm.array<2 x !llvm.array<3 x !llvm.struct<(i32, f32)>>>
llvm.return %0 : !llvm.array<2 x !llvm.array<3 x !llvm.struct<(i32, f32)>>>
}
+
+// CHECK: @test_array_attr_struct_with_ptr = internal constant [2 x { ptr }] [{ ptr } zeroinitializer, { ptr } undef]
+llvm.mlir.global internal constant @test_array_attr_struct_with_ptr() : !llvm.array<2 x struct<(ptr)>> {
+ %0 = llvm.mlir.constant([[#llvm.zero], [#llvm.undef]]) : !llvm.array<2 x struct<(ptr)>>
+ llvm.return %0 : !llvm.array<2 x struct<(ptr)>>
+}
+
+// CHECK: @test_array_attr_struct_with_struct = internal constant [3 x { i32, float }] [{ i32, float } zeroinitializer, { i32, float } { i32 2, float 1.000000e+00 }, { i32, float } undef]
+llvm.mlir.global internal constant @test_array_attr_struct_with_struct() : !llvm.array<3 x struct<(i32, f32)>> {
+ %0 = llvm.mlir.constant([#llvm.zero, [2 : i32, 1.0 : f32], #llvm.undef]) : !llvm.array<3 x struct<(i32, f32)>>
+ llvm.return %0 : !llvm.array<3 x struct<(i32, f32)>>
+}
>From d0861fd34dc5e261a958080e32dadabc99634e91 Mon Sep 17 00:00:00 2001
From: Jean Perier <jperier at nvidia.com>
Date: Wed, 14 May 2025 07:02:56 -0700
Subject: [PATCH 5/6] update llvm.constant doc
---
mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index f19f9d5a3083c..61ba8f7b991c8 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -2073,9 +2073,9 @@ def LLVM_ConstantOp
Unlike LLVM IR, MLIR does not have first-class constant values. Therefore,
all constants must be created as SSA values before being used in other
operations. `llvm.mlir.constant` creates such values for scalars, vectors,
- strings, and structs. It has a mandatory `value` attribute whose type
- depends on the type of the constant value. The type of the constant value
- must correspond to the attribute type converted to LLVM IR type.
+ strings, structs, and array of structs. It has a mandatory `value` attribute
+ whose type depends on the type of the constant value. The type of the constant
+ value must correspond to the attribute type converted to LLVM IR type.
When creating constant scalars, the `value` attribute must be either an
integer attribute or a floating point attribute. The type of the attribute
@@ -2097,6 +2097,11 @@ def LLVM_ConstantOp
must correspond to the type of the corresponding attribute element converted
to LLVM IR.
+ When creating an array of structs, the `value` attribute must be an array
+ attribute, itself containing zero, or undef, or array attributes for each
+ potential nested array type, and the elements of the leaf array attributes
+ for must match the struct element types or be zero or undef attributes.
+
Examples:
```mlir
>From c6c49b6d220a443b880117b9ffcdb9cd42740ee2 Mon Sep 17 00:00:00 2001
From: jeanPerier <jean.perier.polytechnique at gmail.com>
Date: Thu, 15 May 2025 14:20:19 +0200
Subject: [PATCH 6/6] Apply suggestions from code review
Co-authored-by: Tobias Gysi <tobias.gysi at nextsilicon.com>
---
mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp | 5 +++--
mlir/lib/Target/LLVMIR/ModuleTranslation.cpp | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index f495c3b642984..dfef4dd19ba71 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -3142,7 +3142,7 @@ static bool hasScalableVectorType(Type t) {
return false;
}
-/// Verified helper for array of struct constants.
+/// Verifies the constant array represented by `arrayAttr` matches the provided `arrayType`.
static LogicalResult verifyStructArrayConstant(LLVM::ConstantOp op,
LLVM::LLVMArrayType arrayType,
ArrayAttr arrayAttr, int dim) {
@@ -3189,7 +3189,7 @@ static LogicalResult verifyStructArrayConstant(LLVM::ConstantOp op,
// Shallow verification that leaf attributes are appropriate as struct initial
// value.
size_t numStructElements = structType.getBody().size();
- for (auto [idx, elementAttr] : llvm::enumerate(arrayAttr))
+ for (auto [idx, elementAttr] : llvm::enumerate(arrayAttr)) {
if (elementsVerified.insert(elementAttr).second) {
if (isa<LLVM::ZeroAttr, LLVM::UndefAttr>(elementAttr))
continue;
@@ -3204,6 +3204,7 @@ static LogicalResult verifyStructArrayConstant(LLVM::ConstantOp op,
<< idx << " must match struct size: " << subArrayAttr.size()
<< " vs. " << numStructElements;
}
+ }
return success();
}
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 8053367a10936..229682dff7a24 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -727,7 +727,7 @@ llvm::Constant *mlir::LLVM::detail::getLLVMConstant(
constants.reserve(arrayTy->getNumElements());
for (Attribute elementAttr : arrayAttr) {
// Arrays with a single value or with repeating values are quite common.
- // short-circuit the translation when the element value is the same as
+ // Short-circuit the translation when the element value is the same as
// the previous one.
if (!previousElementAttr || previousElementAttr != elementAttr) {
previousElementAttr = elementAttr;
More information about the Mlir-commits
mailing list