-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[MLIR] [Python] ir.Value is now generic in the type of the value it holds
#166148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-mlir-core @llvm/pr-subscribers-mlir Author: Sergei Lebedev (superbobry) ChangesThis makes it similar to Full diff: https://github.com/llvm/llvm-project/pull/166148.diff 1 Files Affected:
diff --git a/mlir/lib/Bindings/Python/IRCore.cpp b/mlir/lib/Bindings/Python/IRCore.cpp
index cda4fe19c16f8..1ef058af2c1ef 100644
--- a/mlir/lib/Bindings/Python/IRCore.cpp
+++ b/mlir/lib/Bindings/Python/IRCore.cpp
@@ -18,6 +18,7 @@
#include "mlir/Bindings/Python/Nanobind.h"
#include "mlir/Bindings/Python/NanobindAdaptors.h"
#include "nanobind/nanobind.h"
+#include "nanobind/typing.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallVector.h"
@@ -4278,7 +4279,10 @@ void mlir::python::populateIRCore(nb::module_ &m) {
//----------------------------------------------------------------------------
// Mapping of Value.
//----------------------------------------------------------------------------
- nb::class_<PyValue>(m, "Value")
+ m.attr("_T") = nb::type_var("_T", nb::arg("bound") = m.attr("Type"));
+
+ nb::class_<PyValue>(m, "Value", nb::is_generic(),
+ nb::sig("class Value(Generic[_T])"))
.def(nb::init<PyValue &>(), nb::keep_alive<0, 1>(), nb::arg("value"))
.def_prop_ro(MLIR_PYTHON_CAPI_PTR_ATTR, &PyValue::getCapsule)
.def_static(MLIR_PYTHON_CAPI_FACTORY_ATTR, &PyValue::createFromCapsule)
@@ -4371,18 +4375,20 @@ void mlir::python::populateIRCore(nb::module_ &m) {
return printAccum.join();
},
nb::arg("state"), kGetNameAsOperand)
- .def_prop_ro("type",
- [](PyValue &self) -> nb::typed<nb::object, PyType> {
- return PyType(self.getParentOperation()->getContext(),
- mlirValueGetType(self.get()))
- .maybeDownCast();
- })
+ .def_prop_ro(
+ "type",
+ [](PyValue &self) {
+ return PyType(self.getParentOperation()->getContext(),
+ mlirValueGetType(self.get()))
+ .maybeDownCast();
+ },
+ nb::sig("def type(self) -> _T"))
.def(
"set_type",
[](PyValue &self, const PyType &type) {
return mlirValueSetType(self.get(), type);
},
- nb::arg("type"))
+ nb::arg("type"), nb::sig("def set_type(self, type: _T)"))
.def(
"replace_all_uses_with",
[](PyValue &self, PyValue &with) {
|
e8d2178 to
7eec476
Compare
7eec476 to
46e29cb
Compare
46e29cb to
27e8308
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
27e8308 to
35ea06e
Compare
makslevental
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (modulo one nit)
5a54a04 to
1091d12
Compare
… holds This makes it similar to `mlir::TypedValue` in the MLIR C++ API and allows users to be more specific about the values they produce or accept. Co-authored-by: Maksim Levental <[email protected]>
1091d12 to
bce885c
Compare
| if (std::strcmp(kind, "operand") == 0) { | ||
| StringRef pythonType = getPythonType(element.constraint.getCppType()); | ||
| if (!pythonType.empty()) | ||
| type += "[" + pythonType.str() + "]"; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is an error here. type may be "_ods_ir.OpOperandList" here which is not Generic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed here #167930
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix!
Change in #166148 caused breaks for some other types. Specifically this error was seen in a downstream project ``` _ods_ir.OpOperandList[_ods_ir.IntegerType]: TypeError: type 'iree.compiler._mlir_libs._mlir.ir.OpOperandList' is not subscriptable ``` This PR tries to make those changes not affect the other types --------- Signed-off-by: Nirvedh Meshram <[email protected]>
…(#167930) Change in llvm/llvm-project#166148 caused breaks for some other types. Specifically this error was seen in a downstream project ``` _ods_ir.OpOperandList[_ods_ir.IntegerType]: TypeError: type 'iree.compiler._mlir_libs._mlir.ir.OpOperandList' is not subscriptable ``` This PR tries to make those changes not affect the other types --------- Signed-off-by: Nirvedh Meshram <[email protected]>
Change in llvm#166148 caused breaks for some other types. Specifically this error was seen in a downstream project ``` _ods_ir.OpOperandList[_ods_ir.IntegerType]: TypeError: type 'iree.compiler._mlir_libs._mlir.ir.OpOperandList' is not subscriptable ``` This PR tries to make those changes not affect the other types --------- Signed-off-by: Nirvedh Meshram <[email protected]>
This makes it similar to
mlir::TypedValuein the MLIR C++ API and allows users to be more specific about the values they produce or accept.