Skip to content

Commit 0401668

Browse files
authored
[mlir] Fix TileUsingForOp attr-dict printing/parsing (#72745)
`TileUsingForOp` has an optional Attribute `interchange` which was given in curly braces like this: `{interchange = [...]}`. The way this was parsed meant that no normal `attr-dict` could be attached to the Op. This patch adds printing / parsing of an `attr-dict` to the Op and treats the `interchange` Attribute as part of that dictionary for now.
1 parent 575c9bf commit 0401668

File tree

3 files changed

+17
-32
lines changed

3 files changed

+17
-32
lines changed

mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1819,7 +1819,7 @@ def TileUsingForOp : Op<Transform_Dialect, "structured.tile_using_for",
18191819
let arguments = (ins TransformHandleTypeInterface:$target,
18201820
Variadic<TransformParamTypeOrAnyHandle>:$dynamic_sizes,
18211821
DefaultValuedOptionalAttr<DenseI64ArrayAttr, "{}">:$static_sizes,
1822-
DefaultValuedOptionalAttr<DenseI64ArrayAttr, "{}">:$interchange,
1822+
DefaultValuedOptionalAttr<I64ArrayAttr, "{}">:$interchange,
18231823
DefaultValuedOptionalAttr<DenseBoolArrayAttr, "{}">:$scalable_sizes);
18241824
let results = (outs TransformHandleTypeInterface:$tiled_linalg_op,
18251825
Variadic<TransformHandleTypeInterface>:$loops);

mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp

Lines changed: 8 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2477,7 +2477,7 @@ void transform::TileUsingForOp::build(
24772477
/*target=*/target,
24782478
/*dynamic_sizes=*/dynamicTileSizes,
24792479
/*static_sizes=*/staticTileSizesAttr,
2480-
/*interchange=*/builder.getDenseI64ArrayAttr(interchange),
2480+
/*interchange=*/builder.getI64ArrayAttr(interchange),
24812481
/*scalable_sizes=*/expandedScalableSizes);
24822482
}
24832483

@@ -2611,7 +2611,8 @@ transform::TileUsingForOp::apply(transform::TransformRewriter &rewriter,
26112611
});
26122612
}
26132613

2614-
tilingOptions.setInterchange(getInterchange());
2614+
tilingOptions.setInterchange(
2615+
extractFromIntegerArrayAttr<int64_t>(getInterchange()));
26152616
FailureOr<scf::SCFTilingResult> maybeTilingResult =
26162617
tileUsingSCFForOp(rewriter, tilingInterface, tilingOptions);
26172618
if (failed(maybeTilingResult))
@@ -2648,33 +2649,6 @@ SmallVector<OpFoldResult> transform::TileUsingForOp::getMixedSizes() {
26482649
return results;
26492650
}
26502651

2651-
// We want to parse `DenseI64ArrayAttr` using the short form without the
2652-
// `array` prefix to be consistent in the IR with `parseDynamicIndexList`.
2653-
ParseResult parseOptionalInterchange(OpAsmParser &parser,
2654-
OperationState &result) {
2655-
if (succeeded(parser.parseOptionalLBrace())) {
2656-
if (failed(parser.parseKeyword("interchange")))
2657-
return parser.emitError(parser.getNameLoc()) << "expect `interchange`";
2658-
if (failed(parser.parseEqual()))
2659-
return parser.emitError(parser.getNameLoc()) << "expect `=`";
2660-
result.addAttribute("interchange",
2661-
DenseI64ArrayAttr::parse(parser, Type{}));
2662-
if (failed(parser.parseRBrace()))
2663-
return parser.emitError(parser.getNameLoc()) << "expect `}`";
2664-
}
2665-
return success();
2666-
}
2667-
2668-
void printOptionalInterchange(OpAsmPrinter &p,
2669-
ArrayRef<int64_t> interchangeVals) {
2670-
if (!interchangeVals.empty()) {
2671-
p << " {interchange = [";
2672-
llvm::interleaveComma(interchangeVals, p,
2673-
[&](int64_t integer) { p << integer; });
2674-
p << "]}";
2675-
}
2676-
}
2677-
26782652
ParseResult transform::TileUsingForOp::parse(OpAsmParser &parser,
26792653
OperationState &result) {
26802654
OpAsmParser::UnresolvedOperand target;
@@ -2686,7 +2660,7 @@ ParseResult transform::TileUsingForOp::parse(OpAsmParser &parser,
26862660

26872661
if (parser.parseOperand(target) || parser.getCurrentLocation(&operandLoc) ||
26882662
parseDynamicIndexList(parser, dynamicSizes, staticSizes, scalableVals) ||
2689-
parseOptionalInterchange(parser, result) ||
2663+
parser.parseOptionalAttrDict(result.attributes) ||
26902664
parser.parseColonType(functionalType))
26912665
return ParseResult::failure();
26922666

@@ -2720,7 +2694,10 @@ void TileUsingForOp::print(OpAsmPrinter &p) {
27202694
printDynamicIndexList(p, getOperation(), getDynamicSizes(), getStaticSizes(),
27212695
/*valueTypes=*/{}, getScalableSizesAttr(),
27222696
OpAsmParser::Delimiter::Square);
2723-
printOptionalInterchange(p, getInterchange());
2697+
p.printOptionalAttrDict(
2698+
(*this)->getAttrs(),
2699+
/*elidedAttrs=*/{getScalableSizesAttrName(getOperation()->getName()),
2700+
getStaticSizesAttrName(getOperation()->getName())});
27242701
p << " : ";
27252702
p.printFunctionalType(getOperands().getTypes(), getResults().getTypes());
27262703
}

mlir/test/Dialect/Linalg/transform-ops.mlir

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@ transform.sequence failures(propagate) {
66
%0, %1:2 = transform.structured.tile_using_for %arg0 [2, 0, 3] : (!transform.any_op) -> (!transform.any_op, !transform.any_op, !transform.any_op)
77
}
88

9+
// check that the Attributes of `tile_using_for` are preserved through printing
10+
// and parsing.
11+
transform.sequence failures(propagate) {
12+
^bb1(%arg0: !transform.any_op):
13+
// CHECK %{{.*}}, %{{.*}}:2 = transform.structured.tile %arg0 [2, 0, 3] {interchange = [2, 1], test_attr1 = 1 : i64, test_attr2}
14+
%0, %1:2 = transform.structured.tile_using_for %arg0 [2, 0, 3] {test_attr1 = 1 : i64, interchange = [2, 1], test_attr2}: (!transform.any_op) -> (!transform.any_op, !transform.any_op, !transform.any_op)
15+
}
16+
917
transform.sequence failures(propagate) {
1018
^bb1(%arg0: !transform.any_op):
1119
%0:2 = transform.structured.split %arg0 after 42 { dimension = 0 } : !transform.any_op

0 commit comments

Comments
 (0)