From 777e7b4f4f6b40314cd484d5fbc60cdd08d700b0 Mon Sep 17 00:00:00 2001 From: River Riddle Date: Fri, 3 May 2019 11:40:57 -0700 Subject: [PATCH] Make the Twine parameter of the current diagnostic emit functions optional. This allows for the ability to exclusively use the new diagnostic interface without breaking all of the existing usages. Several diagnostics emitted in lib/IR have been updated to make use of this functionality. -- PiperOrigin-RevId: 246546044 --- mlir/include/mlir/IR/Diagnostics.h | 10 ++++- mlir/include/mlir/IR/Function.h | 6 +-- mlir/include/mlir/IR/MLIRContext.h | 3 ++ mlir/include/mlir/IR/OpDefinition.h | 14 +++---- mlir/include/mlir/IR/Operation.h | 8 ++-- mlir/lib/IR/Attributes.cpp | 2 +- mlir/lib/IR/Diagnostics.cpp | 9 +++++ mlir/lib/IR/Dialect.cpp | 4 +- mlir/lib/IR/Function.cpp | 20 +++++----- mlir/lib/IR/MLIRContext.cpp | 30 ++++++++++++--- mlir/lib/IR/Operation.cpp | 75 ++++++++++++++++++------------------- mlir/lib/IR/StandardTypes.cpp | 4 +- mlir/lib/IR/Types.cpp | 4 +- 13 files changed, 112 insertions(+), 77 deletions(-) diff --git a/mlir/include/mlir/IR/Diagnostics.h b/mlir/include/mlir/IR/Diagnostics.h index 608f2fe..cc1fe4a 100644 --- a/mlir/include/mlir/IR/Diagnostics.h +++ b/mlir/include/mlir/IR/Diagnostics.h @@ -30,6 +30,7 @@ namespace mlir { class DiagnosticEngine; +class Identifier; class LogicalResult; class Type; @@ -91,13 +92,16 @@ public: private: friend class Diagnostic; - // Construct from an int64_t. + // Construct from a signed integer. explicit DiagnosticArgument(int64_t val) : kind(DiagnosticArgumentKind::Integer), opaqueVal(val) {} + explicit DiagnosticArgument(int val) : DiagnosticArgument(int64_t(val)) {} - // Construct from an uint64_t. + // Construct from an unsigned integer. explicit DiagnosticArgument(uint64_t val) : kind(DiagnosticArgumentKind::Unsigned), opaqueVal(val) {} + explicit DiagnosticArgument(unsigned val) + : DiagnosticArgument(uint64_t(val)) {} // Construct from a string reference. explicit DiagnosticArgument(StringRef val) @@ -179,6 +183,8 @@ public: stringArguments.emplace_back(std::move(str), arguments.size()); return *this; } + /// Stream in an Identifier. + Diagnostic &operator<<(Identifier val); /// Outputs this diagnostic to a stream. void print(raw_ostream &os) const; diff --git a/mlir/include/mlir/IR/Function.h b/mlir/include/mlir/IR/Function.h index 30eca05..8a5b28b 100644 --- a/mlir/include/mlir/IR/Function.h +++ b/mlir/include/mlir/IR/Function.h @@ -251,15 +251,15 @@ public: /// Emit an error about fatal conditions with this function, reporting up to /// any diagnostic handlers that may be listening. - InFlightDiagnostic emitError(const Twine &message); + InFlightDiagnostic emitError(const Twine &message = {}); /// Emit a warning about this function, reporting up to any diagnostic /// handlers that may be listening. - InFlightDiagnostic emitWarning(const Twine &message); + InFlightDiagnostic emitWarning(const Twine &message = {}); /// Emit a remark about this function, reporting up to any diagnostic /// handlers that may be listening. - InFlightDiagnostic emitRemark(const Twine &message); + InFlightDiagnostic emitRemark(const Twine &message = {}); /// Displays the CFG in a window. This is for use from the debugger and /// depends on Graphviz to generate the graph. diff --git a/mlir/include/mlir/IR/MLIRContext.h b/mlir/include/mlir/IR/MLIRContext.h index 4ee0dbd..6165ccb 100644 --- a/mlir/include/mlir/IR/MLIRContext.h +++ b/mlir/include/mlir/IR/MLIRContext.h @@ -62,12 +62,15 @@ public: MLIRContextImpl &getImpl() { return *impl.get(); } /// Emit an error message using the diagnostic engine. + InFlightDiagnostic emitError(Location location); InFlightDiagnostic emitError(Location location, const Twine &message); /// Emit a warning message using the diagnostic engine. + InFlightDiagnostic emitWarning(Location location); InFlightDiagnostic emitWarning(Location location, const Twine &message); /// Emit a remark message using the diagnostic engine. + InFlightDiagnostic emitRemark(Location location); InFlightDiagnostic emitRemark(Location location, const Twine &message); /// Returns the diagnostic engine for this context. diff --git a/mlir/include/mlir/IR/OpDefinition.h b/mlir/include/mlir/IR/OpDefinition.h index 08fd4ef..2532dc8 100644 --- a/mlir/include/mlir/IR/OpDefinition.h +++ b/mlir/include/mlir/IR/OpDefinition.h @@ -103,21 +103,21 @@ public: /// Remove this operation from its parent block and delete it. void erase() { state->erase(); } + /// Emit an error with the op name prefixed, like "'dim' op " which is + /// convenient for verifiers. + InFlightDiagnostic emitOpError(const Twine &message = {}); + /// Emit an error about fatal conditions with this operation, reporting up to /// any diagnostic handlers that may be listening. - InFlightDiagnostic emitError(const Twine &message); - - /// Emit an error with the op name prefixed, like "'dim' op " which is - /// convenient for verifiers. This always returns failure. - InFlightDiagnostic emitOpError(const Twine &message); + InFlightDiagnostic emitError(const Twine &message = {}); /// Emit a warning about this operation, reporting up to any diagnostic /// handlers that may be listening. - InFlightDiagnostic emitWarning(const Twine &message); + InFlightDiagnostic emitWarning(const Twine &message = {}); /// Emit a remark about this operation, reporting up to any diagnostic /// handlers that may be listening. - InFlightDiagnostic emitRemark(const Twine &message); + InFlightDiagnostic emitRemark(const Twine &message = {}); // These are default implementations of customization hooks. public: diff --git a/mlir/include/mlir/IR/Operation.h b/mlir/include/mlir/IR/Operation.h index 908f001..0f1f3e9 100644 --- a/mlir/include/mlir/IR/Operation.h +++ b/mlir/include/mlir/IR/Operation.h @@ -431,19 +431,19 @@ public: /// Emit an error with the op name prefixed, like "'dim' op " which is /// convenient for verifiers. - InFlightDiagnostic emitOpError(const Twine &message); + InFlightDiagnostic emitOpError(const Twine &message = {}); /// Emit an error about fatal conditions with this operation, reporting up to /// any diagnostic handlers that may be listening. - InFlightDiagnostic emitError(const Twine &message); + InFlightDiagnostic emitError(const Twine &message = {}); /// Emit a warning about this operation, reporting up to any diagnostic /// handlers that may be listening. - InFlightDiagnostic emitWarning(const Twine &message); + InFlightDiagnostic emitWarning(const Twine &message = {}); /// Emit a remark about this operation, reporting up to any diagnostic /// handlers that may be listening. - InFlightDiagnostic emitRemark(const Twine &message); + InFlightDiagnostic emitRemark(const Twine &message = {}); private: Operation(Location location, OperationName name, unsigned numResults, diff --git a/mlir/lib/IR/Attributes.cpp b/mlir/lib/IR/Attributes.cpp index 64f5a7e..e4b5b78 100644 --- a/mlir/lib/IR/Attributes.cpp +++ b/mlir/lib/IR/Attributes.cpp @@ -163,7 +163,7 @@ static FloatAttr getFloatAttr(Type type, double value, llvm::Optional loc) { if (!type.isa()) { if (loc) - type.getContext()->emitError(*loc, "expected floating point type"); + type.getContext()->emitError(*loc) << "expected floating point type"; return nullptr; } diff --git a/mlir/lib/IR/Diagnostics.cpp b/mlir/lib/IR/Diagnostics.cpp index ba82040..35ad35e 100644 --- a/mlir/lib/IR/Diagnostics.cpp +++ b/mlir/lib/IR/Diagnostics.cpp @@ -16,6 +16,7 @@ // ============================================================================= #include "mlir/IR/Diagnostics.h" +#include "mlir/IR/Identifier.h" #include "mlir/IR/Location.h" #include "mlir/IR/Types.h" #include "llvm/ADT/Twine.h" @@ -62,6 +63,14 @@ void DiagnosticArgument::print(raw_ostream &os) const { // Diagnostic //===----------------------------------------------------------------------===// +/// Stream in an Identifier. +Diagnostic &Diagnostic::operator<<(Identifier val) { + // An identifier is stored in the context, so we don't need to worry about the + // lifetime of its data. + arguments.push_back(DiagnosticArgument(val.strref())); + return *this; +} + /// Outputs this diagnostic to a stream. void Diagnostic::print(raw_ostream &os) const { for (auto &arg : getArguments()) diff --git a/mlir/lib/IR/Dialect.cpp b/mlir/lib/IR/Dialect.cpp index 89ff1b5..aa0ae76 100644 --- a/mlir/lib/IR/Dialect.cpp +++ b/mlir/lib/IR/Dialect.cpp @@ -70,8 +70,8 @@ Dialect::~Dialect() {} /// Parse a type registered to this dialect. Type Dialect::parseType(StringRef tyData, Location loc) const { - getContext()->emitError(loc, "dialect '" + getNamespace() + - "' provides no type parsing hook"); + getContext()->emitError(loc) + << "dialect '" << getNamespace() << "' provides no type parsing hook"; return Type(); } diff --git a/mlir/lib/IR/Function.cpp b/mlir/lib/IR/Function.cpp index 5e72b83..7651abf 100644 --- a/mlir/lib/IR/Function.cpp +++ b/mlir/lib/IR/Function.cpp @@ -116,10 +116,12 @@ void Function::erase() { getModule()->getFunctions().erase(this); } -/// Emit a remark about this function, reporting up to any diagnostic -/// handlers that may be listening. -InFlightDiagnostic Function::emitRemark(const Twine &message) { - return getContext()->emitRemark(getLoc(), message); +/// Emit an error about fatal conditions with this function, reporting up to +/// any diagnostic handlers that may be listening. This function always +/// returns failure. NOTE: This may terminate the containing application, only +/// use when the IR is in an inconsistent state. +InFlightDiagnostic Function::emitError(const Twine &message) { + return getContext()->emitError(getLoc(), message); } /// Emit a warning about this function, reporting up to any diagnostic @@ -128,12 +130,10 @@ InFlightDiagnostic Function::emitWarning(const Twine &message) { return getContext()->emitWarning(getLoc(), message); } -/// Emit an error about fatal conditions with this function, reporting up to -/// any diagnostic handlers that may be listening. This function always -/// returns failure. NOTE: This may terminate the containing application, only -/// use when the IR is in an inconsistent state. -InFlightDiagnostic Function::emitError(const Twine &message) { - return getContext()->emitError(getLoc(), message); +/// Emit a remark about this function, reporting up to any diagnostic +/// handlers that may be listening. +InFlightDiagnostic Function::emitRemark(const Twine &message) { + return getContext()->emitRemark(getLoc(), message); } /// Clone the internal blocks from this function into dest and all attributes diff --git a/mlir/lib/IR/MLIRContext.cpp b/mlir/lib/IR/MLIRContext.cpp index 9b4ae2c..ac041ae 100644 --- a/mlir/lib/IR/MLIRContext.cpp +++ b/mlir/lib/IR/MLIRContext.cpp @@ -422,24 +422,42 @@ static ArrayRef copyArrayRefInto(llvm::BumpPtrAllocator &allocator, // Diagnostic Handlers //===----------------------------------------------------------------------===// +/// Helper function used to emit a diagnostic with an optionally empty twine +/// message. If the message is empty, then it is not inserted into the +/// diagnostic. +static InFlightDiagnostic emitDiag(MLIRContextImpl &ctx, Location location, + DiagnosticSeverity severity, + const llvm::Twine &message) { + auto diag = ctx.diagEngine.emit(location, severity); + if (!message.isTriviallyEmpty()) + diag << message; + return diag; +} + +InFlightDiagnostic MLIRContext::emitError(Location location) { + return emitError(location, /*message=*/{}); +} InFlightDiagnostic MLIRContext::emitError(Location location, const llvm::Twine &message) { - return getImpl().diagEngine.emit(location, DiagnosticSeverity::Error) - << message; + return emitDiag(getImpl(), location, DiagnosticSeverity::Error, message); } /// Emit a warning message using the diagnostic engine. +InFlightDiagnostic MLIRContext::emitWarning(Location location) { + return emitWarning(location, /*message=*/{}); +} InFlightDiagnostic MLIRContext::emitWarning(Location location, const Twine &message) { - return getImpl().diagEngine.emit(location, DiagnosticSeverity::Warning) - << message; + return emitDiag(getImpl(), location, DiagnosticSeverity::Warning, message); } /// Emit a remark message using the diagnostic engine. +InFlightDiagnostic MLIRContext::emitRemark(Location location) { + return emitRemark(location, /*message=*/{}); +} InFlightDiagnostic MLIRContext::emitRemark(Location location, const Twine &message) { - return getImpl().diagEngine.emit(location, DiagnosticSeverity::Remark) - << message; + return emitDiag(getImpl(), location, DiagnosticSeverity::Remark, message); } /// Returns the diagnostic engine for this context. diff --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp index c214a25..9a13cee 100644 --- a/mlir/lib/IR/Operation.cpp +++ b/mlir/lib/IR/Operation.cpp @@ -305,10 +305,10 @@ void Operation::walk(const std::function &callback) { // Other //===----------------------------------------------------------------------===// -/// Emit a remark about this operation, reporting up to any diagnostic -/// handlers that may be listening. -InFlightDiagnostic Operation::emitRemark(const Twine &message) { - return getContext()->emitRemark(getLoc(), message); +/// Emit an error about fatal conditions with this operation, reporting up to +/// any diagnostic handlers that may be listening. +InFlightDiagnostic Operation::emitError(const Twine &message) { + return getContext()->emitError(getLoc(), message); } /// Emit a warning about this operation, reporting up to any diagnostic @@ -317,10 +317,10 @@ InFlightDiagnostic Operation::emitWarning(const Twine &message) { return getContext()->emitWarning(getLoc(), message); } -/// Emit an error about fatal conditions with this operation, reporting up to -/// any diagnostic handlers that may be listening. -InFlightDiagnostic Operation::emitError(const Twine &message) { - return getContext()->emitError(getLoc(), message); +/// Emit a remark about this operation, reporting up to any diagnostic +/// handlers that may be listening. +InFlightDiagnostic Operation::emitRemark(const Twine &message) { + return getContext()->emitRemark(getLoc(), message); } /// Given an operation 'other' that is within the same parent block, return @@ -542,7 +542,7 @@ LogicalResult Operation::fold(SmallVectorImpl &results) { /// Emit an error with the op name prefixed, like "'dim' op " which is /// convenient for verifiers. InFlightDiagnostic Operation::emitOpError(const Twine &message) { - return emitError(Twine('\'') + getName().getStringRef() + "' op " + message); + return emitError() << "'" << getName().getStringRef() << "' op " << message; } //===----------------------------------------------------------------------===// @@ -671,22 +671,21 @@ InFlightDiagnostic OpState::emitRemark(const Twine &message) { LogicalResult OpTrait::impl::verifyZeroOperands(Operation *op) { if (op->getNumOperands() != 0) - return op->emitOpError("requires zero operands"); + return op->emitOpError() << "requires zero operands"; return success(); } LogicalResult OpTrait::impl::verifyOneOperand(Operation *op) { if (op->getNumOperands() != 1) - return op->emitOpError("requires a single operand"); + return op->emitOpError() << "requires a single operand"; return success(); } LogicalResult OpTrait::impl::verifyNOperands(Operation *op, unsigned numOperands) { if (op->getNumOperands() != numOperands) { - return op->emitOpError("expected " + Twine(numOperands) + - " operands, but found " + - Twine(op->getNumOperands())); + return op->emitOpError() << "expected " << numOperands + << " operands, but found " << op->getNumOperands(); } return success(); } @@ -694,8 +693,8 @@ LogicalResult OpTrait::impl::verifyNOperands(Operation *op, LogicalResult OpTrait::impl::verifyAtLeastNOperands(Operation *op, unsigned numOperands) { if (op->getNumOperands() < numOperands) - return op->emitOpError("expected " + Twine(numOperands) + - " or more operands"); + return op->emitOpError() + << "expected " << numOperands << " or more operands"; return success(); } @@ -715,7 +714,7 @@ LogicalResult OpTrait::impl::verifyOperandsAreIntegerLike(Operation *op) { for (auto *operand : op->getOperands()) { auto type = getTensorOrVectorElementType(operand->getType()); if (!type.isIntOrIndex()) - return op->emitOpError("requires an integer or index type"); + return op->emitOpError() << "requires an integer or index type"; } return success(); } @@ -729,34 +728,34 @@ LogicalResult OpTrait::impl::verifySameTypeOperands(Operation *op) { auto type = op->getOperand(0)->getType(); for (unsigned i = 1; i < nOperands; ++i) if (op->getOperand(i)->getType() != type) - return op->emitOpError("requires all operands to have the same type"); + return op->emitOpError() << "requires all operands to have the same type"; return success(); } LogicalResult OpTrait::impl::verifyZeroResult(Operation *op) { if (op->getNumResults() != 0) - return op->emitOpError("requires zero results"); + return op->emitOpError() << "requires zero results"; return success(); } LogicalResult OpTrait::impl::verifyOneResult(Operation *op) { if (op->getNumResults() != 1) - return op->emitOpError("requires one result"); + return op->emitOpError() << "requires one result"; return success(); } LogicalResult OpTrait::impl::verifyNResults(Operation *op, unsigned numOperands) { if (op->getNumResults() != numOperands) - return op->emitOpError("expected " + Twine(numOperands) + " results"); + return op->emitOpError() << "expected " << numOperands << " results"; return success(); } LogicalResult OpTrait::impl::verifyAtLeastNResults(Operation *op, unsigned numOperands) { if (op->getNumResults() < numOperands) - return op->emitOpError("expected " + Twine(numOperands) + - " or more results"); + return op->emitOpError() + << "expected " << numOperands << " or more results"; return success(); } @@ -788,13 +787,13 @@ LogicalResult OpTrait::impl::verifySameOperandsAndResultShape(Operation *op) { auto type = op->getOperand(0)->getType(); for (unsigned i = 0, e = op->getNumResults(); i < e; ++i) { if (failed(verifyShapeMatch(op->getResult(i)->getType(), type))) - return op->emitOpError( - "requires the same shape for all operands and results"); + return op->emitOpError() + << "requires the same shape for all operands and results"; } for (unsigned i = 1, e = op->getNumOperands(); i < e; ++i) { if (failed(verifyShapeMatch(op->getOperand(i)->getType(), type))) - return op->emitOpError( - "requires the same shape for all operands and results"); + return op->emitOpError() + << "requires the same shape for all operands and results"; } return success(); } @@ -806,13 +805,13 @@ LogicalResult OpTrait::impl::verifySameOperandsAndResultType(Operation *op) { auto type = op->getResult(0)->getType(); for (unsigned i = 1, e = op->getNumResults(); i < e; ++i) { if (op->getResult(i)->getType() != type) - return op->emitOpError( - "requires the same type for all operands and results"); + return op->emitOpError() + << "requires the same type for all operands and results"; } for (unsigned i = 0, e = op->getNumOperands(); i < e; ++i) { if (op->getOperand(i)->getType() != type) - return op->emitOpError( - "requires the same type for all operands and results"); + return op->emitOpError() + << "requires the same type for all operands and results"; } return success(); } @@ -821,14 +820,14 @@ static LogicalResult verifyBBArguments(Operation::operand_range operands, Block *destBB, Operation *op) { unsigned operandCount = std::distance(operands.begin(), operands.end()); if (operandCount != destBB->getNumArguments()) - return op->emitError("branch has " + Twine(operandCount) + - " operands, but target block has " + - Twine(destBB->getNumArguments())); + return op->emitError() << "branch has " << operandCount + << " operands, but target block has " + << destBB->getNumArguments(); auto operandIt = operands.begin(); for (unsigned i = 0, e = operandCount; i != e; ++i, ++operandIt) { if ((*operandIt)->getType() != destBB->getArgument(i)->getType()) - return op->emitError("type mismatch in bb argument #" + Twine(i)); + return op->emitError() << "type mismatch in bb argument #" << i; } return success(); @@ -864,7 +863,7 @@ LogicalResult OpTrait::impl::verifyResultsAreBoolLike(Operation *op) { auto elementType = getTensorOrVectorElementType(result->getType()); bool isBoolType = elementType.isInteger(1); if (!isBoolType) - return op->emitOpError("requires a bool result type"); + return op->emitOpError() << "requires a bool result type"; } return success(); @@ -873,7 +872,7 @@ LogicalResult OpTrait::impl::verifyResultsAreBoolLike(Operation *op) { LogicalResult OpTrait::impl::verifyResultsAreFloatLike(Operation *op) { for (auto *result : op->getResults()) if (!getTensorOrVectorElementType(result->getType()).isa()) - return op->emitOpError("requires a floating point type"); + return op->emitOpError() << "requires a floating point type"; return success(); } @@ -882,7 +881,7 @@ LogicalResult OpTrait::impl::verifyResultsAreIntegerLike(Operation *op) { for (auto *result : op->getResults()) { auto type = getTensorOrVectorElementType(result->getType()); if (!type.isIntOrIndex()) - return op->emitOpError("requires an integer or index type"); + return op->emitOpError() << "requires an integer or index type"; } return success(); } diff --git a/mlir/lib/IR/StandardTypes.cpp b/mlir/lib/IR/StandardTypes.cpp index 25a0233..c14a6b8 100644 --- a/mlir/lib/IR/StandardTypes.cpp +++ b/mlir/lib/IR/StandardTypes.cpp @@ -36,8 +36,8 @@ LogicalResult IntegerType::verifyConstructionInvariants( llvm::Optional loc, MLIRContext *context, unsigned width) { if (width > IntegerType::kMaxWidth) { if (loc) - context->emitError(*loc, "integer bitwidth is limited to " + - Twine(IntegerType::kMaxWidth) + " bits"); + context->emitError(*loc) << "integer bitwidth is limited to " + << IntegerType::kMaxWidth << " bits"; return failure(); } return success(); diff --git a/mlir/lib/IR/Types.cpp b/mlir/lib/IR/Types.cpp index dba33a9..197e669 100644 --- a/mlir/lib/IR/Types.cpp +++ b/mlir/lib/IR/Types.cpp @@ -77,8 +77,8 @@ LogicalResult OpaqueType::verifyConstructionInvariants( StringRef typeData) { if (!Dialect::isValidNamespace(dialect.strref())) { if (loc) - context->emitError(*loc, "invalid dialect namespace '" + - dialect.strref() + "'"); + context->emitError(*loc) + << "invalid dialect namespace '" << dialect << "'"; return failure(); } return success(); -- 2.7.4