Page MenuHomePhabricator

[mlir] Enable specifying verify on OpInterface
ClosedPublic

Authored by jpienaar on Tue, Jan 21, 10:06 AM.

Details

Summary

Add method in ODS to specify verification for operations implementing a
OpInterface. Use this with infer type op interface to verify that the
inferred type matches the return type and remove special case in
TestPatterns.

This could also have been achieved by using OpInterfaceMethod but verify
seems pretty common and it is not an arbitrary method that just happened
to be named verifyTrait, so having it be defined in special way seems
appropriate/better documenting.

Diff Detail

Event Timeline

jpienaar created this revision.Tue, Jan 21, 10:06 AM

Unit tests: pass. 62059 tests passed, 0 failed and 784 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

rriddle added inline comments.Tue, Jan 21, 10:54 AM
mlir/include/mlir/Analysis/InferTypeOpInterface.td
65

Is '$_op' documented somewhere?

mlir/include/mlir/IR/OpBase.td
1417

Why the underscore?

1445

Why the newlines?

mlir/tools/mlir-tblgen/OpInterfacesGen.cpp
160

nit: verify -> 'verify' field

mehdi_amini accepted this revision.Tue, Jan 21, 10:57 AM

Nice!

I was talking to someone last week about this, I just don't remember who :)

mlir/include/mlir/IR/OpBase.td
1446

spurious?

This revision is now accepted and ready to land.Tue, Jan 21, 10:57 AM
jpienaar updated this revision to Diff 239546.Wed, Jan 22, 4:42 AM
jpienaar marked 7 inline comments as done.

Quoted 'verify', removed spurious newlines.

Updated thanks

mlir/include/mlir/Analysis/InferTypeOpInterface.td
65

Yes, in OpBase and this matches placeholder we use in other code body replacements.

mlir/include/mlir/IR/OpBase.td
1417

To match the same placeholder in line 79. I was tempted to just document that op will refer to the op, but given we already have those placeholders, kept it consistent.

This revision was automatically updated to reflect the committed changes.

Unit tests: pass. 62095 tests passed, 0 failed and 785 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

rriddle added inline comments.Wed, Jan 22, 8:42 AM
mlir/include/mlir/IR/OpBase.td
1417

This isn't consistent with the rest of the OpInterface methods though.