[Mlir-commits] [mlir] Lazy initialize diagnostic when handling MLIR properties (PR #65868)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Sat Sep 9 17:29:51 PDT 2023
llvmbot wrote:
@llvm/pr-subscribers-mlir
<details>
<summary>Changes</summary>
Instead of eagerly creating a diagnostic that will be discarded in the normal case, switch to lazy initialization on error.
--
Patch is 31.66 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/65868.diff
16 Files Affected:
- (modified) mlir/include/mlir/IR/ExtensibleDialect.h (+5-4)
- (modified) mlir/include/mlir/IR/ODSSupport.h (+11-6)
- (modified) mlir/include/mlir/IR/OpDefinition.h (+4-3)
- (modified) mlir/include/mlir/IR/Operation.h (+3-2)
- (modified) mlir/include/mlir/IR/OperationSupport.h (+19-20)
- (modified) mlir/include/mlir/IR/Properties.td (+1-1)
- (modified) mlir/lib/AsmParser/Parser.cpp (+24-10)
- (modified) mlir/lib/CAPI/IR/IR.cpp (+14-9)
- (modified) mlir/lib/IR/MLIRContext.cpp (+1-1)
- (modified) mlir/lib/IR/ODSSupport.cpp (+19-22)
- (modified) mlir/lib/IR/Operation.cpp (+3-4)
- (modified) mlir/lib/IR/OperationSupport.cpp (+3-4)
- (modified) mlir/test/lib/Dialect/Test/TestDialect.cpp (+20-24)
- (modified) mlir/test/lib/Dialect/Test/TestDialect.h (+3-3)
- (modified) mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp (+10-12)
- (modified) mlir/unittests/IR/OpPropertiesTest.cpp (+15-13)
<pre>
diff --git a/mlir/include/mlir/IR/ExtensibleDialect.h b/mlir/include/mlir/IR/ExtensibleDialect.h
index e4d8d2d6000fc60..37821d3a2a5163f 100644
--- a/mlir/include/mlir/IR/ExtensibleDialect.h
+++ b/mlir/include/mlir/IR/ExtensibleDialect.h
@@ -486,10 +486,11 @@ class DynamicOpDefinition : public OperationName::Impl {
void populateDefaultProperties(OperationName opName,
OpaqueProperties properties) final {}
- LogicalResult setPropertiesFromAttr(OperationName opName,
- OpaqueProperties properties,
- Attribute attr,
- InFlightDiagnostic *diag) final {
+ LogicalResult
+ setPropertiesFromAttr(OperationName opName, OpaqueProperties properties,
+ Attribute attr,
+ function_ref<InFlightDiagnostic &()> getDiag) final {
+ getDiag() << "extensible Dialects don't support properties";
return failure();
}
Attribute getPropertiesAsAttr(Operation *op) final { return {}; }
diff --git a/mlir/include/mlir/IR/ODSSupport.h b/mlir/include/mlir/IR/ODSSupport.h
index 687f764ae95fd99..748bf52a55c557a 100644
--- a/mlir/include/mlir/IR/ODSSupport.h
+++ b/mlir/include/mlir/IR/ODSSupport.h
@@ -14,6 +14,8 @@
#define MLIR_IR_ODSSUPPORT_H
#include "mlir/IR/Attributes.h"
+#include "mlir/IR/MLIRContext.h"
+#include "mlir/Support/LLVM.h"
namespace mlir {
@@ -24,8 +26,9 @@ namespace mlir {
/// Convert an IntegerAttr attribute to an int64_t, or return an error if the
/// attribute isn't an IntegerAttr. If the optional diagnostic is provided an
/// error message is also emitted.
-LogicalResult convertFromAttribute(int64_t &storage, Attribute attr,
- InFlightDiagnostic *diag);
+LogicalResult
+convertFromAttribute(int64_t &storage, Attribute attr,
+ function_ref<InFlightDiagnostic &()> getDiag);
/// Convert the provided int64_t to an IntegerAttr attribute.
Attribute convertToAttribute(MLIRContext *ctx, int64_t storage);
@@ -34,15 +37,17 @@ Attribute convertToAttribute(MLIRContext *ctx, int64_t storage);
/// storage has the same size as the array. An error is returned if the
/// attribute isn't a DenseI64ArrayAttr or it does not have the same size. If
/// the optional diagnostic is provided an error message is also emitted.
-LogicalResult convertFromAttribute(MutableArrayRef<int64_t> storage,
- Attribute attr, InFlightDiagnostic *diag);
+LogicalResult
+convertFromAttribute(MutableArrayRef<int64_t> storage, Attribute attr,
+ function_ref<InFlightDiagnostic &()> getDiag);
/// Convert a DenseI32ArrayAttr to the provided storage. It is expected that the
/// storage has the same size as the array. An error is returned if the
/// attribute isn't a DenseI32ArrayAttr or it does not have the same size. If
/// the optional diagnostic is provided an error message is also emitted.
-LogicalResult convertFromAttribute(MutableArrayRef<int32_t> storage,
- Attribute attr, InFlightDiagnostic *diag);
+LogicalResult
+convertFromAttribute(MutableArrayRef<int32_t> storage, Attribute attr,
+ function_ref<InFlightDiagnostic &()> getDiag);
/// Convert the provided ArrayRef<int64_t> to a DenseI64ArrayAttr attribute.
Attribute convertToAttribute(MLIRContext *ctx, ArrayRef<int64_t> storage);
diff --git a/mlir/include/mlir/IR/OpDefinition.h b/mlir/include/mlir/IR/OpDefinition.h
index 84ba46f4d6f3ec1..895f17dfe1d07c8 100644
--- a/mlir/include/mlir/IR/OpDefinition.h
+++ b/mlir/include/mlir/IR/OpDefinition.h
@@ -1769,9 +1769,10 @@ class Op : public OpState, public Traits<ConcreteType>... {
/// the namespace where the properties are defined. It can also be overridden
/// in the derived ConcreteOp.
template <typename PropertiesTy>
- static LogicalResult setPropertiesFromAttr(PropertiesTy &prop, Attribute attr,
- InFlightDiagnostic *diag) {
- return setPropertiesFromAttribute(prop, attr, diag);
+ static LogicalResult
+ setPropertiesFromAttr(PropertiesTy &prop, Attribute attr,
+ function_ref<InFlightDiagnostic &()> getDiag) {
+ return setPropertiesFromAttribute(prop, attr, getDiag);
}
/// Convert the provided properties to an attribute. This default
/// implementation forwards to a free function `getPropertiesAsAttribute` that
diff --git a/mlir/include/mlir/IR/Operation.h b/mlir/include/mlir/IR/Operation.h
index 361a38e87b6ba32..b815eaf8899d6fc 100644
--- a/mlir/include/mlir/IR/Operation.h
+++ b/mlir/include/mlir/IR/Operation.h
@@ -882,8 +882,9 @@ class alignas(8) Operation final
/// matching the expectations of the properties for this operation. This is
/// mostly useful for unregistered operations or used when parsing the
/// generic format. An optional diagnostic can be passed in for richer errors.
- LogicalResult setPropertiesFromAttribute(Attribute attr,
- InFlightDiagnostic *diagnostic);
+ LogicalResult
+ setPropertiesFromAttribute(Attribute attr,
+ function_ref<InFlightDiagnostic &()> getDiag);
/// Copy properties from an existing other properties object. The two objects
/// must be the same type.
diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index 670dd289c480a30..19ffddc30904897 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -136,9 +136,9 @@ class OperationName {
virtual void deleteProperties(OpaqueProperties) = 0;
virtual void populateDefaultProperties(OperationName opName,
OpaqueProperties properties) = 0;
- virtual LogicalResult setPropertiesFromAttr(OperationName, OpaqueProperties,
- Attribute,
- InFlightDiagnostic *) = 0;
+ virtual LogicalResult
+ setPropertiesFromAttr(OperationName, OpaqueProperties, Attribute,
+ function_ref<InFlightDiagnostic &()> getDiag) = 0;
virtual Attribute getPropertiesAsAttr(Operation *) = 0;
virtual void copyProperties(OpaqueProperties, OpaqueProperties) = 0;
virtual llvm::hash_code hashProperties(OpaqueProperties) = 0;
@@ -216,8 +216,9 @@ class OperationName {
void deleteProperties(OpaqueProperties) final;
void populateDefaultProperties(OperationName opName,
OpaqueProperties properties) final;
- LogicalResult setPropertiesFromAttr(OperationName, OpaqueProperties,
- Attribute, InFlightDiagnostic *) final;
+ LogicalResult
+ setPropertiesFromAttr(OperationName, OpaqueProperties, Attribute,
+ function_ref<InFlightDiagnostic &()> getDiag) final;
Attribute getPropertiesAsAttr(Operation *) final;
void copyProperties(OpaqueProperties, OpaqueProperties) final;
llvm::hash_code hashProperties(OpaqueProperties) final;
@@ -434,12 +435,10 @@ class OperationName {
}
/// Define the op properties from the provided Attribute.
- LogicalResult
- setOpPropertiesFromAttribute(OperationName opName,
- OpaqueProperties properties, Attribute attr,
- InFlightDiagnostic *diagnostic) const {
- return getImpl()->setPropertiesFromAttr(opName, properties, attr,
- diagnostic);
+ LogicalResult setOpPropertiesFromAttribute(
+ OperationName opName, OpaqueProperties properties, Attribute attr,
+ function_ref<InFlightDiagnostic &()> getDiag) const {
+ return getImpl()->setPropertiesFromAttr(opName, properties, attr, getDiag);
}
void copyOpProperties(OpaqueProperties lhs, OpaqueProperties rhs) const {
@@ -628,16 +627,15 @@ class RegisteredOperationName : public OperationName {
*properties.as<Properties *>());
}
- LogicalResult setPropertiesFromAttr(OperationName opName,
- OpaqueProperties properties,
- Attribute attr,
- InFlightDiagnostic *diag) final {
+ LogicalResult
+ setPropertiesFromAttr(OperationName opName, OpaqueProperties properties,
+ Attribute attr,
+ function_ref<InFlightDiagnostic &()> getDiag) final {
if constexpr (hasProperties) {
auto p = properties.as<Properties *>();
- return ConcreteOp::setPropertiesFromAttr(*p, attr, diag);
+ return ConcreteOp::setPropertiesFromAttr(*p, attr, getDiag);
}
- if (diag)
- *diag << "This operation does not support properties";
+ getDiag() << "this operation does not support properties";
return failure();
}
Attribute getPropertiesAsAttr(Operation *op) final {
@@ -997,8 +995,9 @@ struct OperationState {
// Set the properties defined on this OpState on the given operation,
// optionally emit diagnostics on error through the provided diagnostic.
- LogicalResult setProperties(Operation *op,
- InFlightDiagnostic *diagnostic) const;
+ LogicalResult
+ setProperties(Operation *op,
+ function_ref<InFlightDiagnostic &()> getDiag) const;
void addOperands(ValueRange newOperands);
diff --git a/mlir/include/mlir/IR/Properties.td b/mlir/include/mlir/IR/Properties.td
index 3d7de7e4f460e51..99da1763524fa94 100644
--- a/mlir/include/mlir/IR/Properties.td
+++ b/mlir/include/mlir/IR/Properties.td
@@ -55,7 +55,7 @@ class Property<string storageTypeParam = "", string desc = ""> {
// Format:
// - `$_storage` is the storage type value.
// - `$_attr` is the attribute.
- // - `$_diag` is an optional Diagnostic pointer to emit error.
+ // - `$_diag` is a callback to get a Diagnostic to emit error.
//
// The expression must return a LogicalResult
code convertFromAttribute = [{
diff --git a/mlir/lib/AsmParser/Parser.cpp b/mlir/lib/AsmParser/Parser.cpp
index 02bf9a418063991..84f44dba806df01 100644
--- a/mlir/lib/AsmParser/Parser.cpp
+++ b/mlir/lib/AsmParser/Parser.cpp
@@ -18,8 +18,10 @@
#include "mlir/IR/AffineMap.h"
#include "mlir/IR/AsmState.h"
#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/Diagnostics.h"
#include "mlir/IR/Dialect.h"
#include "mlir/IR/Verifier.h"
+#include "mlir/Support/InterfaceSupport.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/StringSet.h"
@@ -29,6 +31,7 @@
#include "llvm/Support/PrettyStackTrace.h"
#include "llvm/Support/SourceMgr.h"
#include <algorithm>
+#include <memory>
#include <optional>
using namespace mlir;
@@ -1443,12 +1446,17 @@ Operation *OperationParser::parseGenericOperation() {
// Try setting the properties for the operation, using a diagnostic to print
// errors.
if (properties) {
- InFlightDiagnostic diagnostic =
- mlir::emitError(srcLocation, "invalid properties ")
- << properties << " for op " << name << ": ";
- if (failed(op->setPropertiesFromAttribute(properties, &diagnostic)))
+ std::unique_ptr<InFlightDiagnostic> diagnostic;
+ auto getDiag = [&]() -> InFlightDiagnostic & {
+ if (!diagnostic) {
+ diagnostic = std::make_unique<InFlightDiagnostic>(
+ mlir::emitError(srcLocation, "invalid properties ")
+ << properties << " for op " << name << ": ");
+ }
+ return *diagnostic;
+ };
+ if (failed(op->setPropertiesFromAttribute(properties, getDiag)))
return nullptr;
- diagnostic.abandon();
}
return op;
@@ -2001,12 +2009,18 @@ OperationParser::parseCustomOperation(ArrayRef<ResultRecord> resultIDs) {
// Try setting the properties for the operation.
if (properties) {
- InFlightDiagnostic diagnostic =
- mlir::emitError(srcLocation, "invalid properties ")
- << properties << " for op " << op->getName().getStringRef() << ": ";
- if (failed(op->setPropertiesFromAttribute(properties, &diagnostic)))
+ std::unique_ptr<InFlightDiagnostic> diagnostic;
+ auto getDiag = [&]() -> InFlightDiagnostic & {
+ if (!diagnostic) {
+ diagnostic = std::make_unique<InFlightDiagnostic>(
+ mlir::emitError(srcLocation, "invalid properties ")
+ << properties << " for op " << op->getName().getStringRef()
+ << ": ");
+ }
+ return *diagnostic;
+ };
+ if (failed(op->setPropertiesFromAttribute(properties, getDiag)))
return nullptr;
- diagnostic.abandon();
}
return op;
}
diff --git a/mlir/lib/CAPI/IR/IR.cpp b/mlir/lib/CAPI/IR/IR.cpp
index ccdae1424998567..ef234a912490eea 100644
--- a/mlir/lib/CAPI/IR/IR.cpp
+++ b/mlir/lib/CAPI/IR/IR.cpp
@@ -370,16 +370,21 @@ static LogicalResult inferOperationTypes(OperationState &state) {
if (!properties && info->getOpPropertyByteSize() > 0 && !attributes.empty()) {
auto prop = std::make_unique<char[]>(info->getOpPropertyByteSize());
properties = OpaqueProperties(prop.get());
- InFlightDiagnostic diag = emitError(state.location)
- << " failed properties conversion while building "
- << state.name.getStringRef() << " with `"
- << attributes << "`: ";
- if (failed(info->setOpPropertiesFromAttribute(state.name, properties,
- attributes, &diag))) {
- return failure();
+ if (properties) {
+ std::unique_ptr<InFlightDiagnostic> diagnostic;
+ auto getDiag = [&]() -> InFlightDiagnostic & {
+ if (!diagnostic) {
+ diagnostic = std::make_unique<InFlightDiagnostic>(
+ emitError(state.location)
+ << " failed properties conversion while building "
+ << state.name.getStringRef() << " with `" << attributes << "`: ");
+ }
+ return *diagnostic;
+ };
+ if (failed(info->setOpPropertiesFromAttribute(state.name, properties,
+ attributes, getDiag)))
+ return failure();
}
- diag.abandon();
-
if (succeeded(inferInterface->inferReturnTypes(
context, state.location, state.operands, attributes, properties,
state.regions, state.types))) {
diff --git a/mlir/lib/IR/MLIRContext.cpp b/mlir/lib/IR/MLIRContext.cpp
index e19c3d4d54179b3..5f1d036d22b918e 100644
--- a/mlir/lib/IR/MLIRContext.cpp
+++ b/mlir/lib/IR/MLIRContext.cpp
@@ -852,7 +852,7 @@ void OperationName::UnregisteredOpModel::populateDefaultProperties(
OperationName opName, OpaqueProperties properties) {}
LogicalResult OperationName::UnregisteredOpModel::setPropertiesFromAttr(
OperationName opName, OpaqueProperties properties, Attribute attr,
- InFlightDiagnostic *diag) {
+ function_ref<InFlightDiagnostic &()> getDiag) {
*properties.as<Attribute *>() = attr;
return success();
}
diff --git a/mlir/lib/IR/ODSSupport.cpp b/mlir/lib/IR/ODSSupport.cpp
index f67e7dbf38592e2..0601430c4461651 100644
--- a/mlir/lib/IR/ODSSupport.cpp
+++ b/mlir/lib/IR/ODSSupport.cpp
@@ -18,13 +18,12 @@
using namespace mlir;
-LogicalResult mlir::convertFromAttribute(int64_t &storage,
- ::mlir::Attribute attr,
- ::mlir::InFlightDiagnostic *diag) {
+LogicalResult
+mlir::convertFromAttribute(int64_t &storage, Attribute attr,
+ function_ref<InFlightDiagnostic &()> getDiag) {
auto valueAttr = dyn_cast<IntegerAttr>(attr);
if (!valueAttr) {
- if (diag)
- *diag << "expected IntegerAttr for key `value`";
+ getDiag() << "expected IntegerAttr for key `value`";
return failure();
}
storage = valueAttr.getValue().getSExtValue();
@@ -35,35 +34,33 @@ Attribute mlir::convertToAttribute(MLIRContext *ctx, int64_t storage) {
}
template <typename DenseArrayTy, typename T>
-LogicalResult convertDenseArrayFromAttr(MutableArrayRef<T> storage,
- ::mlir::Attribute attr,
- ::mlir::InFlightDiagnostic *diag,
- StringRef denseArrayTyStr) {
+LogicalResult
+convertDenseArrayFromAttr(MutableArrayRef<T> storage, Attribute attr,
+ function_ref<InFlightDiagnostic &()> getDiag,
+ StringRef denseArrayTyStr) {
auto valueAttr = dyn_cast<DenseArrayTy>(attr);
if (!valueAttr) {
- if (diag)
- *diag << "expected " << denseArrayTyStr << " for key `value`";
+ getDiag() << "expected " << denseArrayTyStr << " for key `value`";
return failure();
}
if (valueAttr.size() != static_cast<int64_t>(storage.size())) {
- if (diag)
- *diag << "size mismatch in attribute conversion: " << valueAttr.size()
- << " vs " << storage.size();
+ getDiag() << "size mismatch in attribute conversion: " << valueAttr.size()
+ << " vs " << storage.size();
return failure();
}
llvm::copy(valueAttr.asArrayRef(), storage.begin());
return success();
}
-LogicalResult mlir::convertFromAttribute(MutableArrayRef<int64_t> storage,
- ::mlir::Attribute attr,
- ::mlir::InFlightDiagnostic *diag) {
- return convertDenseArrayFromAttr<DenseI64ArrayAttr>(storage, attr, diag,
+LogicalResult
+mlir::convertFromAttribute(MutableArrayRef<int64_t> storage, Attribute attr,
+ function_ref<InFlightDiagnostic &()> getDiag) {
+ return convertDenseArrayFromAttr<DenseI64ArrayAttr>(storage, attr, getDiag,
"DenseI64ArrayAttr");
}
-LogicalResult mlir::convertFromAttribute(MutableArrayRef<int32_t> storage,
- Attribute attr,
- InFlightDiagnostic *diag) {
- return convertDenseArrayFromAttr<DenseI32ArrayAttr>(storage, attr, diag,
+LogicalResult
+mlir::convertFromAttribute(MutableArrayRef<int32_t> storage, Attribute attr,
+ function_ref<InFlightDiagnostic &()> getDiag) {
+ return convertDenseArrayFromAttr<DenseI32ArrayAttr>(storage, attr, getDiag,
"DenseI32ArrayAttr");
}
diff --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp
index ef98a89f4bb49b6..aa577aa089c6860 100644
--- a/mlir/lib/IR/Operation.cpp
+++ b/mlir/lib/IR/Operation.cpp
@@ -351,16 +351,15 @@ Attribute Operation::getPropertiesAsAttribute() {
return *getPropertiesStorage().as<Attribute *>();
return info->getOpPropertiesAsAttribute(this);
}
-LogicalResult
-Operation::setPropertiesFromAttribute(Attribute attr,
- InFlightDiagnostic *diagnostic) {
+LogicalResult Operation::setPropertiesFromAttribute(
+ Attribute attr, function_ref<InFlightDiagnostic &()> getDiag) {
std::optional<RegisteredOperationName> info = getRegisteredInfo();
if (LLVM_UNLIKELY(!info)) {
*getPropertiesStorage().as<Attribute *>() = attr;
return success();
}
return info->setOpPropertiesFromAttribute(
- this->getName(), this->getPropertiesStorage(...
<truncated>
</pre>
</details>
https://github.com/llvm/llvm-project/pull/65868
More information about the Mlir-commits
mailing list