This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Add support for VariadicOfVariadic operands
ClosedPublic

Authored by rriddle on Aug 9 2021, 11:12 AM.

Details

Summary

This revision adds native ODS support for VariadicOfVariadic operand
groups. An example of this is the SwitchOp, which has a variadic number
of nested operand ranges for each of the case statements, where the
number of case statements is variadic. Builtin ODS support allows for
generating proper accessors for the nested operand ranges, builder
support, and declarative format support. VariadicOfVariadic operands
are supported by providing a segment attribute to use to store the
operand groups, mapping similarly to the AttrSizedOperand trait
(but with a user defined attribute name).

build methods for VariadicOfVariadic operand expect inputs of the
form ArrayRef<ValueRange>. Accessors for the variadic ranges
return a new OperandRangeRange type, which represents a
contiguous range of OperandRange. In the declarative assembly
format, VariadicOfVariadic operands and types are by default
formatted as a comma delimited list of value lists:
(<value>, <value>), (), (<value>).

Diff Detail

Event Timeline

rriddle created this revision.Aug 9 2021, 11:12 AM
rriddle requested review of this revision.Aug 9 2021, 11:12 AM

I haven't completely cleaned everything up, but should be good for review+thoughts.

rriddle updated this revision to Diff 365325.Aug 9 2021, 5:46 PM

Refactor to use new *RangeRange types, infer the segment attribute in more cases, add builtin support for the declarative format.

rriddle edited the summary of this revision. (Show Details)Aug 9 2021, 5:46 PM

Nice!

mlir/include/mlir/IR/OperationSupport.h
734

OOC why not restrict here too to I32ElementsAttr?

767

ranges of types? (some nesting feels missing here(

769

getType feels weird to ask of a range of range of operands. Feels like result should be something akin to a tuple<tuple<type, ...>> What is the use for it?

806–808

Unintended change?

mlir/test/mlir-tblgen/op-format.mlir
155

Could you have 2 elements in this one ? (just to cover showing 0, 1, >1 operands in grouping(

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
867

Why does this one return SmallVector rather RangeRange?

928

Could you expand on what splitting the range entails?

rriddle updated this revision to Diff 365636.Aug 10 2021, 6:02 PM
rriddle marked 7 inline comments as done.

address comments

mlir/include/mlir/IR/OperationSupport.h
769

Several auto-generated things hardcode getType, which makes it weird for ranges. The getType allows for reusing the same code that is used for Value/Attribute/etc.

806–808

No, slice is called from a const instance in the other class so I just added const here (given that this method is const anyways).

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
867

This code is generated for the adaptor class, and I didn't add a ValueRangeRange(no real need for it).

jpienaar accepted this revision.Aug 11 2021, 10:35 AM

Thanks!

This revision is now accepted and ready to land.Aug 11 2021, 10:35 AM
This revision was automatically updated to reflect the committed changes.