[mlir] Add concept of alias blocks #65503
Conversation
This PR is part of https://discourse.llvm.org/t/rfc-supporting-aliases-in-cyclic-types-and-attributes/73236 It implements the concept of "alias blocks", a block of alias definitions which may alias any other alias definitions within the block, regardless of definition order. This is purely a convenience for immutable attributes and types, but is a requirement for supporting aliasing definitions in cyclic mutable attributes and types. The implementation works by first parsing an alias-block, which is simply subsequent alias definitions, in a syntax-only mode. This syntax-only mode only checks for syntactic validity of the parsed attribute or type but does not verify any parsed data. This allows us to essentially skip over alias definitions for the purpose of first collecting them and associating every alias definition with its source region. In a second pass, we can start parsing the attributes and types while at the same time attempting to resolve any unknown alias references with our list of yet-to-be-parsed attributes and types, parsing them on demand if required. A later PR will hook up this mechanism to the `tryStartCyclicParse` method added in llvm@b121c26 to early register cyclic attributes and types, breaking the parsing cycles.
zero9178
force-pushed
the
lazy-print-parse
branch
from
5a84e49 to
055e44d
Compare
|
Friendly ping |
|
|
||
| ```mlir | ||
| // Alias block consisting of #array, !integer_type and #integer_attr. | ||
| #array = [#integer_attr, !integer_type] |
There was a problem hiding this comment.
If understand this correctly, it's basically a forward declaration?
There was a problem hiding this comment.
Not really. It's essentially a use-before-def. Aliases within a block are allowed to reference other aliases within the block prior to their definition.
So in this case #integer_attr and !integer_type are declared afterwards, not prior.
If it were a forward declaration then I'd think it'd look something like:
// "forward" declaration
#array = []
!integer_type = i32
#integer_attr = 8 : !integer_type
#array = [#integer_attr, !integer_type]this would in my opinion not be very intuative and for the larger issue I am trying to solve, that is using aliases with cyclic attributes and types, less ergonomic:
!type = !llvm.struct<"test">
!body = !llvm.ptr<!type>
!type = !llvm.struct<"test", (!body)>instead of just
!type = !llvm.struct<"test", !body>
!body = !llvm.ptr<!type>(llvm dialect only used for illustrative purposes)
This would also require an API/Interface for mutable attributes and types to return a "forward" declaration
| @@ -79,6 +82,9 @@ ParseResult Parser::parseFusedLocation(LocationAttr &loc) { | |||
| LocationAttr newLoc; | |||
| if (parseLocationInstance(newLoc)) | |||
| return failure(); | |||
| if (syntaxOnly()) | |||
There was a problem hiding this comment.
This seems very invasive. There is prior art to allow use-before-def, such as deferred aliases. Why won't that work for your use-case?
There was a problem hiding this comment.
I ruled out the deferred aliases approach as not implementable.
The way it works for locations is that a placeholder opaque location is created that is then later RAUW with the actual location at the end of the module.
The reason I don't think is implementable is because todays attributes and types would not be able to handle having some placeholder returned from parseType or parseAttribute as they tend to check that these are of specific kinds (e.g. IntegerAttr or MemRefElementTypeInterface or fullfill some other specific constraints.
See also https://discourse.llvm.org/t/rfc-supporting-aliases-in-cyclic-types-and-attributes/73236#return-placeholder-attributes-or-types-10
This is not an issue for locations as they are just attached to Operations and do not need to fullfill any specific constraints
I agree that this is invasive and I'd love to hear better options. I initially considered writing separate skipType and skipAttr functions, but there I'd essentially just end up reimplementing the whole parser for the builtin attributes, including error messages, which would lead to a lot of code duplications. Any other way of skipping that isn't based on actually parsing the syntactic elements I'd consider more of a "hack" that is prone to breaking.
The silver lining is that this logic and invasiveness is private to the parser implementation and not user exposed and only part of the builtin attribute and type parsing, not any dialect parsing.
|
Friendly ping |
1 similar comment
|
Friendly ping |
There was a problem hiding this comment.
As @Mogball pointed out, this is indeed quite invasive. Sadly, I do also not know of any other way of supporting this, especially considering the subsequent extension for cyclic structures.
Considering that we would require the support for cyclic structures to fully support LLVM debug metadata import, landing this stack of commits would be very welcome.
What is the stance regarding this change from the people of the core team?
| @@ -15,6 +15,7 @@ | |||
| #include "mlir/IR/BuiltinTypes.h" | |||
| #include "mlir/IR/Dialect.h" | |||
| #include "mlir/IR/DialectImplementation.h" | |||
| #include "llvm/ADT/ScopeExit.h" | |||
There was a problem hiding this comment.
Nit: This seems to be unused
This PR is part of https://discourse.llvm.org/t/rfc-supporting-aliases-in-cyclic-types-and-attributes/73236
It implements the concept of "alias blocks", a block of alias definitions which may alias any other alias definitions within the block, regardless of definition order. This is purely a convenience for immutable attributes and types, but is a requirement for supporting aliasing definitions in cyclic mutable attributes and types.
The implementation works by first parsing an alias-block, which is simply subsequent alias definitions, in a syntax-only mode. This syntax-only mode only checks for syntactic validity of the parsed attribute or type but does not verify any parsed data. This allows us to essentially skip over alias definitions for the purpose of first collecting them and associating every alias definition with its source region.
In a second pass, we can start parsing the attributes and types while at the same time attempting to resolve any unknown alias references with our list of yet-to-be-parsed attributes and types, parsing them on demand if required.
A later PR will hook up this mechanism to the
tryStartCyclicParsemethod added in b121c26 to early register cyclic attributes and types, breaking the parsing cycles.