Page MenuHomePhabricator

[mlir][python] Add docs for op class extension mechanism.
ClosedPublic

Authored by stellaraccident on Thu, Mar 25, 3:52 PM.

Details

Diff Detail

Event Timeline

stellaraccident requested review of this revision.Thu, Mar 25, 3:52 PM
mikeurbach accepted this revision.Thu, Mar 25, 4:48 PM

Looks great, thanks for adding this. I had just one question as I read through this.

mlir/docs/Bindings/Python.md
525

most contexts

There seems to be some subtlety here (I'm not sure myself, I need to review it). Is it worth calling out when this doesn't hold?

This revision is now accepted and ready to land.Thu, Mar 25, 4:48 PM
mehdi_amini added a comment.EditedThu, Mar 25, 6:07 PM

LGTM

Thanks for more doc :)

This is complex to read, but I suspect it describes a complex mechanism in itself... (i.e. the complexity may be inherent)

stellaraccident marked an inline comment as done.Thu, Mar 25, 6:27 PM

LGTM

Thanks for more doc :)

This is complex to read, but I suspect it describes a complex mechanism in itself... (i.e. the complexity may be inherent)

Added a note: "Note that this is a rather complex mechanism and this section errs on the side
of explicitness. Users are encouraged to find an example and duplicate it if
they don't feel the need to understand the subtlety. The builtin dialect
provides some relatively simple examples."

I struggled with the level to write it. Open to revisions!

mlir/docs/Bindings/Python.md
525

Added a note. The caveats are not something that regular use should ever hit. More of a cautionary tale to avoid pushing the edges of the abstraction.

stellaraccident marked an inline comment as done.

Comments and rebase.

Added a note

Yeah that's a great note!

Not sure why it didn't get marked as landed.