Operations that have the InferTypeOpInterface trait can now omit the return
types in their custom assembly formats.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/tools/mlir-tblgen/OpFormatGen.cpp | ||
---|---|---|
1560 | 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? |
mlir/test/lib/Dialect/Test/TestOps.td | ||
---|---|---|
2028–2041 | Can you simplify this to just infer a constant type? That would remove the need to add inputs. | |
mlir/tools/mlir-tblgen/OpFormatGen.cpp | ||
693 | ||
1560 | nit: Drop the trivial braces here. | |
2728 | ||
2737 | 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? |
Simplified FormatInferTypeOp to remove inputs, which led to a fix for handling
zero operand operations.
Addressed other pr comments.
mlir/tools/mlir-tblgen/OpFormatGen.cpp | ||
---|---|---|
2737 | 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. |
mlir/tools/mlir-tblgen/OpFormatGen.cpp | ||
---|---|---|
2737 | 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. |
mlir/tools/mlir-tblgen/OpFormatGen.cpp | ||
---|---|---|
2737 | Ah, I think I understand now. I believe the latest update addresses this. |
Can you simplify this to just infer a constant type? That would remove the need to add inputs.