Skip to content

Commit 8c2bff1

Browse files
joker-ephjoker-eph
authored
Lazy initialize diagnostic when handling MLIR properties (#65868)
Instead of eagerly creating a diagnostic that will be discarded in the normal case, switch to lazy initialization on error.
1 parent 16a2aa3 commit 8c2bff1

16 files changed

+155
-138
lines changed

‎mlir/include/mlir/IR/ExtensibleDialect.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -486,10 +486,11 @@ class DynamicOpDefinition : public OperationName::Impl {
486486
void populateDefaultProperties(OperationName opName,
487487
OpaqueProperties properties) final {}
488488

489-
LogicalResult setPropertiesFromAttr(OperationName opName,
490-
OpaqueProperties properties,
491-
Attribute attr,
492-
InFlightDiagnostic *diag) final {
489+
LogicalResult
490+
setPropertiesFromAttr(OperationName opName, OpaqueProperties properties,
491+
Attribute attr,
492+
function_ref<InFlightDiagnostic &()> getDiag) final {
493+
getDiag() << "extensible Dialects don't support properties";
493494
return failure();
494495
}
495496
Attribute getPropertiesAsAttr(Operation *op) final { return {}; }

‎mlir/include/mlir/IR/ODSSupport.h

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
#define MLIR_IR_ODSSUPPORT_H
1515

1616
#include "mlir/IR/Attributes.h"
17+
#include "mlir/IR/MLIRContext.h"
18+
#include "mlir/Support/LLVM.h"
1719

1820
namespace mlir {
1921

@@ -24,8 +26,9 @@ namespace mlir {
2426
/// Convert an IntegerAttr attribute to an int64_t, or return an error if the
2527
/// attribute isn't an IntegerAttr. If the optional diagnostic is provided an
2628
/// error message is also emitted.
27-
LogicalResult convertFromAttribute(int64_t &storage, Attribute attr,
28-
InFlightDiagnostic *diag);
29+
LogicalResult
30+
convertFromAttribute(int64_t &storage, Attribute attr,
31+
function_ref<InFlightDiagnostic &()> getDiag);
2932

3033
/// Convert the provided int64_t to an IntegerAttr attribute.
3134
Attribute convertToAttribute(MLIRContext *ctx, int64_t storage);
@@ -34,15 +37,17 @@ Attribute convertToAttribute(MLIRContext *ctx, int64_t storage);
3437
/// storage has the same size as the array. An error is returned if the
3538
/// attribute isn't a DenseI64ArrayAttr or it does not have the same size. If
3639
/// the optional diagnostic is provided an error message is also emitted.
37-
LogicalResult convertFromAttribute(MutableArrayRef<int64_t> storage,
38-
Attribute attr, InFlightDiagnostic *diag);
40+
LogicalResult
41+
convertFromAttribute(MutableArrayRef<int64_t> storage, Attribute attr,
42+
function_ref<InFlightDiagnostic &()> getDiag);
3943

4044
/// Convert a DenseI32ArrayAttr to the provided storage. It is expected that the
4145
/// storage has the same size as the array. An error is returned if the
4246
/// attribute isn't a DenseI32ArrayAttr or it does not have the same size. If
4347
/// the optional diagnostic is provided an error message is also emitted.
44-
LogicalResult convertFromAttribute(MutableArrayRef<int32_t> storage,
45-
Attribute attr, InFlightDiagnostic *diag);
48+
LogicalResult
49+
convertFromAttribute(MutableArrayRef<int32_t> storage, Attribute attr,
50+
function_ref<InFlightDiagnostic &()> getDiag);
4651

4752
/// Convert the provided ArrayRef<int64_t> to a DenseI64ArrayAttr attribute.
4853
Attribute convertToAttribute(MLIRContext *ctx, ArrayRef<int64_t> storage);

‎mlir/include/mlir/IR/OpDefinition.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1769,9 +1769,10 @@ class Op : public OpState, public Traits<ConcreteType>... {
17691769
/// the namespace where the properties are defined. It can also be overridden
17701770
/// in the derived ConcreteOp.
17711771
template <typename PropertiesTy>
1772-
static LogicalResult setPropertiesFromAttr(PropertiesTy &prop, Attribute attr,
1773-
InFlightDiagnostic *diag) {
1774-
return setPropertiesFromAttribute(prop, attr, diag);
1772+
static LogicalResult
1773+
setPropertiesFromAttr(PropertiesTy &prop, Attribute attr,
1774+
function_ref<InFlightDiagnostic &()> getDiag) {
1775+
return setPropertiesFromAttribute(prop, attr, getDiag);
17751776
}
17761777
/// Convert the provided properties to an attribute. This default
17771778
/// implementation forwards to a free function `getPropertiesAsAttribute` that

‎mlir/include/mlir/IR/Operation.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -882,8 +882,9 @@ class alignas(8) Operation final
882882
/// matching the expectations of the properties for this operation. This is
883883
/// mostly useful for unregistered operations or used when parsing the
884884
/// generic format. An optional diagnostic can be passed in for richer errors.
885-
LogicalResult setPropertiesFromAttribute(Attribute attr,
886-
InFlightDiagnostic *diagnostic);
885+
LogicalResult
886+
setPropertiesFromAttribute(Attribute attr,
887+
function_ref<InFlightDiagnostic &()> getDiag);
887888

888889
/// Copy properties from an existing other properties object. The two objects
889890
/// must be the same type.

‎mlir/include/mlir/IR/OperationSupport.h

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,9 @@ class OperationName {
136136
virtual void deleteProperties(OpaqueProperties) = 0;
137137
virtual void populateDefaultProperties(OperationName opName,
138138
OpaqueProperties properties) = 0;
139-
virtual LogicalResult setPropertiesFromAttr(OperationName, OpaqueProperties,
140-
Attribute,
141-
InFlightDiagnostic *) = 0;
139+
virtual LogicalResult
140+
setPropertiesFromAttr(OperationName, OpaqueProperties, Attribute,
141+
function_ref<InFlightDiagnostic &()> getDiag) = 0;
142142
virtual Attribute getPropertiesAsAttr(Operation *) = 0;
143143
virtual void copyProperties(OpaqueProperties, OpaqueProperties) = 0;
144144
virtual llvm::hash_code hashProperties(OpaqueProperties) = 0;
@@ -216,8 +216,9 @@ class OperationName {
216216
void deleteProperties(OpaqueProperties) final;
217217
void populateDefaultProperties(OperationName opName,
218218
OpaqueProperties properties) final;
219-
LogicalResult setPropertiesFromAttr(OperationName, OpaqueProperties,
220-
Attribute, InFlightDiagnostic *) final;
219+
LogicalResult
220+
setPropertiesFromAttr(OperationName, OpaqueProperties, Attribute,
221+
function_ref<InFlightDiagnostic &()> getDiag) final;
221222
Attribute getPropertiesAsAttr(Operation *) final;
222223
void copyProperties(OpaqueProperties, OpaqueProperties) final;
223224
llvm::hash_code hashProperties(OpaqueProperties) final;
@@ -434,12 +435,10 @@ class OperationName {
434435
}
435436

436437
/// Define the op properties from the provided Attribute.
437-
LogicalResult
438-
setOpPropertiesFromAttribute(OperationName opName,
439-
OpaqueProperties properties, Attribute attr,
440-
InFlightDiagnostic *diagnostic) const {
441-
return getImpl()->setPropertiesFromAttr(opName, properties, attr,
442-
diagnostic);
438+
LogicalResult setOpPropertiesFromAttribute(
439+
OperationName opName, OpaqueProperties properties, Attribute attr,
440+
function_ref<InFlightDiagnostic &()> getDiag) const {
441+
return getImpl()->setPropertiesFromAttr(opName, properties, attr, getDiag);
443442
}
444443

445444
void copyOpProperties(OpaqueProperties lhs, OpaqueProperties rhs) const {
@@ -628,16 +627,15 @@ class RegisteredOperationName : public OperationName {
628627
*properties.as<Properties *>());
629628
}
630629

631-
LogicalResult setPropertiesFromAttr(OperationName opName,
632-
OpaqueProperties properties,
633-
Attribute attr,
634-
InFlightDiagnostic *diag) final {
630+
LogicalResult
631+
setPropertiesFromAttr(OperationName opName, OpaqueProperties properties,
632+
Attribute attr,
633+
function_ref<InFlightDiagnostic &()> getDiag) final {
635634
if constexpr (hasProperties) {
636635
auto p = properties.as<Properties *>();
637-
return ConcreteOp::setPropertiesFromAttr(*p, attr, diag);
636+
return ConcreteOp::setPropertiesFromAttr(*p, attr, getDiag);
638637
}
639-
if (diag)
640-
*diag << "This operation does not support properties";
638+
getDiag() << "this operation does not support properties";
641639
return failure();
642640
}
643641
Attribute getPropertiesAsAttr(Operation *op) final {
@@ -997,8 +995,9 @@ struct OperationState {
997995

998996
// Set the properties defined on this OpState on the given operation,
999997
// optionally emit diagnostics on error through the provided diagnostic.
1000-
LogicalResult setProperties(Operation *op,
1001-
InFlightDiagnostic *diagnostic) const;
998+
LogicalResult
999+
setProperties(Operation *op,
1000+
function_ref<InFlightDiagnostic &()> getDiag) const;
10021001

10031002
void addOperands(ValueRange newOperands);
10041003

‎mlir/include/mlir/IR/Properties.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class Property<string storageTypeParam = "", string desc = ""> {
5555
// Format:
5656
// - `$_storage` is the storage type value.
5757
// - `$_attr` is the attribute.
58-
// - `$_diag` is an optional Diagnostic pointer to emit error.
58+
// - `$_diag` is a callback to get a Diagnostic to emit error.
5959
//
6060
// The expression must return a LogicalResult
6161
code convertFromAttribute = [{

‎mlir/lib/AsmParser/Parser.cpp

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@
1818
#include "mlir/IR/AffineMap.h"
1919
#include "mlir/IR/AsmState.h"
2020
#include "mlir/IR/BuiltinOps.h"
21+
#include "mlir/IR/Diagnostics.h"
2122
#include "mlir/IR/Dialect.h"
2223
#include "mlir/IR/Verifier.h"
24+
#include "mlir/Support/InterfaceSupport.h"
2325
#include "llvm/ADT/DenseMap.h"
2426
#include "llvm/ADT/ScopeExit.h"
2527
#include "llvm/ADT/StringSet.h"
@@ -29,6 +31,7 @@
2931
#include "llvm/Support/PrettyStackTrace.h"
3032
#include "llvm/Support/SourceMgr.h"
3133
#include <algorithm>
34+
#include <memory>
3235
#include <optional>
3336

3437
using namespace mlir;
@@ -1443,12 +1446,17 @@ Operation *OperationParser::parseGenericOperation() {
14431446
// Try setting the properties for the operation, using a diagnostic to print
14441447
// errors.
14451448
if (properties) {
1446-
InFlightDiagnostic diagnostic =
1447-
mlir::emitError(srcLocation, "invalid properties ")
1448-
<< properties << " for op " << name << ": ";
1449-
if (failed(op->setPropertiesFromAttribute(properties, &diagnostic)))
1449+
std::unique_ptr<InFlightDiagnostic> diagnostic;
1450+
auto getDiag = [&]() -> InFlightDiagnostic & {
1451+
if (!diagnostic) {
1452+
diagnostic = std::make_unique<InFlightDiagnostic>(
1453+
mlir::emitError(srcLocation, "invalid properties ")
1454+
<< properties << " for op " << name << ": ");
1455+
}
1456+
return *diagnostic;
1457+
};
1458+
if (failed(op->setPropertiesFromAttribute(properties, getDiag)))
14501459
return nullptr;
1451-
diagnostic.abandon();
14521460
}
14531461

14541462
return op;
@@ -2001,12 +2009,18 @@ OperationParser::parseCustomOperation(ArrayRef<ResultRecord> resultIDs) {
20012009

20022010
// Try setting the properties for the operation.
20032011
if (properties) {
2004-
InFlightDiagnostic diagnostic =
2005-
mlir::emitError(srcLocation, "invalid properties ")
2006-
<< properties << " for op " << op->getName().getStringRef() << ": ";
2007-
if (failed(op->setPropertiesFromAttribute(properties, &diagnostic)))
2012+
std::unique_ptr<InFlightDiagnostic> diagnostic;
2013+
auto getDiag = [&]() -> InFlightDiagnostic & {
2014+
if (!diagnostic) {
2015+
diagnostic = std::make_unique<InFlightDiagnostic>(
2016+
mlir::emitError(srcLocation, "invalid properties ")
2017+
<< properties << " for op " << op->getName().getStringRef()
2018+
<< ": ");
2019+
}
2020+
return *diagnostic;
2021+
};
2022+
if (failed(op->setPropertiesFromAttribute(properties, getDiag)))
20082023
return nullptr;
2009-
diagnostic.abandon();
20102024
}
20112025
return op;
20122026
}

‎mlir/lib/CAPI/IR/IR.cpp

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -370,16 +370,21 @@ static LogicalResult inferOperationTypes(OperationState &state) {
370370
if (!properties && info->getOpPropertyByteSize() > 0 && !attributes.empty()) {
371371
auto prop = std::make_unique<char[]>(info->getOpPropertyByteSize());
372372
properties = OpaqueProperties(prop.get());
373-
InFlightDiagnostic diag = emitError(state.location)
374-
<< " failed properties conversion while building "
375-
<< state.name.getStringRef() << " with `"
376-
<< attributes << "`: ";
377-
if (failed(info->setOpPropertiesFromAttribute(state.name, properties,
378-
attributes, &diag))) {
379-
return failure();
373+
if (properties) {
374+
std::unique_ptr<InFlightDiagnostic> diagnostic;
375+
auto getDiag = [&]() -> InFlightDiagnostic & {
376+
if (!diagnostic) {
377+
diagnostic = std::make_unique<InFlightDiagnostic>(
378+
emitError(state.location)
379+
<< " failed properties conversion while building "
380+
<< state.name.getStringRef() << " with `" << attributes << "`: ");
381+
}
382+
return *diagnostic;
383+
};
384+
if (failed(info->setOpPropertiesFromAttribute(state.name, properties,
385+
attributes, getDiag)))
386+
return failure();
380387
}
381-
diag.abandon();
382-
383388
if (succeeded(inferInterface->inferReturnTypes(
384389
context, state.location, state.operands, attributes, properties,
385390
state.regions, state.types))) {

‎mlir/lib/IR/MLIRContext.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -852,7 +852,7 @@ void OperationName::UnregisteredOpModel::populateDefaultProperties(
852852
OperationName opName, OpaqueProperties properties) {}
853853
LogicalResult OperationName::UnregisteredOpModel::setPropertiesFromAttr(
854854
OperationName opName, OpaqueProperties properties, Attribute attr,
855-
InFlightDiagnostic *diag) {
855+
function_ref<InFlightDiagnostic &()> getDiag) {
856856
*properties.as<Attribute *>() = attr;
857857
return success();
858858
}

‎mlir/lib/IR/ODSSupport.cpp

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,12 @@
1818

1919
using namespace mlir;
2020

21-
LogicalResult mlir::convertFromAttribute(int64_t &storage,
22-
::mlir::Attribute attr,
23-
::mlir::InFlightDiagnostic *diag) {
21+
LogicalResult
22+
mlir::convertFromAttribute(int64_t &storage, Attribute attr,
23+
function_ref<InFlightDiagnostic &()> getDiag) {
2424
auto valueAttr = dyn_cast<IntegerAttr>(attr);
2525
if (!valueAttr) {
26-
if (diag)
27-
*diag << "expected IntegerAttr for key `value`";
26+
getDiag() << "expected IntegerAttr for key `value`";
2827
return failure();
2928
}
3029
storage = valueAttr.getValue().getSExtValue();
@@ -35,35 +34,33 @@ Attribute mlir::convertToAttribute(MLIRContext *ctx, int64_t storage) {
3534
}
3635

3736
template <typename DenseArrayTy, typename T>
38-
LogicalResult convertDenseArrayFromAttr(MutableArrayRef<T> storage,
39-
::mlir::Attribute attr,
40-
::mlir::InFlightDiagnostic *diag,
41-
StringRef denseArrayTyStr) {
37+
LogicalResult
38+
convertDenseArrayFromAttr(MutableArrayRef<T> storage, Attribute attr,
39+
function_ref<InFlightDiagnostic &()> getDiag,
40+
StringRef denseArrayTyStr) {
4241
auto valueAttr = dyn_cast<DenseArrayTy>(attr);
4342
if (!valueAttr) {
44-
if (diag)
45-
*diag << "expected " << denseArrayTyStr << " for key `value`";
43+
getDiag() << "expected " << denseArrayTyStr << " for key `value`";
4644
return failure();
4745
}
4846
if (valueAttr.size() != static_cast<int64_t>(storage.size())) {
49-
if (diag)
50-
*diag << "size mismatch in attribute conversion: " << valueAttr.size()
51-
<< " vs " << storage.size();
47+
getDiag() << "size mismatch in attribute conversion: " << valueAttr.size()
48+
<< " vs " << storage.size();
5249
return failure();
5350
}
5451
llvm::copy(valueAttr.asArrayRef(), storage.begin());
5552
return success();
5653
}
57-
LogicalResult mlir::convertFromAttribute(MutableArrayRef<int64_t> storage,
58-
::mlir::Attribute attr,
59-
::mlir::InFlightDiagnostic *diag) {
60-
return convertDenseArrayFromAttr<DenseI64ArrayAttr>(storage, attr, diag,
54+
LogicalResult
55+
mlir::convertFromAttribute(MutableArrayRef<int64_t> storage, Attribute attr,
56+
function_ref<InFlightDiagnostic &()> getDiag) {
57+
return convertDenseArrayFromAttr<DenseI64ArrayAttr>(storage, attr, getDiag,
6158
"DenseI64ArrayAttr");
6259
}
63-
LogicalResult mlir::convertFromAttribute(MutableArrayRef<int32_t> storage,
64-
Attribute attr,
65-
InFlightDiagnostic *diag) {
66-
return convertDenseArrayFromAttr<DenseI32ArrayAttr>(storage, attr, diag,
60+
LogicalResult
61+
mlir::convertFromAttribute(MutableArrayRef<int32_t> storage, Attribute attr,
62+
function_ref<InFlightDiagnostic &()> getDiag) {
63+
return convertDenseArrayFromAttr<DenseI32ArrayAttr>(storage, attr, getDiag,
6764
"DenseI32ArrayAttr");
6865
}
6966

‎mlir/lib/IR/Operation.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -351,16 +351,15 @@ Attribute Operation::getPropertiesAsAttribute() {
351351
return *getPropertiesStorage().as<Attribute *>();
352352
return info->getOpPropertiesAsAttribute(this);
353353
}
354-
LogicalResult
355-
Operation::setPropertiesFromAttribute(Attribute attr,
356-
InFlightDiagnostic *diagnostic) {
354+
LogicalResult Operation::setPropertiesFromAttribute(
355+
Attribute attr, function_ref<InFlightDiagnostic &()> getDiag) {
357356
std::optional<RegisteredOperationName> info = getRegisteredInfo();
358357
if (LLVM_UNLIKELY(!info)) {
359358
*getPropertiesStorage().as<Attribute *>() = attr;
360359
return success();
361360
}
362361
return info->setOpPropertiesFromAttribute(
363-
this->getName(), this->getPropertiesStorage(), attr, diagnostic);
362+
this->getName(), this->getPropertiesStorage(), attr, getDiag);
364363
}
365364

366365
void Operation::copyProperties(OpaqueProperties rhs) {

‎mlir/lib/IR/OperationSupport.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -198,12 +198,11 @@ OperationState::~OperationState() {
198198
propertiesDeleter(properties);
199199
}
200200

201-
LogicalResult
202-
OperationState::setProperties(Operation *op,
203-
InFlightDiagnostic *diagnostic) const {
201+
LogicalResult OperationState::setProperties(
202+
Operation *op, function_ref<InFlightDiagnostic &()> getDiag) const {
204203
if (LLVM_UNLIKELY(propertiesAttr)) {
205204
assert(!properties);
206-
return op->setPropertiesFromAttribute(propertiesAttr, diagnostic);
205+
return op->setPropertiesFromAttribute(propertiesAttr, getDiag);
207206
}
208207
if (properties)
209208
propertiesSetter(op->getPropertiesStorage(), properties);

0 commit comments

Comments
 (0)