Skip to content

Conversation

@makslevental
Copy link
Contributor

@makslevental makslevental commented Oct 24, 2025

#157930 changed nb::object getOwner() to PyOpView getOwner() which implicitly constructs the generic OpView from a (possibly) concrete OpView. This PR fixes that.

@uenoku uenoku mentioned this pull request Oct 25, 2025
@makslevental makslevental changed the title [MLIR][Python] fix getOwner [MLIR][Python] fix getOwner to return (typed) nb::object instead of abstract PyOpView Oct 25, 2025
@makslevental makslevental marked this pull request as ready for review October 25, 2025 17:43
@llvmbot llvmbot added the mlir label Oct 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 25, 2025

@llvm/pr-subscribers-mlir

Author: Maksim Levental (makslevental)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/165053.diff

1 Files Affected:

  • (modified) mlir/lib/Bindings/Python/IRCore.cpp (+1-1)
diff --git a/mlir/lib/Bindings/Python/IRCore.cpp b/mlir/lib/Bindings/Python/IRCore.cpp
index 06d0256e6287b..cda4fe19c16f8 100644
--- a/mlir/lib/Bindings/Python/IRCore.cpp
+++ b/mlir/lib/Bindings/Python/IRCore.cpp
@@ -598,7 +598,7 @@ class PyOpOperand {
 public:
   PyOpOperand(MlirOpOperand opOperand) : opOperand(opOperand) {}
 
-  PyOpView getOwner() {
+  nb::typed<nb::object, PyOpView> getOwner() {
     MlirOperation owner = mlirOpOperandGetOwner(opOperand);
     PyMlirContextRef context =
         PyMlirContext::forContext(mlirOperationGetContext(owner));

@github-actions
Copy link

github-actions bot commented Oct 25, 2025

✅ With the latest revision this PR passed the Python code formatter.

@makslevental makslevental force-pushed the users/makslevental/fix-getOwner branch 7 times, most recently from 2bab814 to 61d0cdd Compare October 25, 2025 18:13
@makslevental
Copy link
Contributor Author

Okay this is going to take a little more work because I notice that PyOpOperandIterator isn't being used for op.operands and I'd like to fix that too.

Copy link
Contributor

@fabianschuiki fabianschuiki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for fixing this 🥳!

@makslevental makslevental force-pushed the users/makslevental/fix-getOwner branch from 61d0cdd to cac248c Compare October 26, 2025 01:21
@makslevental
Copy link
Contributor Author

Merging to unblock - will fix the rest later (see #165126)

@makslevental makslevental enabled auto-merge (squash) October 26, 2025 01:24
@makslevental makslevental merged commit c05ce9b into main Oct 26, 2025
10 checks passed
@makslevental makslevental deleted the users/makslevental/fix-getOwner branch October 26, 2025 01:48
varun-r-mallya pushed a commit to varun-r-mallya/llvm-project that referenced this pull request Oct 27, 2025
…bstract PyOpView (llvm#165053)

llvm#157930 changed `nb::object
getOwner()` to `PyOpView getOwner()` which implicitly constructs the
generic OpView against from a (possibly) concrete OpView. This PR fixes
that.
dvbuka pushed a commit to dvbuka/llvm-project that referenced this pull request Oct 27, 2025
…bstract PyOpView (llvm#165053)

llvm#157930 changed `nb::object
getOwner()` to `PyOpView getOwner()` which implicitly constructs the
generic OpView against from a (possibly) concrete OpView. This PR fixes
that.
Lukacma pushed a commit to Lukacma/llvm-project that referenced this pull request Oct 29, 2025
…bstract PyOpView (llvm#165053)

llvm#157930 changed `nb::object
getOwner()` to `PyOpView getOwner()` which implicitly constructs the
generic OpView against from a (possibly) concrete OpView. This PR fixes
that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants