[Mlir-commits] [mlir] add IntegerLikeTypeInterface to enable out-of-tree uses of built-in attributes that otherwise force integer types (PR #137061)

[Jeremy Kun]

j2kun wrote:

> But consumers of the IntegerAttr expect right now that it only encodes an integer: they will consume it as such and manipulate it as such (in folder code for example): I don't quite see how making it extensible would work here?

I think the nuance here is that IntegerAttr stores an integer (which it still will in this PR), but it already allows more than one `Type` to describe that integer, [namely `IndexType` and `IntegerType`](https://github.com/llvm/llvm-project/blob/d7f096e3fe611ae2cc7403c3cf2f88255a47b61d/mlir/include/mlir/IR/BuiltinAttributes.td#L684, which forces users to already handle it in enough generality for this PR to not break anything. Some evidence:

- The vast majority of upstream users of IntegerAttr simply ignore the type and use the underlying int (I have always wondered why IntegerAttr is typed, and I suspect it's only for the parser/printer which doesn't know the bit width for parsing or signedness for printing).
- Places like ArithCanonicalization [support all types generically](https://github.com/llvm/llvm-project/blob/bad8bf56d3e4f107423b307f5f75564296703a76/mlir/lib/Dialect/Arith/IR/ArithOps.cpp#L43-L50) and only manipulate the underlying APInt. This would be invalid for a new type, but Arith is not possible to use with out-of-tree types.
- Common folders like `constFoldBinaryOp` are agnostic of the type and attribute entirely. They do nothing with it except pass it along to the result attribute of the fold. Users of these [folders in Arith](https://github.com/llvm/llvm-project/blob/d7f096e3fe611ae2cc7403c3cf2f88255a47b61d/mlir/lib/Dialect/Arith/IR/ArithOps.cpp#L316-L319) (which again cannot be used with out-of-tree types anyway) only require IntegerAttr stores an APInt. The data the type provides in these cases is not used.
- Documentation demonstrates that [type checking should be done on IntegerAttr](https://github.com/llvm/llvm-project/blob/d7f096e3fe611ae2cc7403c3cf2f88255a47b61d/mlir/docs/DefiningDialects/Operations.md?plain=1#L1426)
- Because of the joint index and integer type support, many places in the codebase [already have fallbacks](https://github.com/llvm/llvm-project/blob/d7f096e3fe611ae2cc7403c3cf2f88255a47b61d/mlir/lib/Dialect/Arith/Utils/Utils.cpp#L130-L133) and assertions to guard against an unexpected type.

> > Would it make sense to have an IntLikeElementsAttr upstream that supports more type compatibility but also uses integer storage?
> 
> Would that be an interface or a concrete attribute? If a concrete one, would it replace (or compete with) DenseIntOrFPElementsAttr for integers? 

I am happy to take advice here. One of the difficulties of having a new attribute (interface or concrete) is that the `Parser::parseAttribute` accepts only a type as input, and infers from that the attribute to construct. I can avoid this by having a concrete `IntLikeElementsAttr` which defines its own custom parser/printer, though that may require duplicating some of the nice aspects of the builtin attributes (like the conveniences around splats). If it were an interface and the concrete attribute were out-of-tree, I'd have to think about how that would work (how would the interface manage something like folding to a splat if it doesn't manage the storage of the concrete attribute?).

> At this point I lost track if it's possible to allow other "int types" for DenseIntOrFPElementsAttr instead?

I believe this is what this PR demonstrates, at least because it makes our out-of-tree `mod_arith` dialect https://github.com/google/heir/pull/1758/ work with `dense<...>`.

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