From 99499c3ea7ca922e6f1b7a209f81ffcef03d7d90 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Thu, 28 Apr 2022 11:37:17 -0700 Subject: [PATCH] [OpAsmParser] Simplify logic for requiredOperandCount in parseOperandList. I would ideally like to eliminate 'requiredOperandCount' as a bit of verification that should be in the client side, but it is much more widely used than I expected. Just tidy some pieces up around it given we can't drop it immediately. NFC. Differential Revision: https://reviews.llvm.org/D124629 --- mlir/include/mlir/IR/OpImplementation.h | 39 ++++++++------------ mlir/lib/Dialect/Affine/IR/AffineOps.cpp | 4 +- mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 16 +++----- mlir/lib/Dialect/SCF/SCF.cpp | 3 +- mlir/lib/Parser/Parser.cpp | 21 +---------- 5 files changed, 27 insertions(+), 56 deletions(-) diff --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h index e7123228e835..0399f1082569 100644 --- a/mlir/include/mlir/IR/OpImplementation.h +++ b/mlir/include/mlir/IR/OpImplementation.h @@ -1122,29 +1122,30 @@ public: /// Parse zero or more SSA comma-separated operand references with a specified /// surrounding delimiter, and an optional required operand count. - virtual ParseResult parseOperandList( - SmallVectorImpl &result, int requiredOperandCount = -1, - Delimiter delimiter = Delimiter::None, bool allowResultNumber = true) = 0; + virtual ParseResult + parseOperandList(SmallVectorImpl &result, + Delimiter delimiter = Delimiter::None, + bool allowResultNumber = true, + int requiredOperandCount = -1) = 0; + /// Parse a specified number of comma separated operands. ParseResult parseOperandList(SmallVectorImpl &result, - Delimiter delimiter, - bool allowResultNumber = true) { - return parseOperandList(result, /*requiredOperandCount=*/-1, delimiter, - allowResultNumber); + int requiredOperandCount, + Delimiter delimiter = Delimiter::None) { + return parseOperandList(result, delimiter, + /*allowResultNumber=*/true, requiredOperandCount); } /// Parse zero or more trailing SSA comma-separated trailing operand /// references with a specified surrounding delimiter, and an optional - /// required operand count. A leading comma is expected before the operands. - virtual ParseResult - parseTrailingOperandList(SmallVectorImpl &result, - int requiredOperandCount = -1, - Delimiter delimiter = Delimiter::None) = 0; + /// required operand count. A leading comma is expected before the + /// operands. ParseResult parseTrailingOperandList(SmallVectorImpl &result, - Delimiter delimiter) { - return parseTrailingOperandList(result, /*requiredOperandCount=*/-1, - delimiter); + Delimiter delimiter = Delimiter::None) { + if (failed(parseOptionalComma())) + return success(); // The comma is optional. + return parseOperandList(result, delimiter); } /// Resolve an operand to an SSA value, emitting an error on failure. @@ -1297,14 +1298,6 @@ public: parseOptionalAssignmentListWithTypes(SmallVectorImpl &lhs, SmallVectorImpl &rhs, SmallVectorImpl &types) = 0; - -private: - /// Parse either an operand list or a region argument list depending on - /// whether isOperandList is true. - ParseResult - parseOperandOrRegionArgList(SmallVectorImpl &result, - bool isOperandList, int requiredOperandCount, - Delimiter delimiter); }; //===--------------------------------------------------------------------===// diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp index b37e4f2d47ce..ad04c6f7e210 100644 --- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp +++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp @@ -1075,9 +1075,9 @@ ParseResult AffineDmaStartOp::parse(OpAsmParser &parser, return failure(); // Parse optional stride and elements per stride. - if (parser.parseTrailingOperandList(strideInfo)) { + if (parser.parseTrailingOperandList(strideInfo)) return failure(); - } + if (!strideInfo.empty() && strideInfo.size() != 2) { return parser.emitError(parser.getNameLoc(), "expected two stride related operands"); diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp index 64aecf81e3eb..e2fb9bbcc02a 100644 --- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp +++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp @@ -530,22 +530,18 @@ parseWsLoopControl(OpAsmParser &parser, Region ®ion, size_t numIVs = ivs.size(); Type loopVarType; - if (parser.parseColonType(loopVarType)) - return failure(); - - // Parse loop bounds. - if (parser.parseEqual() || + if (parser.parseColonType(loopVarType) || + // Parse loop bounds. + parser.parseEqual() || parser.parseOperandList(lowerBound, numIVs, - OpAsmParser::Delimiter::Paren)) - return failure(); - if (parser.parseKeyword("to") || + OpAsmParser::Delimiter::Paren) || + parser.parseKeyword("to") || parser.parseOperandList(upperBound, numIVs, OpAsmParser::Delimiter::Paren)) return failure(); - if (succeeded(parser.parseOptionalKeyword("inclusive"))) { + if (succeeded(parser.parseOptionalKeyword("inclusive"))) inclusive = UnitAttr::get(parser.getBuilder().getContext()); - } // Parse step values. if (parser.parseKeyword("step") || diff --git a/mlir/lib/Dialect/SCF/SCF.cpp b/mlir/lib/Dialect/SCF/SCF.cpp index 16de538b36af..299e60b4d4a9 100644 --- a/mlir/lib/Dialect/SCF/SCF.cpp +++ b/mlir/lib/Dialect/SCF/SCF.cpp @@ -2006,8 +2006,7 @@ ParseResult ParallelOp::parse(OpAsmParser &parser, OperationState &result) { // Parse init values. SmallVector initVals; if (succeeded(parser.parseOptionalKeyword("init"))) { - if (parser.parseOperandList(initVals, /*requiredOperandCount=*/-1, - OpAsmParser::Delimiter::Paren)) + if (parser.parseOperandList(initVals, OpAsmParser::Delimiter::Paren)) return failure(); } diff --git a/mlir/lib/Parser/Parser.cpp b/mlir/lib/Parser/Parser.cpp index c3a9056af403..c50cd31badc5 100644 --- a/mlir/lib/Parser/Parser.cpp +++ b/mlir/lib/Parser/Parser.cpp @@ -1297,9 +1297,9 @@ public: /// Parse zero or more SSA comma-separated operand references with a specified /// surrounding delimiter, and an optional required operand count. ParseResult parseOperandList(SmallVectorImpl &result, - int requiredOperandCount = -1, Delimiter delimiter = Delimiter::None, - bool allowResultNumber = true) override { + bool allowResultNumber = true, + int requiredOperandCount = -1) override { auto startLoc = parser.getToken().getLoc(); // The no-delimiter case has some special handling for better diagnostics. @@ -1339,23 +1339,6 @@ public: return success(); } - /// Parse zero or more trailing SSA comma-separated trailing operand - /// references with a specified surrounding delimiter, and an optional - /// required operand count. A leading comma is expected before the operands. - ParseResult - parseTrailingOperandList(SmallVectorImpl &result, - int requiredOperandCount, - Delimiter delimiter) override { - if (parser.getToken().is(Token::comma)) { - parseComma(); - return parseOperandList(result, requiredOperandCount, delimiter); - } - if (requiredOperandCount != -1) - return emitError(parser.getToken().getLoc(), "expected ") - << requiredOperandCount << " operands"; - return success(); - } - /// Resolve an operand to an SSA value, emitting an error on failure. ParseResult resolveOperand(const UnresolvedOperand &operand, Type type, SmallVectorImpl &result) override {