Page MenuHomePhabricator

[mlir] Support for mutable types
ClosedPublic

Authored by ftynse on Jul 20 2020, 7:16 AM.

Details

Summary

Introduce support for mutable storage in the StorageUniquer infrastructure.
This makes MLIR have key-value storage instead of just uniqued key storage. A
storage instance now contains a unique immutable key and a mutable value, both
stored in the arena allocator that belongs to the context. This is a
preconditio for supporting recursive types that require delayed initialization,
in particular LLVM structure types. The functionality is exercised in the test
pass with trivial self-recursive type. So far, recursive types can only be
printed in parsed in a closed type system. Removing this restriction is left
for future work.

Diff Detail

Event Timeline

ftynse created this revision.Jul 20 2020, 7:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 20 2020, 7:16 AM
rriddle accepted this revision.Jul 21 2020, 2:51 PM

Thanks Alex, I think this is an elegant solution.

Could you update https://mlir.llvm.org/docs/Tutorials/DefiningAttributesAndTypes/? It's gotten quite crusty and needs a good cleanup, so I can add docs when I clean it up if you don't want to do it now. This also might be something that we could include in the Toy dialect.

Just one conceptual question, what are the intended use cases for a failed mutation? In the current use cases we have, the type would realistically assert that the body was either not set or identical to the newly provided body.
For the cases where you envision this being used, what would a type do with the result of mutate?

Adding approval here, because the result of the above discussion doesn't change much of anything user facing.

mlir/include/mlir/IR/TypeSupport.h
136

Can you add the same to the AttributeUniquer for uniformity?

mlir/test/lib/IR/TestTypes.cpp
28

nit: Can you move the body of this function out to a new function?

This revision is now accepted and ready to land.Jul 21 2020, 2:51 PM
ftynse updated this revision to Diff 279763.Jul 22 2020, 4:05 AM
ftynse marked 2 inline comments as done.

Address review

Thanks Alex, I think this is an elegant solution.

Could you update https://mlir.llvm.org/docs/Tutorials/DefiningAttributesAndTypes/? It's gotten quite crusty and needs a good cleanup, so I can add docs when I clean it up if you don't want to do it now. This also might be something that we could include in the Toy dialect.

Sure, I was waiting to make sure we agree on the approach before updating that. I added a description of types with mutable components, but haven't touched the rest. I want to make sure LLVM dialect type rework is on tracks first, and then can look into updating the rest if you don't beat me to it.

Just one conceptual question, what are the intended use cases for a failed mutation? In the current use cases we have, the type would realistically assert that the body was either not set or identical to the newly provided body.
For the cases where you envision this being used, what would a type do with the result of mutate?

One thing is to have a graceful failure mechanism similar to getChecked. A bit stretched example: imagine creating two types with the same name by parsing them from strings (we intend to expose this through C API), it's better to be able to return null to the caller instead of asserting inside the library. One could argue that it's a wrong use of the library, but it can be triggered by user input so having an option not to assert sounds better.

Another thing is partly provisioning for the type auto-renaming scheme along the lines of

Type getTypeWithRenaming(StringRef name, ...) {
  Type t;
  do {
    t = RecursiveType::get(name);
    if (succeeded(t.setBody(...)) break;
    name = nextPossibleName(name);
  } while (true);
  return t ;
}
This revision was automatically updated to reflect the committed changes.