[mlir][python] allow DenseIntElementsAttr for index type #118947
Merged
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
Model the IndexType as size_t when converting to a python integer. This should follow the definition of the IndexType as system-dependent. This solves the following issue when for example an attribute is defined as: ``` denseattr = dense<[1, 2, 3, 4]> : vector<4xindex> ``` which is a valid DenseElementsAttr. The change will fix an access issue when reading the attribute in python. E.g. ``` DenseIntElementsAttr(op.attributes["denseattr"]) ``` Co-authored-by: Tiago Trevisan Jost <[email protected]> Co-authored-by: Matthias Gehre <[email protected]>
mgehre-amd
requested review from
ftynse,
makslevental and
stellaraccident
as code owners
|
@llvm/pr-subscribers-mlir Author: Matthias Gehre (mgehre-amd) ChangesModel the With the python bindings, DenseIntElementsAttr(op.attributes["attr"])used to Full diff: https://github.com/llvm/llvm-project/pull/118947.diff 6 Files Affected:
diff --git a/mlir/include/mlir-c/BuiltinAttributes.h b/mlir/include/mlir-c/BuiltinAttributes.h
index 7c8c84e55b962f..ece4e7d5e4ac16 100644
--- a/mlir/include/mlir-c/BuiltinAttributes.h
+++ b/mlir/include/mlir-c/BuiltinAttributes.h
@@ -556,6 +556,8 @@ MLIR_CAPI_EXPORTED int64_t
mlirDenseElementsAttrGetInt64Value(MlirAttribute attr, intptr_t pos);
MLIR_CAPI_EXPORTED uint64_t
mlirDenseElementsAttrGetUInt64Value(MlirAttribute attr, intptr_t pos);
+MLIR_CAPI_EXPORTED size_t mlirDenseElementsAttrGetIndexValue(MlirAttribute attr,
+ intptr_t pos);
MLIR_CAPI_EXPORTED float mlirDenseElementsAttrGetFloatValue(MlirAttribute attr,
intptr_t pos);
MLIR_CAPI_EXPORTED double
diff --git a/mlir/lib/Bindings/Python/IRAttributes.cpp b/mlir/lib/Bindings/Python/IRAttributes.cpp
index cc9532f4e33b2c..f05f9f02e50faa 100644
--- a/mlir/lib/Bindings/Python/IRAttributes.cpp
+++ b/mlir/lib/Bindings/Python/IRAttributes.cpp
@@ -1172,13 +1172,19 @@ class PyDenseIntElementsAttribute
MlirType type = mlirAttributeGetType(*this);
type = mlirShapedTypeGetElementType(type);
- assert(mlirTypeIsAInteger(type) &&
- "expected integer element type in dense int elements attribute");
+ // Index type can also appear as a DenseIntElementsAttr and therefore can be
+ // casted to integer.
+ assert(mlirTypeIsAInteger(type) ||
+ mlirTypeIsAIndex(type) && "expected integer/index element type in "
+ "dense int elements attribute");
// Dispatch element extraction to an appropriate C function based on the
// elemental type of the attribute. py::int_ is implicitly constructible
// from any C++ integral type and handles bitwidth correctly.
// TODO: consider caching the type properties in the constructor to avoid
// querying them on each element access.
+ if (mlirTypeIsAIndex(type)) {
+ return mlirDenseElementsAttrGetIndexValue(*this, pos);
+ }
unsigned width = mlirIntegerTypeGetWidth(type);
bool isUnsigned = mlirIntegerTypeIsUnsigned(type);
if (isUnsigned) {
diff --git a/mlir/lib/CAPI/IR/BuiltinAttributes.cpp b/mlir/lib/CAPI/IR/BuiltinAttributes.cpp
index 11d1ade552f5a2..2a44533d781856 100644
--- a/mlir/lib/CAPI/IR/BuiltinAttributes.cpp
+++ b/mlir/lib/CAPI/IR/BuiltinAttributes.cpp
@@ -758,6 +758,9 @@ int64_t mlirDenseElementsAttrGetInt64Value(MlirAttribute attr, intptr_t pos) {
uint64_t mlirDenseElementsAttrGetUInt64Value(MlirAttribute attr, intptr_t pos) {
return llvm::cast<DenseElementsAttr>(unwrap(attr)).getValues<uint64_t>()[pos];
}
+size_t mlirDenseElementsAttrGetIndexValue(MlirAttribute attr, intptr_t pos) {
+ return llvm::cast<DenseElementsAttr>(unwrap(attr)).getValues<size_t>()[pos];
+}
float mlirDenseElementsAttrGetFloatValue(MlirAttribute attr, intptr_t pos) {
return llvm::cast<DenseElementsAttr>(unwrap(attr)).getValues<float>()[pos];
}
diff --git a/mlir/test/python/dialects/builtin.py b/mlir/test/python/dialects/builtin.py
index 18ebba61e7fea8..973a0eaeca2cdb 100644
--- a/mlir/test/python/dialects/builtin.py
+++ b/mlir/test/python/dialects/builtin.py
@@ -246,3 +246,7 @@ def testDenseElementsAttr():
# CHECK{LITERAL}: dense<[[0, 1], [2, 3]]> : tensor<2x2xi32>
print(DenseElementsAttr.get(values, type=VectorType.get((2, 2), i32)))
# CHECK{LITERAL}: dense<[[0, 1], [2, 3]]> : vector<2x2xi32>
+ idx_values = np.arange(4, dtype=np.int64)
+ idx_type = IndexType.get()
+ print(DenseElementsAttr.get(idx_values, type=VectorType.get([4], idx_type)))
+ # CHECK{LITERAL}: dense<[0, 1, 2, 3]> : vector<4xindex>
diff --git a/mlir/test/python/ir/array_attributes.py b/mlir/test/python/ir/array_attributes.py
index 256a69a939658d..ef1d835fc64012 100644
--- a/mlir/test/python/ir/array_attributes.py
+++ b/mlir/test/python/ir/array_attributes.py
@@ -572,6 +572,10 @@ def testGetDenseElementsIndex():
print(arr)
# CHECK: True
print(arr.dtype == np.int64)
+ array = np.array([1, 2, 3], dtype=np.int64)
+ attr = DenseIntElementsAttr.get(array, type=VectorType.get([3], idx_type))
+ # CHECK: [1, 2, 3]
+ print(list(DenseIntElementsAttr(attr)))
# CHECK-LABEL: TEST: testGetDenseResourceElementsAttr
diff --git a/mlir/test/python/ir/attributes.py b/mlir/test/python/ir/attributes.py
index 00c3e1b4decdb7..2f3c4460d3f590 100644
--- a/mlir/test/python/ir/attributes.py
+++ b/mlir/test/python/ir/attributes.py
@@ -366,6 +366,10 @@ def testDenseIntAttr():
# CHECK: i1
print(ShapedType(a.type).element_type)
+ shape = Attribute.parse("dense<[0, 1, 2, 3]> : vector<4xindex>")
+ # CHECK: attr: dense<[0, 1, 2, 3]>
+ print("attr:", shape)
+
@run
def testDenseArrayGetItem():
|
|
I'm always hazy on the semantics of 64b integer types - this looks fine to me but just double checking that |
|
@makslevental, who can answer your question? Is it for @stellaraccident @ftynse ? |
I mean it'd be enough if you said to me "I'm sure these are the right semantics because XYZ" |
|
@stellaraccident, I implemented your review comments. Friendly ping for review |
makslevental
approved these changes
Jan 28, 2025
There was a problem hiding this comment.
I hereby do solemnly swear I reviewed that @stellaraccident's asks were fulfilled.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/138/builds/9554 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/177/builds/12095 Here is the relevant piece of the build log for the reference |
|
Will revert and adapt to nanobind |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/116/builds/9452 Here is the relevant piece of the build log for the reference |
mgehre-amd
added a commit
to Xilinx/llvm-project
that referenced
this pull request
Jan 28, 2025
…vm#118947)" This reverts commit 1b729c3 and adapts to nanobind.
stellaraccident
approved these changes
Jan 28, 2025
mgehre-amd
added a commit
that referenced
this pull request
Jan 29, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/21217 Here is the relevant piece of the build log for the reference |
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.
Model the
IndexTypeassize_twhen converting to a python integer.With the python bindings,
used to
assertwhenattrhadindextype likedense<[1, 2, 3, 4]> : vector<4xindex>.