[Mlir-commits] [mlir] [mlir] Add support for recursive elements in DICompositeTypeAttr. (PR #74948)

[Tobias Gysi]

gysit wrote:

Thanks for tackling this problem!

I have two initial comments:

1) This problem looks like an interesting use case for https://github.com/llvm/llvm-project/pull/66663. With that PR, it may be possible to solve the printing and parsing without writing a lot of code by hand, which we should avoid if possible. The idea of the pr is to support attribute alias printing in the presence of cyclic dependencies. 

2) I think it may be worth considering `DistinctAttr` here instead of a string identifier (https://reviews.llvm.org/D153360). The distinct attribute was developed to model distinct metadata nodes in MLIR. You can create distinct attributes in parallel code and they are then numbered in a deterministic way during printing and parsing similar to SSA values. It makes especially sense since AFAIK the identifier of a DICompositeType is not necessarily sufficient for uniquing. There can be multiple DICompositeType metadata nodes with the same identifier (the definition and possibly multiple forward declarations). At least this is my interpretation of the language reference (https://llvm.org/docs/LangRef.html#dicompositetype). Also is there a guarantee that a recursive DICompositeType has an identifier?

I am not sure how easy it is to land 1) but it may be worth considering to revive the discussion (either on the PR or maybe better on discourse (https://discourse.llvm.org/t/rfc-supporting-aliases-in-cyclic-types-and-attributes/73236).

https://github.com/llvm/llvm-project/pull/74948