[Mlir-commits] [mlir] [mlir] Add the concept of ASM dialect aliases (PR #86033)

[Markus Böck]

https://github.com/zero9178 commented:

You're both correct. My concern is with the usability and the understandability of the IR and I think having it be supported in ODS to look as much as normal types and attributes with all the supporting infrastructure would be a big help.
That said, I don't want to demand ODS support in this PR but rather having it as a future goal. 

> > a way to assign identifiers to types and attributes for syntax sugar
> This is what we want. Because !llvm.ptr<3> is just syntax sugar for !ptr.ptr<#llvm.address_space<3>>. When interacting > with !llvm.ptr<3> one is still using !ptr.ptr methods.

The current aliasing infrastructure supported by `getAlias` does not change the syntax of attributes and types, but just their uses so to say, and is fully transparent to any dialect. As you noticed when using `dump`, if these concepts are conflated then it becomes more difficult to change the behaviour of either "dialect aliases" or "attribute/type aliases" without affecting the other. I don't really see a lot of overlap between the two besides both being syntax sugar and having "alias" in their name.

What do you think about having an API like `Dialect::addDialectAlias<Ptr::PointerType, LLVM::LLVMPointerType>()` function, where `LLVM::LLVMPointerType` optionally subclasses `Ptr::PointerType` and implements `classof`, `print` and `parse`?

The `classof` is used so that `isa` calls work and after registration can also be used by the printer to check whether a `Ptr::PointerType` is actually a `LLVM::LLVMPointerType` that should be printed as such. Whether and how much this changes core, the context etc I see more as an implementation detail at this point of the design stage and not a constraint (after all we all love changing core :slightly_smiling_face:) and something that automatically requires larger discussions compared to the current stage. As mehdi said, the `LLVM::LLVMPointerType` could then be ODS generated in the future. By requiring and using `print` and `parse` methods it'd also work with existing MLIR features such as unqualified printing, something that I don't think works in the current implementation?

If you think about it, the concept of a class acting as the dialect alias is not a new concept in MLIR core either as the builtin dialect does the same with `BoolAttr`, `FlatSymbolRefAttr` etc. Its only really the custom printing support that is missing. This could allow having `addDialectAlias<IntegerAttr, BoolAttr>` with `BoolAttr` being ODS generated in the future :slightly_smiling_face:

If that doesn't sound good to you I'd already be happy with just not using `getAlias` to implement this. I admittedly don't care too much about the performance of printing and parsing (as long as its not horrible), and mostly about the usability and understandability of the APIs and IR. Making concepts first class helps in this regard IMO.


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