This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Python] Add missing capsule->module and Context.create_module.
ClosedPublic

Authored by stellaraccident on Oct 12 2020, 9:20 PM.

Details

Diff Detail

Event Timeline

stellaraccident requested review of this revision.Oct 12 2020, 9:20 PM
mehdi_amini added inline comments.Oct 12 2020, 10:29 PM
mlir/include/mlir-c/Bindings/Python/Interop.h
93

This function isn't used anywhere right now?

mlir/include/mlir-c/Bindings/Python/Interop.h
93

Good catch. I should implement the module uniquing like I did with context, then it will be used and testable easily. We probably want that anyway, so it would be better than creating a hidden API just for testing this.

Intern modules and add tests.

stellaraccident marked an inline comment as done.Oct 13 2020, 9:36 AM

PTAL - with module interning, the capsule accessors are verified internally via tests now.

stellaraccident edited reviewers, added: mehdi_amini; removed: ftynse, silvas.Oct 13 2020, 9:47 AM
mehdi_amini accepted this revision.Oct 13 2020, 12:52 PM
mehdi_amini added inline comments.
mlir/lib/Bindings/Python/IRModules.cpp
594

Don't you need to take the GIL?

(seeing that you took it line 602)

This revision is now accepted and ready to land.Oct 13 2020, 12:52 PM
stellaraccident marked an inline comment as done.Oct 13 2020, 1:09 PM

Rebase and comment.

This revision was landed with ongoing or failed builds.Oct 13 2020, 1:17 PM
This revision was automatically updated to reflect the committed changes.
mehdi_amini added inline comments.Oct 13 2020, 1:54 PM
mlir/lib/Bindings/Python/IRModules.cpp
594

You haven't answered here, so I am not sure if you missed it or not?