Page MenuHomePhabricator

[mlir][ODS] Support result type inference in custom assembly format

Authored by trilorez on Oct 7 2021, 10:32 AM.



Operations that have the InferTypeOpInterface trait can now omit the return
types in their custom assembly formats.

Diff Detail

Event Timeline

trilorez created this revision.Oct 7 2021, 10:32 AM
trilorez requested review of this revision.Oct 7 2021, 10:32 AM
trilorez updated this revision to Diff 377912.EditedOct 7 2021, 10:50 AM

Ran clang-format, thought I had ran it before

Mogball added inline comments.

Can you move hoist this to the top as a const char *const and add a small snippet of documentation like the other code snippets?

trilorez updated this revision to Diff 377925.Oct 7 2021, 11:36 AM

Moved infer return type parser code snippet to a constant

rriddle added inline comments.Oct 7 2021, 12:35 PM

Can you simplify this to just infer a constant type? That would remove the need to add inputs.


nit: Drop the trivial braces here.


What about situations where only some types can be resolved this way? Should we just default to using inferResultTypes if the user doesn't specify any results in the format?

lattner added a subscriber: lattner.Oct 7 2021, 2:17 PM

This looks great @trilorez ! Thank you for working on this

trilorez updated this revision to Diff 378032.Oct 7 2021, 3:41 PM

Simplified FormatInferTypeOp to remove inputs, which led to a fix for handling
zero operand operations.

Addressed other pr comments.

trilorez added inline comments.Oct 7 2021, 3:46 PM

The resolvers for result types can only be set by AllTypesMatch, SameOperandsAndResultType, and TypesMatchWith, which look like they can't have partial type inference.

For other cases, like one of several result types being manually specified, I'm not sure how partial type inference would work. I suppose already inferred types could be passed to inferReturnTypes along with the operands, but then they'd need to either be in order or pass along their index.

trilorez updated this revision to Diff 378698.Oct 11 2021, 9:26 AM

Added InferTypeOpInterface section to

rriddle added inline comments.Oct 11 2021, 10:51 AM

I was more saying that if the user didn't explicitly add a type to the format, we could just always use inferResultTypes(for all of the types, ignoring the other resolvers). I think it would be problematic if the user had one result that was a constant(e.g. Index or something), but had others that required being referred. Right now we would never give them a chance to infer the other result types.

trilorez updated this revision to Diff 378749.Oct 11 2021, 11:50 AM

Prefer inferring result types over using buildable type builders

trilorez updated this revision to Diff 378752.Oct 11 2021, 11:59 AM

Infer all result types if none are specified

trilorez added inline comments.Oct 11 2021, 12:00 PM

Ah, I think I understand now. I believe the latest update addresses this.

rriddle accepted this revision.Oct 11 2021, 12:43 PM

LG, thanks.

This revision is now accepted and ready to land.Oct 11 2021, 12:43 PM