Page MenuHomePhabricator

[mlir:OpConversionPattern] Add overloads for taking an Adaptor instead of ArrayRef

Authored by rriddle on Wed, Sep 22, 3:14 PM.



This has been a TODO for a long time, and it brings about many advantages (namely nice accessors, and less fragile code). The existing overloads that accept ArrayRef are now treated as deprecated and will be removed in a followup (after a small grace period). Most of the upstream MLIR usages have been fixed by this commit, the rest will be handled in a followup.

Diff Detail

Event Timeline

rriddle created this revision.Wed, Sep 22, 3:14 PM
rriddle requested review of this revision.Wed, Sep 22, 3:14 PM
ftynse accepted this revision.Thu, Sep 23, 12:55 AM

Very nice usability improvement!


Nit: I know this is a large-scale refactoring, but could we fix this particular place in the documentation to be adaptor.source() instead of using getOperands + magic number? For the sake of documenting best practices.


Is it too early to put LLVM_ATTRIBUTE_DEPRECATED here?

This revision is now accepted and ready to land.Thu, Sep 23, 12:55 AM
rriddle updated this revision to Diff 374649.Thu, Sep 23, 12:45 PM
rriddle marked 2 inline comments as done.


aartbik added inline comments.Thu, Sep 23, 12:59 PM

heads up: I added an emitCInterface parameter to some of these calls at exactly the same line, so you probably need to resolve some conflicts on the rebase with main

Thanks for the review Alex!

(Also thanks for the heads up Aart!)


Yeah, I think so. I'm going to send out the PSA first and then mark deprecated sometime early next week.

antiagainst accepted this revision.Thu, Sep 23, 3:23 PM

This is nice!

This revision was automatically updated to reflect the committed changes.
rriddle marked an inline comment as done.