From: Stella Stamenova Date: Sat, 24 Dec 2022 01:31:08 +0000 (-0800) Subject: Revert "Apply shortened printing/parsing form to linalg.reduce." X-Git-Tag: upstream/17.0.6~22680 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5759d9467cbec1440be499448bf086db384fd1d2;p=platform%2Fupstream%2Fllvm.git Revert "Apply shortened printing/parsing form to linalg.reduce." This reverts commit 281c2d49c929c2130e5f1f0e02d6e96dbf14494a. This broke the windows mlir buildbot: https://lab.llvm.org/buildbot/#/builders/13/builds/30167 --- diff --git a/mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOps.td b/mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOps.td index dd2a943..5456ca1 100644 --- a/mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOps.td +++ b/mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOps.td @@ -255,16 +255,6 @@ def MapOp : LinalgStructuredBase_Op<"map", [ linalg.yield %0: f32 } ``` - - Shortened print form is available. Applies to simple maps with one - non-yield operation inside the body. - - The example above will be printed as: - ``` - %add = linalg.map { arith.addf } - ins(%lhs, %rhs : tensor<64xf32>, tensor<64xf32>) - outs(%init: tensor<64xf32>) - ``` }]; let arguments = (ins @@ -339,22 +329,10 @@ def ReduceOp : LinalgStructuredBase_Op<"reduce", [ outs(%init:tensor<16x64xf32>) dimensions = [1] (%in: f32, %out: f32) { - %0 = arith.addf %out, %in: f32 + %0 = arith.addf %in, %out: f32 linalg.yield %0: f32 } ``` - - Shortened print form is available. Applies to simple (not variadic) reduces - with one non-yield operation inside the body. Applies only if the operation - takes `%out` as the first argument. - - The example above will be printed as: - ``` - %reduce = linalg.reduce { arith.addf } - ins(%input:tensor<16x32x64xf32>) - outs(%init:tensor<16x64xf32>) - dimensions = [1] - ``` }]; let arguments = (ins diff --git a/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp b/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp index 87ee3eb..b74537e 100644 --- a/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp +++ b/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp @@ -1016,8 +1016,7 @@ void MapOp::build( static void addBodyWithPayloadOp(OpAsmParser &parser, OperationState &result, const OperationName &payloadOpName, const NamedAttrList &payloadOpAttrs, - ArrayRef operands, - bool initFirst = false) { + ArrayRef operands) { OpBuilder b(parser.getContext()); Region *body = result.addRegion(); Block &block = body->emplaceBlock(); @@ -1027,24 +1026,14 @@ static void addBodyWithPayloadOp(OpAsmParser &parser, OperationState &result, block.addArgument(operand.getType().cast().getElementType(), b.getUnknownLoc()); } - SmallVector payloadOpOperands; - // If initFirst flag is enabled, we consider init as the first position of - // payload operands. - if (initFirst) { - payloadOpOperands.push_back(block.getArguments().back()); - for (const auto& arg : block.getArguments().drop_back()) - payloadOpOperands.push_back(arg); - } else { - payloadOpOperands = {block.getArguments().begin(), - block.getArguments().end()}; - } Operation *payloadOp = b.create( result.location, b.getStringAttr(payloadOpName.getStringRef()), - payloadOpOperands, + block.getArguments(), TypeRange{ result.operands.back().getType().cast().getElementType()}, payloadOpAttrs); + b.create(result.location, payloadOp->getResults()); } @@ -1083,9 +1072,7 @@ ParseResult MapOp::parse(OpAsmParser &parser, OperationState &result) { // Retrieve the operation from the body, if it is the only one (except // yield) and if it gets the same amount of arguments as the body does. -// If initFirst flag is enabled, we check that init takes the first position in -// operands of payload. -static Operation *findPayloadOp(Block *body, bool initFirst = false) { +static Operation *findPayloadOp(Block *body) { if (body->getOperations().size() != 2) return nullptr; Operation &payload = body->getOperations().front(); @@ -1094,22 +1081,10 @@ static Operation *findPayloadOp(Block *body, bool initFirst = false) { if (payload.getNumOperands() == 0 || payload.getNumOperands() != body->getNumArguments()) return nullptr; - if (initFirst) { - // check init - if (payload.getOperands().back() != body->getArgument(0)) + for (const auto &[bbArg, operand] : + llvm::zip(payload.getOperands(), body->getArguments())) { + if (bbArg != operand) return nullptr; - // check rest - for (int i = 1; i < body->getNumArguments(); ++i) { - if (payload.getOperand(i - 1) != body->getArgument(i)) { - return nullptr; - } - } - } else { - for (const auto &[bbArg, operand] : - llvm::zip(payload.getOperands(), body->getArguments())) { - if (bbArg != operand) - return nullptr; - } } return &payload; } @@ -1308,7 +1283,7 @@ ParseResult ReduceOp::parse(OpAsmParser &parser, OperationState &result) { if (payloadOpName.has_value()) { addBodyWithPayloadOp(parser, result, payloadOpName.value(), payloadOpAttrs, - makeArrayRef(result.operands), /*initFirst=*/true); + makeArrayRef(result.operands)); } else { SmallVector regionArgs; if (parser.parseArgumentList(regionArgs, OpAsmParser::Delimiter::Paren, @@ -1331,7 +1306,7 @@ static void printDenseI64ArrayAttr(OpAsmPrinter &p, StringRef attributeName, void ReduceOp::print(OpAsmPrinter &p) { Block *mapper = getBody(); - Operation *payloadOp = findPayloadOp(mapper, /*initFirst=*/true); + Operation *payloadOp = findPayloadOp(mapper); if (payloadOp) { printShortForm(p, payloadOp); } diff --git a/mlir/test/Dialect/Linalg/one-shot-bufferize.mlir b/mlir/test/Dialect/Linalg/one-shot-bufferize.mlir index 7795b63..87763c9 100644 --- a/mlir/test/Dialect/Linalg/one-shot-bufferize.mlir +++ b/mlir/test/Dialect/Linalg/one-shot-bufferize.mlir @@ -363,7 +363,7 @@ func.func @reduce(%input: tensor<16x32x64xf32>, outs(%init:tensor<16x64xf32>) dimensions = [1] (%in: f32, %out: f32) { - %0 = arith.addf %out, %in: f32 + %0 = arith.addf %in, %out: f32 linalg.yield %0: f32 } func.return %reduce : tensor<16x64xf32> diff --git a/mlir/test/Dialect/Linalg/roundtrip.mlir b/mlir/test/Dialect/Linalg/roundtrip.mlir index c665366..611d428 100644 --- a/mlir/test/Dialect/Linalg/roundtrip.mlir +++ b/mlir/test/Dialect/Linalg/roundtrip.mlir @@ -414,7 +414,7 @@ func.func @reduce(%input: tensor<16x32x64xf32>, outs(%init:tensor<16x64xf32>) dimensions = [1] (%in: f32, %out: f32) { - %0 = arith.addf %out, %in: f32 + %0 = arith.addf %in, %out: f32 linalg.yield %0: f32 } func.return %reduce : tensor<16x64xf32> @@ -433,7 +433,7 @@ func.func @reduce_memref(%input: memref<16x32x64xf32>, outs(%init:memref<16x64xf32>) dimensions = [1] (%in: f32, %out: f32) { - %0 = arith.addf %out, %in: f32 + %0 = arith.addf %in, %out: f32 linalg.yield %0: f32 } func.return @@ -587,7 +587,7 @@ func.func @reduce_arith_with_attr(%input: tensor<16x32x64xf32>, outs(%init:tensor<16x64xf32>) dimensions = [1] (%in: f32, %out: f32) { - %0 = arith.addf %out, %in fastmath : f32 + %0 = arith.addf %in, %out fastmath : f32 linalg.yield %0: f32 } func.return %reduce : tensor<16x64xf32>