This is an archive of the discontinued LLVM Phabricator instance.

[mlir][linalg] Support parsing attributes in named op spec
ClosedPublic

Authored by antiagainst on Jan 7 2021, 9:04 AM.

Details

Summary

With this, now we can specify a list of attributes on named ops
generated from the spec. The format is defined as

attr-id ::= bare-id (`?`)?
attr-typedef ::= type (`[` `]`)?
attr-def ::= attr-id `:` attr-typedef

tc-attr-def ::= `attr` `(` attr-def-list `)`
tc-def ::= `def` bare-id
  `(`tensor-def-list`)` `->` `(` tensor-def-list`)`
  (tc-attr-def)?

For example,

ods_def<SomeCppOp>
def some_op(...) -> (...)
attr(
  f32_attr: f32,
  i32_attr: i32,
  array_attr : f32[],
  optional_attr? : f32
)

where ? means optional attribute and [] means array type.

Diff Detail

Event Timeline

antiagainst created this revision.Jan 7 2021, 9:04 AM
antiagainst requested review of this revision.Jan 7 2021, 9:04 AM

Not specific to this revision, but is there a documentation for the .tc? (I don't see docs being updated here)

Not specific to this revision, but is there a documentation for the .tc? (I don't see docs being updated here)

Good question. The closet I can find is https://mlir.llvm.org/docs/Dialects/Linalg/#named-payload-ops-specification, which talks about it but doesn't use explicit grammars there. I need more patches to fully utilize these attributes, I'll make sure update the docs afterwards. :)

Add support for vector attributes

Minor fixes

Minor fixes

Harbormaster completed remote builds in B84379: Diff 315226.
hanchung accepted this revision.Jan 8 2021, 8:12 AM
hanchung added inline comments.
mlir/test/mlir-linalg-ods-gen/test-linalg-ods-gen.tc
84

Let's follow other test case -- add // for this blank line.

mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-gen.cpp
30–33

nit: I think we would add a blank line between LLVM project/subproject headers and System #includes.

164

Remove one space after the first sentence.

This revision is now accepted and ready to land.Jan 8 2021, 8:12 AM
nicolasvasilache accepted this revision.Jan 9 2021, 8:42 AM
nicolasvasilache added inline comments.
mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-gen.cpp
31

if you use StringMap you can drop this include.

1069

why not use StringMap like 2 lines above ?

antiagainst marked 5 inline comments as done.Jan 11 2021, 6:05 AM
antiagainst added inline comments.
mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-gen.cpp
31

StringMap iteration is not guaranteed to be deterministic. I need to print the whole map as ODS attributes so that can cause a problem for tests.

1069

Explained before. StringMap does not provide deterministic iteration.

This revision was automatically updated to reflect the committed changes.
antiagainst marked 2 inline comments as done.

Sorry I had to revert in 110775809ad1 as it broke the gcc-5 build (which has been broken for hours by another change that just got reverted, so I didn't want to lose more coverage right now and need to get it back green ASAP).

Sorry I had to revert in 110775809ad1 as it broke the gcc-5 build (which has been broken for hours by another change that just got reverted, so I didn't want to lose more coverage right now and need to get it back green ASAP).

No problem! Thanks for the explanation!