This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Refactor alias generation to support nested aliases
ClosedPublic

Authored by rriddle on Oct 22 2022, 2:15 PM.

Details

Summary

We currently only support one level of aliases, which isn't great
in situations where an attribute/type can have multiple duplicated
components nested within it(e.g. debuginfo metadata). This commit
refactors alias generation to support nested aliases, which requires
changing alias grouping to take into account the depth of child
aliases, to ensure that attributes/types aren't printed before the
aliases they use.

The only real user facing change here was that we no longer print
0 as an alias suffix, which would be unnecessarily expensive to keep
in the new alias generation method (and isn't that valuable of a
behavior to preserve).

Diff Detail

Event Timeline

rriddle created this revision.Oct 22 2022, 2:15 PM
rriddle requested review of this revision.Oct 22 2022, 2:15 PM
rriddle updated this revision to Diff 469944.Oct 22 2022, 4:29 PM
rriddle edited the summary of this revision. (Show Details)
Mogball accepted this revision.Oct 22 2022, 11:54 PM

The zero index always not being printed hurts my OCD a little but I won't cry over it.

Also, jog my memory if I'm wrong but wasn't there something related to recursive LLVM structs and aliases? Or is that being handled by identified structs.

mlir/lib/IR/AsmPrinter.cpp
831

*cough* types and attributes are the same *cough* /s

This revision is now accepted and ready to land.Oct 22 2022, 11:54 PM

The zero index always not being printed hurts my OCD a little but I won't cry over it.

Also, jog my memory if I'm wrong but wasn't there something related to recursive LLVM structs and aliases? Or is that being handled by identified structs.

Right now if you have a recursive attr/type you have to handle the recursiveness yourself (otherwise the printer will implode). Just to be extra sure though, I'll add a safetly check for mutable things to disable nested aliases for now. To properly support these things, we'll have to change the printer API to enable only printing the immutable bits in certain situations (I might just do that in a follow up to finally finish that whole thing off).

mlir/lib/IR/AsmPrinter.cpp
831

Hush you!

This revision was landed with ongoing or failed builds.Oct 24 2022, 12:00 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 24 2022, 12:00 AM