iree icon indicating copy to clipboard operation
iree copied to clipboard

Drop `EncodingRole` enum, replace by operand index

Open bjacob opened this issue 1 year ago • 1 comments

Currently, one of the fields in the encoding attribute on a tensor is role, https://github.com/iree-org/iree/blob/58feff319e2fd0dff7909358741a26ffa5807823/compiler/src/iree/compiler/Dialect/Encoding/IR/EncodingBase.td#L79

Which is an enum defined in the same file, https://github.com/iree-org/iree/blob/58feff319e2fd0dff7909358741a26ffa5807823/compiler/src/iree/compiler/Dialect/Encoding/IR/EncodingBase.td#L51-L56

These values LHS, RHS, etc, don't scale very well to other ops we want to support data-tiling for, such as convolutions, for which we rather talk of input and filter and would then want to add new enumeration values for that.

Instead, this role attribute should just be replaced by the operand index, just an integer (index) attribute. It could be renamed operand_index.

For a matmul: LHS = operand index 0, RHS = operand index 1, RESULT (by which we really meant the accumulator) = operand index 2.

Question - do we want to separate the inputs (ins) from the Inits (outs)? I don't think that's necessary. Just we will have to get used to the result value being encoded as the corresponding init-value (outs[0] => operand index 2 since the outs come after the ins).

bjacob avatar Jun 07 '24 15:06 bjacob

implemented in https://github.com/iree-org/iree/pull/17708

lialan avatar Jun 24 '24 13:06 lialan