Page MenuHomePhabricator

Implement IR versioning through post-parsing upgrade through OpAsmDialectInterface
Needs ReviewPublic

Authored by mehdi_amini on Jan 20 2022, 2:12 AM.

Details

Reviewers
rriddle
Summary

Dialect can opt-in to handle versioning in a custom way. The input IR may start
with the keyword dialect_versions, followed by a dictionary attribute where
the keys are dialect names and the value are attributes representing the
producer version for the given dialect. There is no restriction on what kind of
attribute a dialect is using to model its versioning.

On parsing the version is made available through the parser, and a dialect for
which a version is present has the ability to use the stored version to upgrade
the IR.

Diff Detail

Event Timeline

mehdi_amini created this revision.Jan 20 2022, 2:12 AM
mehdi_amini requested review of this revision.Jan 20 2022, 2:12 AM
mehdi_amini edited the summary of this revision. (Show Details)Jan 20 2022, 2:12 AM

One thing that this doesn't really cover right now is how the assembly syntax itself can differ between versions. Realistically, if we do allow versioning the IR I would also assume that we may also want to be able to tag versions of the IR syntax. You wouldn't want to ship all old versions of the IR syntax with your current dialect obviously, but we could conceptually have mechanisms of linking in extensions that support parsing old versions? The alternative there obviously is that we just say "you only get versioned output when using the generic form". I see all of that as separable though, upgrading an in-memory form of an operation is separable from parsing and upgrading an old syntax; though they could be hooked into via the same versioning mechanism I assume.

mlir/include/mlir/IR/OpImplementation.h
1358

I like using an attribute for this as it leaves it all up to the dialect to define. Do we impose any kind of constraints on this though? i.e., if these were dialect attributes, would they need to be upgraded as well (if the dialect changed)?

1359–1362

What are the constraints on what this is allowed to modify? Can it touch anything? I guess in some cases it kind of should be able to (e.g. if old argument attributes are now represented via some other contract, the dialect would want to be able to update the function).

mlir/lib/IR/AsmPrinter.cpp
2501

Not important for now, but it'd be nice to have some reserved syntax for directives (helps prevent confusing it for an operation or some other IR construct).

One thing that this doesn't really cover right now is how the assembly syntax itself can differ between versions

Oh right, there is something I haven't plumbed yet: exposing an accessor for the version through the AsmParser, so that custom parsing function can query it before parsing a given operation.

mlir/include/mlir/IR/OpImplementation.h
1358

I'd say it's up to the dialect to figure it out?

1359–1362

I didn't think about putting any constraint right now, I couldn't foresee a reason to limit this.

mlir/lib/IR/AsmPrinter.cpp
2501

Yeah I wrote this anticipating that you would complain about it, so that I could ask you for a suggestion ;)

rriddle added inline comments.Jan 20 2022, 10:22 PM
mlir/include/mlir/IR/OpImplementation.h
1358

I would be fine leaving it up to the dialect, but it still feels like we need to understand the implications here: just so that dialect authors don't feel fancy, only for it to break in some way down the line.

1359–1362

I'm fine with leaving it open, but it feels like we need some kind of order for upgrades, otherwise we might end up some weird situations with cross dialect things. For example, with the example case I picked above: what if the new way of representing a dialect attribute requires some new construct that is only available in a new version; what happens if the dialect attribute is upgraded first? will these things even compose(i.e. will we crash?) or will the upgrade for the function potentially destroy/overwrite that? We may need some other callback that acts on dialect attributes that gets called after an operation is upgraded? I don't entirely know, but feels like we should sketch out some potential cross-interaction cases.

mlir/lib/IR/AsmPrinter.cpp
2501

Dang. I naturally would just go with #directive, but unfortunately we use that for attributes... We could go with something similar to rust metadata: #![thing = ...]/#!thing = .... I'd say if we can reuse a scheme from an existing language, that makes sense (from a perspective of reusing existing conventions).

One thing that this doesn't really cover right now is how the assembly syntax itself can differ between versions

Oh right, there is something I haven't plumbed yet: exposing an accessor for the version through the AsmParser, so that custom parsing function can query it before parsing a given operation.

Nevermind, I already exposed it: virtual Attribute getDialectVersion(StringRef dialect) = 0; is available on the AsmParser, someone really motivated can use this in their custom parser!

mehdi_amini retitled this revision from WIP on a IR versionning through post-parsing upgrade through OpAsmDialectInterface to Implement IR versioning through post-parsing upgrade through OpAsmDialectInterface.
mehdi_amini edited the summary of this revision. (Show Details)

Rebase, document, and add a test.

mlir/include/mlir/IR/OpImplementation.h
1367–1375

This seems awkward to me, since it assumes that the syntax can always be parsed. Why not just rely on the parser to do this without a separate hook?

mlir/lib/IR/AsmPrinter.cpp
2501

+1 for some distinguished syntax here.

One thing that this doesn't really cover right now is how the assembly syntax itself can differ between versions. Realistically, if we do allow versioning the IR I would also assume that we may also want to be able to tag versions of the IR syntax. You wouldn't want to ship all old versions of the IR syntax with your current dialect obviously, but we could conceptually have mechanisms of linking in extensions that support parsing old versions? The alternative there obviously is that we just say "you only get versioned output when using the generic form". I see all of that as separable though, upgrading an in-memory form of an operation is separable from parsing and upgrading an old syntax; though they could be hooked into via the same versioning mechanism I assume.

I was thinking that there could be an an addition version like "assembly_version" or something. Could just be hard-coded in the source code and we bump it on breaking printer/parser changes? The naming of "dialect_versions" somewhat precludes putting such a thing in the dictionary, but maybe one special exception spelled to not conflict with a dialect name?

mlir/lib/Parser/Parser.cpp
2190

sp. "dictionary"

mehdi_amini added inline comments.Jan 21 2022, 9:14 PM
mlir/include/mlir/IR/OpImplementation.h
1367–1375

Because the main point is to support the generic IR format. Also, you can rename operation, or even delete an operation from a dialect, while being able to "upgrade it" to the new form.

jpienaar added inline comments.
mlir/lib/IR/AsmPrinter.cpp
2501

May I suggest https://downloads.haskell.org/~ghc/7.0.4/docs/html/users_guide/pragmas.html , it is "foreign" enough that it sticks out (and can't be confused with IR), these naturally feel like file scope pragmas, it is familiar to some (but not as general (i think :-)) as Rust), and i don't think any parsing ambiguity at file scope.