[mlir][IR] Add Builder::getArrayAttr overload for concrete attributes
#170870
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
matthias-springer
requested review from
Groverkss,
PeimingLiu,
aartbik,
banach-space,
dcaballe,
nicolasvasilache,
rengolin and
yinying-lisa-li
as code owners
llvmbot
added
mlir:core
Member
|
@llvm/pr-subscribers-mlir-linalg @llvm/pr-subscribers-mlir-sparse Author: Matthias Springer (matthias-springer) ChangesThis commit adds a new Full diff: https://github.com/llvm/llvm-project/pull/170870.diff 6 Files Affected:
diff --git a/mlir/include/mlir/IR/Builders.h b/mlir/include/mlir/IR/Builders.h
index 3ba6818204ba0..09d6f0612c7ac 100644
--- a/mlir/include/mlir/IR/Builders.h
+++ b/mlir/include/mlir/IR/Builders.h
@@ -112,6 +112,15 @@ class Builder {
StringAttr getStringAttr(const Twine &bytes);
ArrayAttr getArrayAttr(ArrayRef<Attribute> value);
+ // Convenience method for containers of specific attribute types. E.g., this
+ // overload will match SmallVector<IntegerAttr>.
+ template <typename ContainerTy>
+ ArrayAttr getArrayAttr(const ContainerTy &value) {
+ auto ref = ArrayRef(value);
+ return getArrayAttr(ArrayRef<Attribute>(
+ static_cast<const Attribute *>(ref.data()), ref.size()));
+ }
+
// Returns a 0-valued attribute of the given `type`. This function only
// supports boolean, integer, and 16-/32-/64-bit float types, and vector or
// ranked tensor of them. Returns null attribute otherwise.
diff --git a/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp b/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
index 33ec79b1b4b1b..21a29aaf6bfd6 100644
--- a/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
+++ b/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
@@ -193,12 +193,10 @@ static void buildMatmulOp(OpBuilder &b, OperationState &state,
RegionBuilderFn regionBuilder,
ArrayRef<AffineMap> indexingMaps) {
// Initialize indexingMaps attribute, for MatmulOp.
- SmallVector<Attribute, 3> indexingMapsAttrVal;
- indexingMapsAttrVal =
- llvm::map_to_vector(indexingMaps, [](AffineMap map) -> Attribute {
- return AffineMapAttr::get(map);
- });
- state.addAttribute("indexing_maps", b.getArrayAttr(indexingMapsAttrVal));
+ state.addAttribute("indexing_maps", b.getArrayAttr(llvm::map_to_vector(
+ indexingMaps, [](AffineMap map) {
+ return AffineMapAttr::get(map);
+ })));
return buildStructuredOp(b, state, resultTensorTypes, inputs, outputs,
attributes, regionBuilder);
}
@@ -210,12 +208,10 @@ static void buildBatchMatmulOp(OpBuilder &b, OperationState &state,
RegionBuilderFn regionBuilder,
ArrayRef<AffineMap> indexingMaps) {
// Initialize indexingMaps attribute, for BatchMatmulOp.
- SmallVector<Attribute, 4> indexingMapsAttrVal;
- indexingMapsAttrVal =
- llvm::map_to_vector(indexingMaps, [](AffineMap map) -> Attribute {
- return AffineMapAttr::get(map);
- });
- state.addAttribute("indexing_maps", b.getArrayAttr(indexingMapsAttrVal));
+ state.addAttribute("indexing_maps", b.getArrayAttr(llvm::map_to_vector(
+ indexingMaps, [](AffineMap map) {
+ return AffineMapAttr::get(map);
+ })));
return buildStructuredOp(b, state, resultTensorTypes, inputs, outputs,
attributes, regionBuilder);
}
@@ -227,12 +223,10 @@ static void buildBatchReduceMatmulOp(OpBuilder &b, OperationState &state,
RegionBuilderFn regionBuilder,
ArrayRef<AffineMap> indexingMaps) {
// Initialize indexingMaps attribute, for BatchReduceMatmulOp.
- SmallVector<Attribute, 4> indexingMapsAttrVal;
- indexingMapsAttrVal =
- llvm::map_to_vector(indexingMaps, [](AffineMap map) -> Attribute {
- return AffineMapAttr::get(map);
- });
- state.addAttribute("indexing_maps", b.getArrayAttr(indexingMapsAttrVal));
+ state.addAttribute("indexing_maps", b.getArrayAttr(llvm::map_to_vector(
+ indexingMaps, [](AffineMap map) {
+ return AffineMapAttr::get(map);
+ })));
return buildStructuredOp(b, state, resultTensorTypes, inputs, outputs,
attributes, regionBuilder);
}
@@ -1121,7 +1115,7 @@ void GenericOp::build(
builder.getAffineMapArrayAttr(indexingMaps),
builder.getArrayAttr(llvm::to_vector(llvm::map_range(
iteratorTypes,
- [&](utils::IteratorType iter) -> mlir::Attribute {
+ [&](utils::IteratorType iter) {
return IteratorTypeAttr::get(builder.getContext(), iter);
}))),
doc.empty() ? StringAttr() : builder.getStringAttr(doc),
@@ -3914,7 +3908,7 @@ ParseResult MatmulOp::parse(OpAsmParser &parser, OperationState &result) {
if (*indexingMapsAttr == nullptr) {
auto indexingMapAttrs = llvm::map_to_vector(
MatmulOp::getDefaultIndexingMaps(parser.getContext()),
- [](AffineMap map) -> Attribute { return AffineMapAttr::get(map); });
+ [](AffineMap map) { return AffineMapAttr::get(map); });
indexingMapsAttr = parser.getBuilder().getArrayAttr(indexingMapAttrs);
}
diff --git a/mlir/lib/Dialect/SparseTensor/Transforms/SparseReinterpretMap.cpp b/mlir/lib/Dialect/SparseTensor/Transforms/SparseReinterpretMap.cpp
index 0fc5cc76de39c..e3220815b94d1 100644
--- a/mlir/lib/Dialect/SparseTensor/Transforms/SparseReinterpretMap.cpp
+++ b/mlir/lib/Dialect/SparseTensor/Transforms/SparseReinterpretMap.cpp
@@ -325,10 +325,9 @@ translateMap(linalg::GenericOp op, PatternRewriter &rewriter) {
}
};
- SmallVector<Attribute> iterAttr =
- llvm::map_to_vector(itTps, [ctx](auto itTp) -> Attribute {
- return linalg::IteratorTypeAttr::get(ctx, itTp);
- });
+ auto iterAttr = llvm::map_to_vector(itTps, [ctx](auto itTp) {
+ return linalg::IteratorTypeAttr::get(ctx, itTp);
+ });
return std::make_pair(rewriter.getAffineMapArrayAttr(idxMapArray),
rewriter.getArrayAttr(iterAttr));
diff --git a/mlir/lib/Dialect/Utils/ReshapeOpsUtils.cpp b/mlir/lib/Dialect/Utils/ReshapeOpsUtils.cpp
index 6e9118e1f7b0b..0f1682f045077 100644
--- a/mlir/lib/Dialect/Utils/ReshapeOpsUtils.cpp
+++ b/mlir/lib/Dialect/Utils/ReshapeOpsUtils.cpp
@@ -422,11 +422,10 @@ static unsigned getMaxPosOfType(ArrayRef<ReassociationExprs> exprArrays) {
ArrayAttr mlir::getReassociationIndicesAttribute(
Builder &b, ArrayRef<ReassociationIndices> reassociation) {
- SmallVector<Attribute, 4> reassociationAttr =
- llvm::to_vector<4>(llvm::map_range(
- reassociation, [&](const ReassociationIndices &indices) -> Attribute {
- return cast<Attribute>(b.getI64ArrayAttr(indices));
- }));
+ auto reassociationAttr = llvm::map_to_vector(
+ reassociation, [&](const ReassociationIndices &indices) {
+ return b.getI64ArrayAttr(indices);
+ });
return b.getArrayAttr(reassociationAttr);
}
diff --git a/mlir/lib/Dialect/Vector/IR/VectorOps.cpp b/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
index 2789f63555524..1f3611d4cd6a3 100644
--- a/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
+++ b/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
@@ -798,10 +798,10 @@ void vector::ContractionOp::build(OpBuilder &builder, OperationState &result,
AffineMap::inferFromExprList(indexingExprs, builder.getContext())));
result.addAttribute(
getIteratorTypesAttrName(result.name),
- builder.getArrayAttr(llvm::to_vector(llvm::map_range(
- iteratorTypes, [&](IteratorType t) -> mlir::Attribute {
+ builder.getArrayAttr(
+ llvm::map_to_vector(iteratorTypes, [&](IteratorType t) {
return IteratorTypeAttr::get(builder.getContext(), t);
- }))));
+ })));
}
void vector::ContractionOp::build(OpBuilder &builder, OperationState &result,
diff --git a/mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp b/mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp
index 726da1e9a3d14..a5f20d52fdb71 100644
--- a/mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp
+++ b/mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp
@@ -105,10 +105,10 @@ struct MultiReduceToContract
rewriter.replaceOpWithNewOp<mlir::vector::ContractionOp>(
reduceOp, mulOp->getOperand(0), mulOp->getOperand(1), reduceOp.getAcc(),
rewriter.getAffineMapArrayAttr({srcMap, srcMap, dstMap}),
- rewriter.getArrayAttr(llvm::to_vector(llvm::map_range(
- iteratorTypes, [&](IteratorType t) -> mlir::Attribute {
+ rewriter.getArrayAttr(
+ llvm::map_to_vector(iteratorTypes, [&](IteratorType t) {
return IteratorTypeAttr::get(rewriter.getContext(), t);
- }))));
+ })));
return success();
}
};
|
Member
|
@llvm/pr-subscribers-mlir Author: Matthias Springer (matthias-springer) ChangesThis commit adds a new Full diff: https://github.com/llvm/llvm-project/pull/170870.diff 6 Files Affected:
diff --git a/mlir/include/mlir/IR/Builders.h b/mlir/include/mlir/IR/Builders.h
index 3ba6818204ba0..09d6f0612c7ac 100644
--- a/mlir/include/mlir/IR/Builders.h
+++ b/mlir/include/mlir/IR/Builders.h
@@ -112,6 +112,15 @@ class Builder {
StringAttr getStringAttr(const Twine &bytes);
ArrayAttr getArrayAttr(ArrayRef<Attribute> value);
+ // Convenience method for containers of specific attribute types. E.g., this
+ // overload will match SmallVector<IntegerAttr>.
+ template <typename ContainerTy>
+ ArrayAttr getArrayAttr(const ContainerTy &value) {
+ auto ref = ArrayRef(value);
+ return getArrayAttr(ArrayRef<Attribute>(
+ static_cast<const Attribute *>(ref.data()), ref.size()));
+ }
+
// Returns a 0-valued attribute of the given `type`. This function only
// supports boolean, integer, and 16-/32-/64-bit float types, and vector or
// ranked tensor of them. Returns null attribute otherwise.
diff --git a/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp b/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
index 33ec79b1b4b1b..21a29aaf6bfd6 100644
--- a/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
+++ b/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
@@ -193,12 +193,10 @@ static void buildMatmulOp(OpBuilder &b, OperationState &state,
RegionBuilderFn regionBuilder,
ArrayRef<AffineMap> indexingMaps) {
// Initialize indexingMaps attribute, for MatmulOp.
- SmallVector<Attribute, 3> indexingMapsAttrVal;
- indexingMapsAttrVal =
- llvm::map_to_vector(indexingMaps, [](AffineMap map) -> Attribute {
- return AffineMapAttr::get(map);
- });
- state.addAttribute("indexing_maps", b.getArrayAttr(indexingMapsAttrVal));
+ state.addAttribute("indexing_maps", b.getArrayAttr(llvm::map_to_vector(
+ indexingMaps, [](AffineMap map) {
+ return AffineMapAttr::get(map);
+ })));
return buildStructuredOp(b, state, resultTensorTypes, inputs, outputs,
attributes, regionBuilder);
}
@@ -210,12 +208,10 @@ static void buildBatchMatmulOp(OpBuilder &b, OperationState &state,
RegionBuilderFn regionBuilder,
ArrayRef<AffineMap> indexingMaps) {
// Initialize indexingMaps attribute, for BatchMatmulOp.
- SmallVector<Attribute, 4> indexingMapsAttrVal;
- indexingMapsAttrVal =
- llvm::map_to_vector(indexingMaps, [](AffineMap map) -> Attribute {
- return AffineMapAttr::get(map);
- });
- state.addAttribute("indexing_maps", b.getArrayAttr(indexingMapsAttrVal));
+ state.addAttribute("indexing_maps", b.getArrayAttr(llvm::map_to_vector(
+ indexingMaps, [](AffineMap map) {
+ return AffineMapAttr::get(map);
+ })));
return buildStructuredOp(b, state, resultTensorTypes, inputs, outputs,
attributes, regionBuilder);
}
@@ -227,12 +223,10 @@ static void buildBatchReduceMatmulOp(OpBuilder &b, OperationState &state,
RegionBuilderFn regionBuilder,
ArrayRef<AffineMap> indexingMaps) {
// Initialize indexingMaps attribute, for BatchReduceMatmulOp.
- SmallVector<Attribute, 4> indexingMapsAttrVal;
- indexingMapsAttrVal =
- llvm::map_to_vector(indexingMaps, [](AffineMap map) -> Attribute {
- return AffineMapAttr::get(map);
- });
- state.addAttribute("indexing_maps", b.getArrayAttr(indexingMapsAttrVal));
+ state.addAttribute("indexing_maps", b.getArrayAttr(llvm::map_to_vector(
+ indexingMaps, [](AffineMap map) {
+ return AffineMapAttr::get(map);
+ })));
return buildStructuredOp(b, state, resultTensorTypes, inputs, outputs,
attributes, regionBuilder);
}
@@ -1121,7 +1115,7 @@ void GenericOp::build(
builder.getAffineMapArrayAttr(indexingMaps),
builder.getArrayAttr(llvm::to_vector(llvm::map_range(
iteratorTypes,
- [&](utils::IteratorType iter) -> mlir::Attribute {
+ [&](utils::IteratorType iter) {
return IteratorTypeAttr::get(builder.getContext(), iter);
}))),
doc.empty() ? StringAttr() : builder.getStringAttr(doc),
@@ -3914,7 +3908,7 @@ ParseResult MatmulOp::parse(OpAsmParser &parser, OperationState &result) {
if (*indexingMapsAttr == nullptr) {
auto indexingMapAttrs = llvm::map_to_vector(
MatmulOp::getDefaultIndexingMaps(parser.getContext()),
- [](AffineMap map) -> Attribute { return AffineMapAttr::get(map); });
+ [](AffineMap map) { return AffineMapAttr::get(map); });
indexingMapsAttr = parser.getBuilder().getArrayAttr(indexingMapAttrs);
}
diff --git a/mlir/lib/Dialect/SparseTensor/Transforms/SparseReinterpretMap.cpp b/mlir/lib/Dialect/SparseTensor/Transforms/SparseReinterpretMap.cpp
index 0fc5cc76de39c..e3220815b94d1 100644
--- a/mlir/lib/Dialect/SparseTensor/Transforms/SparseReinterpretMap.cpp
+++ b/mlir/lib/Dialect/SparseTensor/Transforms/SparseReinterpretMap.cpp
@@ -325,10 +325,9 @@ translateMap(linalg::GenericOp op, PatternRewriter &rewriter) {
}
};
- SmallVector<Attribute> iterAttr =
- llvm::map_to_vector(itTps, [ctx](auto itTp) -> Attribute {
- return linalg::IteratorTypeAttr::get(ctx, itTp);
- });
+ auto iterAttr = llvm::map_to_vector(itTps, [ctx](auto itTp) {
+ return linalg::IteratorTypeAttr::get(ctx, itTp);
+ });
return std::make_pair(rewriter.getAffineMapArrayAttr(idxMapArray),
rewriter.getArrayAttr(iterAttr));
diff --git a/mlir/lib/Dialect/Utils/ReshapeOpsUtils.cpp b/mlir/lib/Dialect/Utils/ReshapeOpsUtils.cpp
index 6e9118e1f7b0b..0f1682f045077 100644
--- a/mlir/lib/Dialect/Utils/ReshapeOpsUtils.cpp
+++ b/mlir/lib/Dialect/Utils/ReshapeOpsUtils.cpp
@@ -422,11 +422,10 @@ static unsigned getMaxPosOfType(ArrayRef<ReassociationExprs> exprArrays) {
ArrayAttr mlir::getReassociationIndicesAttribute(
Builder &b, ArrayRef<ReassociationIndices> reassociation) {
- SmallVector<Attribute, 4> reassociationAttr =
- llvm::to_vector<4>(llvm::map_range(
- reassociation, [&](const ReassociationIndices &indices) -> Attribute {
- return cast<Attribute>(b.getI64ArrayAttr(indices));
- }));
+ auto reassociationAttr = llvm::map_to_vector(
+ reassociation, [&](const ReassociationIndices &indices) {
+ return b.getI64ArrayAttr(indices);
+ });
return b.getArrayAttr(reassociationAttr);
}
diff --git a/mlir/lib/Dialect/Vector/IR/VectorOps.cpp b/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
index 2789f63555524..1f3611d4cd6a3 100644
--- a/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
+++ b/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
@@ -798,10 +798,10 @@ void vector::ContractionOp::build(OpBuilder &builder, OperationState &result,
AffineMap::inferFromExprList(indexingExprs, builder.getContext())));
result.addAttribute(
getIteratorTypesAttrName(result.name),
- builder.getArrayAttr(llvm::to_vector(llvm::map_range(
- iteratorTypes, [&](IteratorType t) -> mlir::Attribute {
+ builder.getArrayAttr(
+ llvm::map_to_vector(iteratorTypes, [&](IteratorType t) {
return IteratorTypeAttr::get(builder.getContext(), t);
- }))));
+ })));
}
void vector::ContractionOp::build(OpBuilder &builder, OperationState &result,
diff --git a/mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp b/mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp
index 726da1e9a3d14..a5f20d52fdb71 100644
--- a/mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp
+++ b/mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp
@@ -105,10 +105,10 @@ struct MultiReduceToContract
rewriter.replaceOpWithNewOp<mlir::vector::ContractionOp>(
reduceOp, mulOp->getOperand(0), mulOp->getOperand(1), reduceOp.getAcc(),
rewriter.getAffineMapArrayAttr({srcMap, srcMap, dstMap}),
- rewriter.getArrayAttr(llvm::to_vector(llvm::map_range(
- iteratorTypes, [&](IteratorType t) -> mlir::Attribute {
+ rewriter.getArrayAttr(
+ llvm::map_to_vector(iteratorTypes, [&](IteratorType t) {
return IteratorTypeAttr::get(rewriter.getContext(), t);
- }))));
+ })));
return success();
}
};
|
joker-eph
reviewed
Dec 5, 2025
| ArrayAttr getArrayAttr(const ContainerTy &value) { | ||
| auto ref = ArrayRef(value); | ||
| return getArrayAttr(ArrayRef<Attribute>( | ||
| static_cast<const Attribute *>(ref.data()), ref.size())); |
Collaborator
There was a problem hiding this comment.
This isn't safe in general in C++ to cast an array of Derived to an array of Base objects.
I'm not sure if we are in a special case with our attribute hierarchy but that seems scary actually.
Member
Author
There was a problem hiding this comment.
I thought it would be safe for Attribute because derived attributes don't have any extra state. (Everything is in the impl (?).) Something seems to be wrong though because the Windows build is failing...
Collaborator
There was a problem hiding this comment.
The math work if you think of it in terms of assembly, but C++ rules are more abstracts.
For example you can't take unrelated structures and do the same kind of things (or accessing similar fields) even if they have the same memory layout in practice.
It's also why memcpy is the safest "type punning" idiom I believe.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit adds a new
Builder::getArrayAttroverload that accepts parameters such asSmallVector<IntegerAttr>. Previously, only containers such asSmallVector<Attribute>were accepted, so programmers had to explicitly cast toAttributein some cases.