Page MenuHomePhabricator

[mlir] use OpBuilderDAG instead of OpBuilder
ClosedPublic

Authored by ftynse on Oct 23 2020, 6:31 AM.

Details

Summary

A recent commit introduced a new syntax for specifying builder arguments in
ODS, which is better amenable to automated processing, and deprecated the old
form. Transition all dialects as well as Linalg ODS generator to use the new
syntax.

Add a deprecation notice to ODS generator.

Diff Detail

Event Timeline

ftynse created this revision.Oct 23 2020, 6:31 AM
Herald added a reviewer: aartbik. · View Herald Transcript
Herald added 1 blocking reviewer(s): jpienaar. · View Herald Transcript
Herald added a reviewer: silvas. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
ftynse requested review of this revision.Oct 23 2020, 6:31 AM
rriddle accepted this revision.Oct 23 2020, 10:38 AM
rriddle added inline comments.
mlir/examples/toy/Ch2/include/toy/Ops.td
72

Can we keep this on the previous line still?

mlir/examples/toy/Ch3/include/toy/Ops.td
71

Same here and others. Keeping the brace on the previous line seems like a common stylistic thing that we do.

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
1236

Can you state which format is deprecated? and maybe hint at which one should be used instead?

mravishankar resigned from this revision.Oct 24 2020, 3:44 PM
ftynse updated this revision to Diff 300713.Oct 26 2020, 10:10 AM
ftynse marked 2 inline comments as done.

Address review

rriddle accepted this revision.Oct 26 2020, 11:22 AM

LGTM

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
1235

nit: Add braces here given the body isn't trivial.

jpienaar accepted this revision.Oct 26 2020, 5:10 PM
This revision is now accepted and ready to land.Oct 26 2020, 5:10 PM
ftynse marked 2 inline comments as done.Oct 27 2020, 1:43 AM
ftynse added inline comments.
mlir/examples/toy/Ch3/include/toy/Ops.td
71

Some files don't do this, so I kept them as is, basically reducing the style-only diff I introduced.

I wish we had tblgen-format :)

ftynse updated this revision to Diff 300915.Oct 27 2020, 1:44 AM

Address more review and rebase

This revision was automatically updated to reflect the committed changes.