Skip to content

Commit 8b42200

Browse files
jpienaarjpienaar
authored
[mlir][ods] Enable basic string interpolation in constraint summary. (#153603)
This enables printing, for example, the attribute value from a mismatched predicate. Example of resultant output (here made non-negative report value seen as sign-extended int): ``` PDL/ops.mlir:21:1: error: 'pdl.pattern' op attribute 'benefit' failed to satisfy constraint: 16-bit signless integer attribute whose value is non-negative (got -31) pdl.pattern @rewrite_with_args : benefit(-31) { ^ ``` This is primarily the mechanism and didn't change any existing constraints. I also attempted to keep the error format as close to the original as possible - but did notice 2 errors that were inconsistent with the rest and updated them to be consistent.
1 parent fa83723 commit 8b42200

File tree

8 files changed

+150
-48
lines changed

8 files changed

+150
-48
lines changed

‎mlir/include/mlir/TableGen/CodeGenHelpers.h‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,15 @@ class DialectNamespaceEmitter {
5252
std::optional<llvm::NamespaceEmitter> nsEmitter;
5353
};
5454

55+
/// This class represents how an error stream string being constructed will be
56+
/// consumed.
57+
enum class ErrorStreamType {
58+
// Inside a string that's streamed into an InflightDiagnostic.
59+
InString,
60+
// Inside a string inside an OpError.
61+
InsideOpError,
62+
};
63+
5564
/// This class deduplicates shared operation verification code by emitting
5665
/// static functions alongside the op definitions. These methods are local to
5766
/// the definition file, and are invoked within the operation verify methods.
@@ -192,7 +201,8 @@ class StaticVerifierFunctionEmitter {
192201

193202
/// A generic function to emit constraints
194203
void emitConstraints(const ConstraintMap &constraints, StringRef selfName,
195-
const char *codeTemplate);
204+
const char *codeTemplate,
205+
ErrorStreamType errorStreamType);
196206

197207
/// Assign a unique name to a unique constraint.
198208
std::string getUniqueName(StringRef kind, unsigned index);
@@ -243,6 +253,18 @@ std::string stringify(T &&t) {
243253
apply(std::forward<T>(t));
244254
}
245255

256+
/// Helper to generate a C++ streaming error message from a given message.
257+
/// Message can contain '{{...}}' placeholders that are substituted with
258+
/// C-expressions via tgfmt. It would effectively convert:
259+
/// "failed to verify {{foo}}"
260+
/// into:
261+
/// "failed to verify " << bar
262+
/// where bar is the result of evaluating 'tgfmt("foo", &ctx)' at compile
263+
/// time.
264+
std::string buildErrorStreamingString(
265+
StringRef message, const FmtContext &ctx,
266+
ErrorStreamType errorStreamType = ErrorStreamType::InString);
267+
246268
} // namespace tblgen
247269
} // namespace mlir
248270

‎mlir/lib/TableGen/CodeGenHelpers.cpp‎

Lines changed: 79 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,26 @@
1212
//===----------------------------------------------------------------------===//
1313

1414
#include "mlir/TableGen/CodeGenHelpers.h"
15+
#include "mlir/Support/LLVM.h"
16+
#include "mlir/TableGen/Argument.h"
17+
#include "mlir/TableGen/Attribute.h"
18+
#include "mlir/TableGen/Format.h"
1519
#include "mlir/TableGen/Operator.h"
1620
#include "mlir/TableGen/Pattern.h"
21+
#include "mlir/TableGen/Property.h"
22+
#include "mlir/TableGen/Region.h"
23+
#include "mlir/TableGen/Successor.h"
24+
#include "llvm/ADT/StringExtras.h"
25+
#include "llvm/ADT/StringRef.h"
1726
#include "llvm/Support/FormatVariadic.h"
1827
#include "llvm/Support/Path.h"
28+
#include "llvm/Support/raw_ostream.h"
1929
#include "llvm/TableGen/CodeGenHelpers.h"
30+
#include "llvm/TableGen/Error.h"
2031
#include "llvm/TableGen/Record.h"
32+
#include <cassert>
33+
#include <optional>
34+
#include <string>
2135

2236
using namespace llvm;
2337
using namespace mlir;
@@ -112,6 +126,55 @@ StringRef StaticVerifierFunctionEmitter::getRegionConstraintFn(
112126
// Constraint Emission
113127
//===----------------------------------------------------------------------===//
114128

129+
/// Helper to generate a C++ string expression from a given message.
130+
/// Message can contain '{{...}}' placeholders that are substituted with
131+
/// C-expressions via tgfmt.
132+
std::string mlir::tblgen::buildErrorStreamingString(
133+
StringRef message, const FmtContext &ctx, ErrorStreamType errorStreamType) {
134+
std::string result;
135+
raw_string_ostream os(result);
136+
137+
std::string msgStr = escapeString(message);
138+
StringRef msg = msgStr;
139+
140+
// Split the message by '{{' and '}}' and build a streaming expression.
141+
auto split = msg.split("{{");
142+
os << split.first;
143+
if (split.second.empty()) {
144+
return msgStr;
145+
}
146+
147+
if (errorStreamType == ErrorStreamType::InsideOpError)
148+
os << "\")";
149+
else
150+
os << '"';
151+
152+
msg = split.second;
153+
while (!msg.empty()) {
154+
split = msg.split("}}");
155+
StringRef var = split.first;
156+
StringRef rest = split.second;
157+
158+
os << " << " << tgfmt(var, &ctx);
159+
160+
if (rest.empty())
161+
break;
162+
163+
split = rest.split("{{");
164+
if (split.second.empty() &&
165+
errorStreamType == ErrorStreamType::InsideOpError) {
166+
// To enable having part of string post, this adds a parenthesis before
167+
// the last string segment to match the existing one.
168+
os << " << (\"" << split.first;
169+
} else {
170+
os << " << \"" << split.first;
171+
}
172+
msg = split.second;
173+
}
174+
175+
return os.str();
176+
}
177+
115178
/// Code templates for emitting type, attribute, successor, and region
116179
/// constraints. Each of these templates require the following arguments:
117180
///
@@ -224,22 +287,24 @@ static ::llvm::LogicalResult {0}(
224287

225288
void StaticVerifierFunctionEmitter::emitConstraints(
226289
const ConstraintMap &constraints, StringRef selfName,
227-
const char *const codeTemplate) {
290+
const char *const codeTemplate, ErrorStreamType errorStreamType) {
228291
FmtContext ctx;
229292
ctx.addSubst("_op", "*op").withSelf(selfName);
293+
230294
for (auto &it : constraints) {
231295
os << formatv(codeTemplate, it.second,
232296
tgfmt(it.first.getConditionTemplate(), &ctx),
233-
escapeString(it.first.getSummary()));
297+
buildErrorStreamingString(it.first.getSummary(), ctx));
234298
}
235299
}
236-
237300
void StaticVerifierFunctionEmitter::emitTypeConstraints() {
238-
emitConstraints(typeConstraints, "type", typeConstraintCode);
301+
emitConstraints(typeConstraints, "type", typeConstraintCode,
302+
ErrorStreamType::InString);
239303
}
240304

241305
void StaticVerifierFunctionEmitter::emitAttrConstraints() {
242-
emitConstraints(attrConstraints, "attr", attrConstraintCode);
306+
emitConstraints(attrConstraints, "attr", attrConstraintCode,
307+
ErrorStreamType::InString);
243308
}
244309

245310
/// Unlike with the other helpers, this one has to substitute in the interface
@@ -251,17 +316,19 @@ void StaticVerifierFunctionEmitter::emitPropConstraints() {
251316
auto propConstraint = cast<PropConstraint>(it.first);
252317
os << formatv(propConstraintCode, it.second,
253318
tgfmt(propConstraint.getConditionTemplate(), &ctx),
254-
escapeString(it.first.getSummary()),
319+
buildErrorStreamingString(it.first.getSummary(), ctx),
255320
propConstraint.getInterfaceType());
256321
}
257322
}
258323

259324
void StaticVerifierFunctionEmitter::emitSuccessorConstraints() {
260-
emitConstraints(successorConstraints, "successor", successorConstraintCode);
325+
emitConstraints(successorConstraints, "successor", successorConstraintCode,
326+
ErrorStreamType::InString);
261327
}
262328

263329
void StaticVerifierFunctionEmitter::emitRegionConstraints() {
264-
emitConstraints(regionConstraints, "region", regionConstraintCode);
330+
emitConstraints(regionConstraints, "region", regionConstraintCode,
331+
ErrorStreamType::InString);
265332
}
266333

267334
void StaticVerifierFunctionEmitter::emitPatternConstraints() {
@@ -270,13 +337,14 @@ void StaticVerifierFunctionEmitter::emitPatternConstraints() {
270337
for (auto &it : typeConstraints) {
271338
os << formatv(patternConstraintCode, it.second,
272339
tgfmt(it.first.getConditionTemplate(), &ctx),
273-
escapeString(it.first.getSummary()), "::mlir::Type type");
340+
buildErrorStreamingString(it.first.getSummary(), ctx),
341+
"::mlir::Type type");
274342
}
275343
ctx.withSelf("attr");
276344
for (auto &it : attrConstraints) {
277345
os << formatv(patternConstraintCode, it.second,
278346
tgfmt(it.first.getConditionTemplate(), &ctx),
279-
escapeString(it.first.getSummary()),
347+
buildErrorStreamingString(it.first.getSummary(), ctx),
280348
"::mlir::Attribute attr");
281349
}
282350
ctx.withSelf("prop");
@@ -291,7 +359,7 @@ void StaticVerifierFunctionEmitter::emitPatternConstraints() {
291359
}
292360
os << formatv(patternConstraintCode, it.second,
293361
tgfmt(propConstraint.getConditionTemplate(), &ctx),
294-
escapeString(propConstraint.getSummary()),
362+
buildErrorStreamingString(propConstraint.getSummary(), ctx),
295363
Twine(interfaceType) + " prop");
296364
}
297365
}

‎mlir/test/mlir-tblgen/constraint-unique.td‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,15 @@ def AType : Type<ATypePred, "a type">;
1616
def OtherType : Type<ATypePred, "another type">;
1717

1818
def AnAttrPred : CPred<"attrPred($_self, $_op)">;
19-
def AnAttr : Attr<AnAttrPred, "an attribute">;
19+
def AnAttr : Attr<AnAttrPred, "an attribute (got {{reformat($_self)}})">;
2020
def OtherAttr : Attr<AnAttrPred, "another attribute">;
2121

2222
def ASuccessorPred : CPred<"successorPred($_self, $_op)">;
2323
def ASuccessor : Successor<ASuccessorPred, "a successor">;
2424
def OtherSuccessor : Successor<ASuccessorPred, "another successor">;
2525

2626
def ARegionPred : CPred<"regionPred($_self, $_op)">;
27-
def ARegion : Region<ARegionPred, "a region">;
27+
def ARegion : Region<ARegionPred, "a region ({{find(foo)}})">;
2828
def OtherRegion : Region<ARegionPred, "another region">;
2929

3030
// OpA and OpB have the same type, attribute, successor, and region constraints.
@@ -71,10 +71,10 @@ def OpC : NS_Op<"op_c"> {
7171
// CHECK: static ::llvm::LogicalResult [[$A_ATTR_CONSTRAINT:__mlir_ods_local_attr_constraint.*]](
7272
// CHECK: if (attr && !((attrPred(attr, *op))))
7373
// CHECK-NEXT: return emitError() << "attribute '" << attrName
74-
// CHECK-NEXT: << "' failed to satisfy constraint: an attribute";
74+
// CHECK-NEXT: << "' failed to satisfy constraint: an attribute (got " << reformat(attr) << ")";
7575

7676
/// Test that duplicate attribute constraint was not generated.
77-
// CHECK-NOT: << "' failed to satisfy constraint: an attribute";
77+
// CHECK-NOT: << "' failed to satisfy constraint: an attribute
7878

7979
/// Test that a attribute constraint with a different description was generated.
8080
// CHECK: static ::llvm::LogicalResult [[$O_ATTR_CONSTRAINT:__mlir_ods_local_attr_constraint.*]](
@@ -103,7 +103,7 @@ def OpC : NS_Op<"op_c"> {
103103
// CHECK: if (!((regionPred(region, *op)))) {
104104
// CHECK-NEXT: return op->emitOpError("region #") << regionIndex
105105
// CHECK-NEXT: << (regionName.empty() ? " " : " ('" + regionName + "') ")
106-
// CHECK-NEXT: << "failed to verify constraint: a region";
106+
// CHECK-NEXT: << "failed to verify constraint: a region (" << find(foo) << ")";
107107

108108
/// Test that duplicate region constraint was not generated.
109109
// CHECK-NOT: << "failed to verify constraint: a region";

‎mlir/test/mlir-tblgen/op-attribute.td‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,19 +69,19 @@ def AOp : NS_Op<"a_op", []> {
6969

7070
// DEF: ::llvm::LogicalResult AOpAdaptor::verify
7171
// DEF-NEXT: auto tblgen_aAttr = getProperties().aAttr; (void)tblgen_aAttr;
72-
// DEF-NEXT: if (!tblgen_aAttr) return emitError(loc, "'test.a_op' op ""requires attribute 'aAttr'");
72+
// DEF-NEXT: if (!tblgen_aAttr) return emitError(loc, "'test.a_op' op requires attribute 'aAttr'");
7373
// DEF-NEXT: auto tblgen_bAttr = getProperties().bAttr; (void)tblgen_bAttr;
7474
// DEF-NEXT: auto tblgen_cAttr = getProperties().cAttr; (void)tblgen_cAttr;
7575
// DEF-NEXT: auto tblgen_dAttr = getProperties().dAttr; (void)tblgen_dAttr;
7676

7777
// DEF: if (tblgen_aAttr && !((some-condition)))
78-
// DEF-NEXT: return emitError(loc, "'test.a_op' op ""attribute 'aAttr' failed to satisfy constraint: some attribute kind");
78+
// DEF-NEXT: return emitError(loc, "'test.a_op' op attribute 'aAttr' failed to satisfy constraint: some attribute kind");
7979
// DEF: if (tblgen_bAttr && !((some-condition)))
80-
// DEF-NEXT: return emitError(loc, "'test.a_op' op ""attribute 'bAttr' failed to satisfy constraint: some attribute kind");
80+
// DEF-NEXT: return emitError(loc, "'test.a_op' op attribute 'bAttr' failed to satisfy constraint: some attribute kind");
8181
// DEF: if (tblgen_cAttr && !((some-condition)))
82-
// DEF-NEXT: return emitError(loc, "'test.a_op' op ""attribute 'cAttr' failed to satisfy constraint: some attribute kind");
82+
// DEF-NEXT: return emitError(loc, "'test.a_op' op attribute 'cAttr' failed to satisfy constraint: some attribute kind");
8383
// DEF: if (tblgen_dAttr && !((some-condition)))
84-
// DEF-NEXT: return emitError(loc, "'test.a_op' op ""attribute 'dAttr' failed to satisfy constraint: some attribute kind");
84+
// DEF-NEXT: return emitError(loc, "'test.a_op' op attribute 'dAttr' failed to satisfy constraint: some attribute kind");
8585

8686
// Test getter methods
8787
// ---
@@ -219,13 +219,13 @@ def AgetOp : Op<Test2_Dialect, "a_get_op", []> {
219219

220220
// DEF: ::llvm::LogicalResult AgetOpAdaptor::verify
221221
// DEF: auto tblgen_aAttr = getProperties().aAttr; (void)tblgen_aAttr;
222-
// DEF: if (!tblgen_aAttr) return emitError(loc, "'test2.a_get_op' op ""requires attribute 'aAttr'");
222+
// DEF: if (!tblgen_aAttr) return emitError(loc, "'test2.a_get_op' op requires attribute 'aAttr'");
223223
// DEF: auto tblgen_bAttr = getProperties().bAttr; (void)tblgen_bAttr;
224224
// DEF: auto tblgen_cAttr = getProperties().cAttr; (void)tblgen_cAttr;
225225
// DEF: if (tblgen_bAttr && !((some-condition)))
226-
// DEF-NEXT: return emitError(loc, "'test2.a_get_op' op ""attribute 'bAttr' failed to satisfy constraint: some attribute kind");
226+
// DEF-NEXT: return emitError(loc, "'test2.a_get_op' op attribute 'bAttr' failed to satisfy constraint: some attribute kind");
227227
// DEF: if (tblgen_cAttr && !((some-condition)))
228-
// DEF-NEXT: return emitError(loc, "'test2.a_get_op' op ""attribute 'cAttr' failed to satisfy constraint: some attribute kind");
228+
// DEF-NEXT: return emitError(loc, "'test2.a_get_op' op attribute 'cAttr' failed to satisfy constraint: some attribute kind");
229229

230230
// Test getter methods
231231
// ---

‎mlir/test/mlir-tblgen/op-properties-predicates.td‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ def OpWithPredicates : NS_Op<"op_with_predicates"> {
7474
// Note: comprehensive emission of verifiers is tested in verifyINvariantsImpl() below
7575
// CHECK: int64_t tblgen_scalar = this->getScalar();
7676
// CHECK: if (!((tblgen_scalar >= 0)))
77-
// CHECK: return emitError(loc, "'test.op_with_predicates' op ""property 'scalar' failed to satisfy constraint: non-negative int64_t");
77+
// CHECK: return emitError(loc, "'test.op_with_predicates' op property 'scalar' failed to satisfy constraint: non-negative int64_t");
7878

7979
// CHECK-LABEL: OpWithPredicates::verifyInvariantsImpl()
8080
// Note: for test readability, we capture [[maybe_unused]] into the variable maybe_unused

‎mlir/test/mlir-tblgen/predicate.td‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,55 +55,55 @@ def OpF : NS_Op<"op_for_int_min_val", []> {
5555

5656
// CHECK-LABEL: OpFAdaptor::verify
5757
// CHECK: (::llvm::cast<::mlir::IntegerAttr>(tblgen_attr).getInt() >= 10)
58-
// CHECK-NEXT: "attribute 'attr' failed to satisfy constraint: 32-bit signless integer attribute whose minimum value is 10"
58+
// CHECK-NEXT: attribute 'attr' failed to satisfy constraint: 32-bit signless integer attribute whose minimum value is 10"
5959

6060
def OpFX : NS_Op<"op_for_int_max_val", []> {
6161
let arguments = (ins ConfinedAttr<I32Attr, [IntMaxValue<10>]>:$attr);
6262
}
6363

6464
// CHECK-LABEL: OpFXAdaptor::verify
6565
// CHECK: (::llvm::cast<::mlir::IntegerAttr>(tblgen_attr).getInt() <= 10)
66-
// CHECK-NEXT: "attribute 'attr' failed to satisfy constraint: 32-bit signless integer attribute whose maximum value is 10"
66+
// CHECK-NEXT: attribute 'attr' failed to satisfy constraint: 32-bit signless integer attribute whose maximum value is 10"
6767

6868
def OpG : NS_Op<"op_for_arr_min_count", []> {
6969
let arguments = (ins ConfinedAttr<ArrayAttr, [ArrayMinCount<8>]>:$attr);
7070
}
7171

7272
// CHECK-LABEL: OpGAdaptor::verify
7373
// CHECK: (::llvm::cast<::mlir::ArrayAttr>(tblgen_attr).size() >= 8)
74-
// CHECK-NEXT: "attribute 'attr' failed to satisfy constraint: array attribute with at least 8 elements"
74+
// CHECK-NEXT: attribute 'attr' failed to satisfy constraint: array attribute with at least 8 elements"
7575

7676
def OpH : NS_Op<"op_for_arr_value_at_index", []> {
7777
let arguments = (ins ConfinedAttr<ArrayAttr, [IntArrayNthElemEq<0, 8>]>:$attr);
7878
}
7979

8080
// CHECK-LABEL: OpHAdaptor::verify
8181
// CHECK: (((::llvm::cast<::mlir::ArrayAttr>(tblgen_attr).size() > 0)) && ((::llvm::cast<::mlir::IntegerAttr>(::llvm::cast<::mlir::ArrayAttr>(tblgen_attr)[0]).getInt() == 8)))))
82-
// CHECK-NEXT: "attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be 8"
82+
// CHECK-NEXT: attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be 8"
8383

8484
def OpI: NS_Op<"op_for_arr_min_value_at_index", []> {
8585
let arguments = (ins ConfinedAttr<ArrayAttr, [IntArrayNthElemMinValue<0, 8>]>:$attr);
8686
}
8787

8888
// CHECK-LABEL: OpIAdaptor::verify
8989
// CHECK: (((::llvm::cast<::mlir::ArrayAttr>(tblgen_attr).size() > 0)) && ((::llvm::cast<::mlir::IntegerAttr>(::llvm::cast<::mlir::ArrayAttr>(tblgen_attr)[0]).getInt() >= 8)))))
90-
// CHECK-NEXT: "attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be at least 8"
90+
// CHECK-NEXT: attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be at least 8"
9191

9292
def OpJ: NS_Op<"op_for_arr_max_value_at_index", []> {
9393
let arguments = (ins ConfinedAttr<ArrayAttr, [IntArrayNthElemMaxValue<0, 8>]>:$attr);
9494
}
9595

9696
// CHECK-LABEL: OpJAdaptor::verify
9797
// CHECK: (((::llvm::cast<::mlir::ArrayAttr>(tblgen_attr).size() > 0)) && ((::llvm::cast<::mlir::IntegerAttr>(::llvm::cast<::mlir::ArrayAttr>(tblgen_attr)[0]).getInt() <= 8)))))
98-
// CHECK-NEXT: "attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be at most 8"
98+
// CHECK-NEXT: attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be at most 8"
9999

100100
def OpK: NS_Op<"op_for_arr_in_range_at_index", []> {
101101
let arguments = (ins ConfinedAttr<ArrayAttr, [IntArrayNthElemInRange<0, 4, 8>]>:$attr);
102102
}
103103

104104
// CHECK-LABEL: OpKAdaptor::verify
105105
// CHECK: (((::llvm::cast<::mlir::ArrayAttr>(tblgen_attr).size() > 0)) && ((::llvm::cast<::mlir::IntegerAttr>(::llvm::cast<::mlir::ArrayAttr>(tblgen_attr)[0]).getInt() >= 4)) && ((::llvm::cast<::mlir::IntegerAttr>(::llvm::cast<::mlir::ArrayAttr>(tblgen_attr)[0]).getInt() <= 8)))))
106-
// CHECK-NEXT: "attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be at least 4 and at most 8"
106+
// CHECK-NEXT: attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be at least 4 and at most 8"
107107

108108
def OpL: NS_Op<"op_for_TCopVTEtAreSameAt", [
109109
PredOpTrait<"operands indexed at 0, 2, 3 should all have "
@@ -121,7 +121,7 @@ def OpL: NS_Op<"op_for_TCopVTEtAreSameAt", [
121121
// CHECK: ::llvm::all_equal(::llvm::map_range(
122122
// CHECK-SAME: ::mlir::ArrayRef<unsigned>({0, 2, 3}),
123123
// CHECK-SAME: [this](unsigned i) { return getElementTypeOrSelf(this->getOperand(i)); }))
124-
// CHECK: "failed to verify that operands indexed at 0, 2, 3 should all have the same type"
124+
// CHECK: failed to verify that operands indexed at 0, 2, 3 should all have the same type"
125125

126126
def OpM : NS_Op<"op_for_AnyTensorOf", []> {
127127
let arguments = (ins TensorOf<[F32, I32]>:$x);

0 commit comments

Comments
 (0)