[Mlir-commits] [mlir] [mlir] Retain original identifier names for debugging (PR #79704)

[Perry Gibson]

Wheest wrote:

> For 2 have we considered enabling creating an AsmState from an AsmParserState?

I hadn't, but it looks like AsmState could be a good way to manage the name data.
Looking at the [description of AsmState](https://github.com/llvm/llvm-project/blob/26db845536aa4ada6231873a01252c75637fcbae/mlir/include/mlir/IR/AsmState.h#L39-L45):

> The following classes enable support for parsing and printing resources
> within MLIR assembly formats. Resources are a mechanism by which dialects,
> and external clients, may attach additional information when parsing or
> printing IR without that information being encoded in the IR itself.
> Resources are not uniqued within the MLIR context, are not attached directly
> to any operation, and are solely intended to live and be processed outside
> of the immediate IR.

That looks like a good way to store this data, rather than letting mlir-opt handle it as I suggested.

>  It may require allowing mutations of AsmState ... so potentially a separate mapping may be better.

There is a member variable for `AsmResourceBlob`, `dataIsMutable`, which could mean this is already possible.  But I'm not that familiar with how `AsmState` behaves, so perhaps that's not possible.

Regarding making an `AsmState` from `AsmParserState`, I think it would be simpler to just create an `AsmState` from a `DenseMap<Value, StringRef>` (though I think due to how `AsmResourceBlob` works, it would need to be serialized).

We _could_ have something akin to `DenseMap<Value, AsmParserState>`, but I'm not sure what benefit that would bring.  We could recover the `StringRef` for the identifiers by looking at the location information in `SMDefinition`.  Perhaps the extra data would make it more extenisble?  But this extra processing of `AsmParserState` at print time would add complexity that this feature doesn't really need.

Thoughts on this design?  I'll try making another version that has the external data structure, first managed by `mlir-opt`, and then stored as an `AsmState`.

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