[Mlir-commits] [mlir] [mlir] Add struct parsing and printing utilities (PR #133939)

[River Riddle]

River707 wrote:

> > Instead of adding more c++ code, did you consider just expanding what the struct directive in tablegen can support? Why not extend that to support using a custom directive for the individual fields? This is generally what we have done when the declarative form is lacking something, try and extend that first.
> 
> Yes, I briefly thought about that and tried something like `struct(field1, custom<Field2>(field2))`, but I ran into the current assumption that these directives have top-level context and it felt like a larger change to reconsider that as this potentially opens up a bunch of nested variations of directives that need to be supported or properly excluded:
> 
> ```
> custom<Field1>(field1, custom<Field2>(field2))
> struct(field1, struct(field2, field3), field4) -> probably doesn't make sense, or maybe it does as a nested dictionary or so?
> custom<Field1>(field1, struct(field2, field3)) -> probably doesn't make sense?
> ...
> ```
> 
> The assumption that this is a larger change might be misguided though as I am not too familiar with tablegen (yet).
> 
> Additionally, I didn't see these two approaches as mutually exclusive as some fully custom printer/parser that can't be represented with the above directives might want to reuse these utilities as well for a part of it.
> 
> Anyway, if the declarative form is the way to go or if you want both implementations, I am happy to implement the nested directives as well.

The general thing for the assembly formats is that we should always to try to push for and evolve the tablegen side first, you should only default back to C++ when you really have to. Based on my observation of this MR, I would have expected we could just extend struct directive to support the very limited additional case of: a custom directive with a single parameter (aside from `ref`, though `ref` is not strictly necessary to support). You can then infer the name from the single parameter, and allow for custom parsing of that parameter. This would correspond to something like:

```
struct($field1, custom<Field2>($field2))
```

For a few of the cases in the StableHlo case, you could do something like:

```
struct(
    custom<Dims>($updateWindowDims),
    custom<Dims>($insertedWindowDims),
    ...,
    $indexVectorDim
)
```
Then in your code, you only have one custom directive (for dim parsing/printing), and your done (your attr parser/printer are now declarative). That feels much better, and for your case would remove a lot of the C++ (in your case, there are a few ops which would be simplified just from that).

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