This is an archive of the discontinued LLVM Phabricator instance.

[mlir][ods] Optional Attribute or Type Parameters
ClosedPublic

Authored by Mogball on Jan 25 2022, 6:28 PM.

Details

Summary

Implements optional attribute or type parameters, including support for such parameters in the assembly format struct directive. Also implements optional groups.

Depends on D117971

Diff Detail

Event Timeline

Mogball created this revision.Jan 25 2022, 6:28 PM
Mogball requested review of this revision.Jan 25 2022, 6:28 PM
mehdi_amini added inline comments.Jan 25 2022, 11:45 PM
mlir/docs/Tutorials/DefiningAttributesAndTypes.md
1

Seeing this file path again, that reminds me: wasn't it supposed to be converted to a doc like the other instead of being under "tutorial"?

rriddle added inline comments.Jan 25 2022, 11:48 PM
mlir/docs/Tutorials/DefiningAttributesAndTypes.md
1

Oooh, never pushed that for review. Let me send that out tomorrow.

rriddle added inline comments.Jan 31 2022, 1:09 AM
mlir/docs/Tutorials/DefiningAttributesAndTypes.md
504

?

mlir/include/mlir/IR/OpBase.td
3119
mlir/tools/mlir-tblgen/AttrOrTypeFormatGen.cpp
260–264
328–330

Avoid floating statements.

373

This seems quadratic, why not cache this computation off somewhere.

402

nit: Why not use ods as prefix instead of _?

548

Is the *_result_{0} here presenting a parse failure?

568–569
735–736
Mogball marked 11 inline comments as done.Jan 31 2022, 9:38 PM
Mogball added inline comments.
mlir/tools/mlir-tblgen/AttrOrTypeFormatGen.cpp
548

This condition guards on "no value". E.g. FailureOr<Attribute> = failure() is parse failure but FailureOr<Attribute> = nullptr is no error but no value either.

Mogball updated this revision to Diff 404817.Jan 31 2022, 11:09 PM
Mogball marked an inline comment as done.

review comments

Mogball updated this revision to Diff 404821.Jan 31 2022, 11:18 PM

review comments

rriddle accepted this revision.Feb 7 2022, 11:26 AM
This revision is now accepted and ready to land.Feb 7 2022, 11:26 AM

The windows build is failing BTW, might want to look into that.

Mogball updated this revision to Diff 406656.Feb 7 2022, 5:15 PM

fix windows build

Mogball updated this revision to Diff 406869.Feb 8 2022, 9:28 AM

another one

This revision was automatically updated to reflect the committed changes.