Page MenuHomePhabricator

[mlir][ods] Unify Attr/TypeDef and Operation Format Parsing
ClosedPublic

Authored by Mogball on Jan 22 2022, 3:01 PM.

Details

Summary

Part 2 of 3 of unifying the assembly formats of attributes/types and operations.The last patch that introduced attribute/type formats (D111594) factored out the format lexer entirely. This patch factors out most of the format parsers such that the attribute/type and op parsers only need to implement handling for specific elements.

Certain things could be factored better (element verification, 'seen' variables) but the primary goal of factoring is so that features can be used across both assembly formats.

Diff Detail

Event Timeline

Mogball created this revision.Jan 22 2022, 3:01 PM
Mogball requested review of this revision.Jan 22 2022, 3:01 PM
Mogball retitled this revision from [mlir][ods] Unify Attr/TypeDef and Operation Format Parsing Part 2 of 3 of unifying the assembly formats of attributes/types and operations. The last patch that introduced attribute/type formats (D111594) factored out the format lexer entirely. to [mlir][ods] Unify Attr/TypeDef and Operation Format Parsing.Jan 22 2022, 3:02 PM
Mogball edited the summary of this revision. (Show Details)
Mogball added reviewers: rriddle, mravishankar.
Mogball updated this revision to Diff 402771.Jan 24 2022, 11:46 PM

fix MSVC build

Letting you know I haven't missed this, will get to it in the next day or two.

rriddle accepted this revision.Jan 31 2022, 12:53 AM

All of the llvm::SMLoc can be SMLoc now, but otherwise LGTM.

mlir/tools/mlir-tblgen/OpFormatGen.cpp
66

Why do these need llvm:: now?

2170

Can we drop the mlir:: on all of these?

This revision is now accepted and ready to land.Jan 31 2022, 12:53 AM
Mogball marked 2 inline comments as done.Jan 31 2022, 3:57 PM
Mogball added inline comments.
mlir/tools/mlir-tblgen/OpFormatGen.cpp
66

Conflicting with VariableKind::Optional

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

review comments

Mogball updated this revision to Diff 404822.Jan 31 2022, 11:19 PM

review comments

This revision was landed with ongoing or failed builds.Jan 31 2022, 11:28 PM
This revision was automatically updated to reflect the committed changes.

This seems to be hitting ASAN error:

Direct leak of 128 byte(s) in 8 object(s) allocated from:
...
#6 0x569acb in mlir::tblgen::FormatParser::parseCustomDirective(llvm::SMLoc, mlir::tblgen::FormatParser::Context) Compiles/llvm/mlir/tools/mlir-tblgen/FormatGen.cpp:360:15