Extending `InferTypeOpInterface` to parsing a custom assembly format

Currently it looks like InferTypeOpInterface generates builder functions that only depend on operands. It would be nice if it could also be used to omit result types in an operation’s assembly format.

For example, in this CIRCT pr, I omitted the result type of the concat operation where the previous signature:

%0 = comb.concat %false, %b : (i1, i4) -> i5

was simplified to:

%0 = comb.concat %false, %b : i1, i4

I had to implement a custom parser and printer but it would be nicer to be able to just write:

let assemblyFormat = "$inputs attr-dict `:` type($inputs)";

I was thinking here might be a good place to check for the presence of InferTypeOpInterface and infer the result type. Does anyone have any thoughts on this?

4 Likes

Given that InferTypeOpInterface is supported in ODS, I’m fine with adding support for it in the assembly format generator. Would you like to prepare a patch for that? (If not feel free to file a bug/feature request and I can get to it in the next few days)

– River

Sure, I can give it a shot

+1 from me too, thanks

While this feature is great to have, I personally think ... : (i1, i4) -> i5 is a better choice here. Consider (i16, i64) - you’d have to add in your mind “locally” figure out the result type when reading IR. So I’m not sure you achieve simplification at all by omitting a few characters at the end; being explicit may be better. And it’s really not adding ambiguity here as to whether the result type could have been something different. In either case, you’ll have a verifier to check the sum matches.

Only in cases like SameOperandsAndResultType and perhaps some more, it’s obvious that omitting the result type is desirable and the assembly format already supports that.

1 Like

Of course, I think the author/designer of op should be allowed to include the result type if they want it. I think it is very useful for ODS to allow omitting it if they don’t.

-Chris

+1 on adding this support to ODS. I ran into this before.

Here’s my revision:
https://reviews.llvm.org/D111326