This is an archive of the discontinued LLVM Phabricator instance.

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
1352

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)?

1353–1356

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
2505

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
1352

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

1353–1356

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

mlir/lib/IR/AsmPrinter.cpp
2505

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
1352

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.

1353–1356

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
2505

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
1361–1369

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
2505

+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
2159

sp. "dictionary"

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

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
2505

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.

jpienaar added inline comments.Jul 24 2022, 10:38 AM
mlir/docs/LangRef.md
836 ↗(On Diff #401883)

This should probably now be part of config section

mlir/include/mlir/IR/OpImplementation.h
1361

This covers the main use case but makes the current version of dialect implicit/the hook always gets fed latest. Is that sufficient? E.g., if one wanted to upgrade to an older version (say i want to use this as part of upgrade to certain version for deployment) it means you need to be synced to that version when building and so if you have multiple target versions, you'd need binary per?

mlir/lib/IR/AsmPrinter.cpp
2505

External resources uses this syntax, so if this is moved there then perfect

Herald added a project: Restricted Project. · View Herald TranscriptJul 24 2022, 10:38 AM
Herald added a subscriber: bzcheeseman. · View Herald Transcript
mehdi_amini added inline comments.Jul 24 2022, 12:17 PM
mlir/include/mlir/IR/OpImplementation.h
1361

Yes: this only offers the ability for the compiler to support the "latest version of the dialect" and upgrade older versions to the current one supported by the compiler.

It seems that what you're describing is a bit more involved: wouldn't that account to include the notion of versioning to the ODS definitions directly somehow?

jpienaar added inline comments.Jul 28 2022, 1:56 PM
mlir/include/mlir/IR/OpImplementation.h
1361

Would it? E.g., in the simple case I see this as a sequence of if statements in upgrade hook which incrementally updates. So initially no hooks , then at version Foo one needs to introduce upgrades, so upgrade from < Foo to Foo. Now at version Baz another bump is needed so add upgrade from Foo to Baz. Of course one could rewrite upgrade hooks for every version and so have more efficient ones and that means you only have head as target possible. These should mostly not be on the critical path though (if you are actually serving, run update offline and reserialize is option rather than adding any cost). But I don't want to restrict being able to do optimized hooks and for those one can't guarantee it'll be anything other than input or head.

mehdi_amini added inline comments.Jul 28 2022, 2:04 PM
mlir/include/mlir/IR/OpImplementation.h
1361

But regardless of the input, we still upgrade to the current latest version of the dialect right?

I guess I don't get what you meant with "one wanted to upgrade to an older version"?
I thought you meant I want my 4.x compiler to be able to load version 1.x and decide at runtime with a flag whether it'll upgrade to 3.x or 4.x : that would require to support two versions of the dialect in the current compiler.