[mlir][ods] Enable basic string interpolation in constraint summary. #153603
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-mlir-core @llvm/pr-subscribers-mlir Author: Jacques Pienaar (jpienaar) ChangesThis 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): 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. Patch is 25.21 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/153603.diff 9 Files Affected:
diff --git a/mlir/include/mlir/TableGen/CodeGenHelpers.h b/mlir/include/mlir/TableGen/CodeGenHelpers.h
index cf14f65b93ed2..59baaa20682ed 100644
--- a/mlir/include/mlir/TableGen/CodeGenHelpers.h
+++ b/mlir/include/mlir/TableGen/CodeGenHelpers.h
@@ -78,6 +78,13 @@ class NamespaceEmitter {
SmallVector<StringRef, 2> namespaces;
};
+enum class ErrorStreamType {
+ // Inside a string that's streamed into an InflightDiagnostic.
+ InString,
+ // Inside a string inside an OpError.
+ InsideOpError,
+};
+
/// This class deduplicates shared operation verification code by emitting
/// static functions alongside the op definitions. These methods are local to
/// the definition file, and are invoked within the operation verify methods.
@@ -218,7 +225,8 @@ class StaticVerifierFunctionEmitter {
/// A generic function to emit constraints
void emitConstraints(const ConstraintMap &constraints, StringRef selfName,
- const char *codeTemplate);
+ const char *codeTemplate,
+ ErrorStreamType errorStreamType);
/// Assign a unique name to a unique constraint.
std::string getUniqueName(StringRef kind, unsigned index);
@@ -269,6 +277,16 @@ std::string stringify(T &&t) {
apply(std::forward<T>(t));
}
+/// Helper to generate a C++ streaming error messages from a given message.
+/// Message can contain '{{...}}' placeholders that are substituted with
+/// C-expressions via tgfmt. It would effectively convert:
+/// "Failed to verify {{foo}}"
+/// into:
+/// "Failed to verify " << tgfmt(foo, &ctx)
+std::string buildErrorStreamingString(
+ StringRef message, const FmtContext &ctx,
+ ErrorStreamType errorStreamType = ErrorStreamType::InString);
+
} // namespace tblgen
} // namespace mlir
diff --git a/mlir/lib/TableGen/CodeGenHelpers.cpp b/mlir/lib/TableGen/CodeGenHelpers.cpp
index 2c119fd680b69..cae6bd015a497 100644
--- a/mlir/lib/TableGen/CodeGenHelpers.cpp
+++ b/mlir/lib/TableGen/CodeGenHelpers.cpp
@@ -12,11 +12,25 @@
//===----------------------------------------------------------------------===//
#include "mlir/TableGen/CodeGenHelpers.h"
+#include "mlir/Support/LLVM.h"
+#include "mlir/TableGen/Argument.h"
+#include "mlir/TableGen/Attribute.h"
+#include "mlir/TableGen/Format.h"
#include "mlir/TableGen/Operator.h"
#include "mlir/TableGen/Pattern.h"
+#include "mlir/TableGen/Property.h"
+#include "mlir/TableGen/Region.h"
+#include "mlir/TableGen/Successor.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/TableGen/Error.h"
#include "llvm/TableGen/Record.h"
+#include <cassert>
+#include <optional>
+#include <string>
using namespace llvm;
using namespace mlir;
@@ -113,6 +127,56 @@ StringRef StaticVerifierFunctionEmitter::getRegionConstraintFn(
// Constraint Emission
//===----------------------------------------------------------------------===//
+/// Helper to generate a C++ string expression from a given message.
+/// Message can contain '{{...}}' placeholders that are substituted with
+/// C-expressions via tgfmt.
+std::string mlir::tblgen::buildErrorStreamingString(
+ StringRef message, const FmtContext &ctx, ErrorStreamType errorStreamType) {
+ std::string result;
+ raw_string_ostream os(result);
+
+ std::string msgStr = escapeString(message);
+ StringRef msg = msgStr;
+
+ // Split the message by '{{' and '}}' and build a streaming expression.
+ auto split = msg.split("{{");
+ if (split.second.empty()) {
+ os << split.first;
+ return msgStr;
+ }
+
+ os << split.first;
+ if (errorStreamType == ErrorStreamType::InsideOpError)
+ os << "\")";
+ else
+ os << '"';
+
+ msg = split.second;
+ while (!msg.empty()) {
+ split = msg.split("}}");
+ StringRef var = split.first;
+ StringRef rest = split.second;
+
+ os << " << " << tgfmt(var, &ctx);
+
+ if (rest.empty())
+ break;
+
+ split = rest.split("{{");
+ if (split.second.empty() &&
+ errorStreamType == ErrorStreamType::InsideOpError) {
+ // To enable having part of string post, this adds a parenthesis before
+ // the last string segment to match the exiting one.
+ os << " << (\"" << split.first;
+ } else {
+ os << " << \"" << split.first;
+ }
+ msg = split.second;
+ }
+
+ return os.str();
+}
+
/// Code templates for emitting type, attribute, successor, and region
/// constraints. Each of these templates require the following arguments:
///
@@ -225,22 +289,24 @@ static ::llvm::LogicalResult {0}(
void StaticVerifierFunctionEmitter::emitConstraints(
const ConstraintMap &constraints, StringRef selfName,
- const char *const codeTemplate) {
+ const char *const codeTemplate, ErrorStreamType errorStreamType) {
FmtContext ctx;
ctx.addSubst("_op", "*op").withSelf(selfName);
+
for (auto &it : constraints) {
os << formatv(codeTemplate, it.second,
tgfmt(it.first.getConditionTemplate(), &ctx),
- escapeString(it.first.getSummary()));
+ buildErrorStreamingString(it.first.getSummary(), ctx));
}
}
-
void StaticVerifierFunctionEmitter::emitTypeConstraints() {
- emitConstraints(typeConstraints, "type", typeConstraintCode);
+ emitConstraints(typeConstraints, "type", typeConstraintCode,
+ ErrorStreamType::InString);
}
void StaticVerifierFunctionEmitter::emitAttrConstraints() {
- emitConstraints(attrConstraints, "attr", attrConstraintCode);
+ emitConstraints(attrConstraints, "attr", attrConstraintCode,
+ ErrorStreamType::InString);
}
/// Unlike with the other helpers, this one has to substitute in the interface
@@ -252,17 +318,19 @@ void StaticVerifierFunctionEmitter::emitPropConstraints() {
auto propConstraint = cast<PropConstraint>(it.first);
os << formatv(propConstraintCode, it.second,
tgfmt(propConstraint.getConditionTemplate(), &ctx),
- escapeString(it.first.getSummary()),
+ buildErrorStreamingString(it.first.getSummary(), ctx),
propConstraint.getInterfaceType());
}
}
void StaticVerifierFunctionEmitter::emitSuccessorConstraints() {
- emitConstraints(successorConstraints, "successor", successorConstraintCode);
+ emitConstraints(successorConstraints, "successor", successorConstraintCode,
+ ErrorStreamType::InString);
}
void StaticVerifierFunctionEmitter::emitRegionConstraints() {
- emitConstraints(regionConstraints, "region", regionConstraintCode);
+ emitConstraints(regionConstraints, "region", regionConstraintCode,
+ ErrorStreamType::InString);
}
void StaticVerifierFunctionEmitter::emitPatternConstraints() {
@@ -271,13 +339,14 @@ void StaticVerifierFunctionEmitter::emitPatternConstraints() {
for (auto &it : typeConstraints) {
os << formatv(patternConstraintCode, it.second,
tgfmt(it.first.getConditionTemplate(), &ctx),
- escapeString(it.first.getSummary()), "::mlir::Type type");
+ buildErrorStreamingString(it.first.getSummary(), ctx),
+ "::mlir::Type type");
}
ctx.withSelf("attr");
for (auto &it : attrConstraints) {
os << formatv(patternConstraintCode, it.second,
tgfmt(it.first.getConditionTemplate(), &ctx),
- escapeString(it.first.getSummary()),
+ buildErrorStreamingString(it.first.getSummary(), ctx),
"::mlir::Attribute attr");
}
ctx.withSelf("prop");
@@ -292,7 +361,7 @@ void StaticVerifierFunctionEmitter::emitPatternConstraints() {
}
os << formatv(patternConstraintCode, it.second,
tgfmt(propConstraint.getConditionTemplate(), &ctx),
- escapeString(propConstraint.getSummary()),
+ buildErrorStreamingString(propConstraint.getSummary(), ctx),
Twine(interfaceType) + " prop");
}
}
diff --git a/mlir/test/mlir-tblgen/constraint-unique.td b/mlir/test/mlir-tblgen/constraint-unique.td
index d51e1a5f43ee7..44dc57d524fb6 100644
--- a/mlir/test/mlir-tblgen/constraint-unique.td
+++ b/mlir/test/mlir-tblgen/constraint-unique.td
@@ -16,7 +16,7 @@ def AType : Type<ATypePred, "a type">;
def OtherType : Type<ATypePred, "another type">;
def AnAttrPred : CPred<"attrPred($_self, $_op)">;
-def AnAttr : Attr<AnAttrPred, "an attribute">;
+def AnAttr : Attr<AnAttrPred, "an attribute (got {{reformat($_self)}})">;
def OtherAttr : Attr<AnAttrPred, "another attribute">;
def ASuccessorPred : CPred<"successorPred($_self, $_op)">;
@@ -71,10 +71,10 @@ def OpC : NS_Op<"op_c"> {
// CHECK: static ::llvm::LogicalResult [[$A_ATTR_CONSTRAINT:__mlir_ods_local_attr_constraint.*]](
// CHECK: if (attr && !((attrPred(attr, *op))))
// CHECK-NEXT: return emitError() << "attribute '" << attrName
-// CHECK-NEXT: << "' failed to satisfy constraint: an attribute";
+// CHECK-NEXT: << "' failed to satisfy constraint: an attribute (got " << reformat(attr) << ")";
/// Test that duplicate attribute constraint was not generated.
-// CHECK-NOT: << "' failed to satisfy constraint: an attribute";
+// CHECK-NOT: << "' failed to satisfy constraint: an attribute
/// Test that a attribute constraint with a different description was generated.
// CHECK: static ::llvm::LogicalResult [[$O_ATTR_CONSTRAINT:__mlir_ods_local_attr_constraint.*]](
diff --git a/mlir/test/mlir-tblgen/op-attribute.td b/mlir/test/mlir-tblgen/op-attribute.td
index 549830e06042f..a3cb9a41a5b7f 100644
--- a/mlir/test/mlir-tblgen/op-attribute.td
+++ b/mlir/test/mlir-tblgen/op-attribute.td
@@ -69,19 +69,19 @@ def AOp : NS_Op<"a_op", []> {
// DEF: ::llvm::LogicalResult AOpAdaptor::verify
// DEF-NEXT: auto tblgen_aAttr = getProperties().aAttr; (void)tblgen_aAttr;
-// DEF-NEXT: if (!tblgen_aAttr) return emitError(loc, "'test.a_op' op ""requires attribute 'aAttr'");
+// DEF-NEXT: if (!tblgen_aAttr) return emitError(loc, "'test.a_op' op requires attribute 'aAttr'");
// DEF-NEXT: auto tblgen_bAttr = getProperties().bAttr; (void)tblgen_bAttr;
// DEF-NEXT: auto tblgen_cAttr = getProperties().cAttr; (void)tblgen_cAttr;
// DEF-NEXT: auto tblgen_dAttr = getProperties().dAttr; (void)tblgen_dAttr;
// DEF: if (tblgen_aAttr && !((some-condition)))
-// DEF-NEXT: return emitError(loc, "'test.a_op' op ""attribute 'aAttr' failed to satisfy constraint: some attribute kind");
+// DEF-NEXT: return emitError(loc, "'test.a_op' op attribute 'aAttr' failed to satisfy constraint: some attribute kind");
// DEF: if (tblgen_bAttr && !((some-condition)))
-// DEF-NEXT: return emitError(loc, "'test.a_op' op ""attribute 'bAttr' failed to satisfy constraint: some attribute kind");
+// DEF-NEXT: return emitError(loc, "'test.a_op' op attribute 'bAttr' failed to satisfy constraint: some attribute kind");
// DEF: if (tblgen_cAttr && !((some-condition)))
-// DEF-NEXT: return emitError(loc, "'test.a_op' op ""attribute 'cAttr' failed to satisfy constraint: some attribute kind");
+// DEF-NEXT: return emitError(loc, "'test.a_op' op attribute 'cAttr' failed to satisfy constraint: some attribute kind");
// DEF: if (tblgen_dAttr && !((some-condition)))
-// DEF-NEXT: return emitError(loc, "'test.a_op' op ""attribute 'dAttr' failed to satisfy constraint: some attribute kind");
+// DEF-NEXT: return emitError(loc, "'test.a_op' op attribute 'dAttr' failed to satisfy constraint: some attribute kind");
// Test getter methods
// ---
@@ -219,13 +219,13 @@ def AgetOp : Op<Test2_Dialect, "a_get_op", []> {
// DEF: ::llvm::LogicalResult AgetOpAdaptor::verify
// DEF: auto tblgen_aAttr = getProperties().aAttr; (void)tblgen_aAttr;
-// DEF: if (!tblgen_aAttr) return emitError(loc, "'test2.a_get_op' op ""requires attribute 'aAttr'");
+// DEF: if (!tblgen_aAttr) return emitError(loc, "'test2.a_get_op' op requires attribute 'aAttr'");
// DEF: auto tblgen_bAttr = getProperties().bAttr; (void)tblgen_bAttr;
// DEF: auto tblgen_cAttr = getProperties().cAttr; (void)tblgen_cAttr;
// DEF: if (tblgen_bAttr && !((some-condition)))
-// DEF-NEXT: return emitError(loc, "'test2.a_get_op' op ""attribute 'bAttr' failed to satisfy constraint: some attribute kind");
+// DEF-NEXT: return emitError(loc, "'test2.a_get_op' op attribute 'bAttr' failed to satisfy constraint: some attribute kind");
// DEF: if (tblgen_cAttr && !((some-condition)))
-// DEF-NEXT: return emitError(loc, "'test2.a_get_op' op ""attribute 'cAttr' failed to satisfy constraint: some attribute kind");
+// DEF-NEXT: return emitError(loc, "'test2.a_get_op' op attribute 'cAttr' failed to satisfy constraint: some attribute kind");
// Test getter methods
// ---
diff --git a/mlir/test/mlir-tblgen/op-properties-predicates.td b/mlir/test/mlir-tblgen/op-properties-predicates.td
index af09ee7c12f53..7cc9633850069 100644
--- a/mlir/test/mlir-tblgen/op-properties-predicates.td
+++ b/mlir/test/mlir-tblgen/op-properties-predicates.td
@@ -74,7 +74,7 @@ def OpWithPredicates : NS_Op<"op_with_predicates"> {
// Note: comprehensive emission of verifiers is tested in verifyINvariantsImpl() below
// CHECK: int64_t tblgen_scalar = this->getScalar();
// CHECK: if (!((tblgen_scalar >= 0)))
-// CHECK: return emitError(loc, "'test.op_with_predicates' op ""property 'scalar' failed to satisfy constraint: non-negative int64_t");
+// CHECK: return emitError(loc, "'test.op_with_predicates' op property 'scalar' failed to satisfy constraint: non-negative int64_t");
// CHECK-LABEL: OpWithPredicates::verifyInvariantsImpl()
// Note: for test readability, we capture [[maybe_unused]] into the variable maybe_unused
diff --git a/mlir/test/mlir-tblgen/predicate.td b/mlir/test/mlir-tblgen/predicate.td
index c1fcd3fa76089..41e041f171213 100644
--- a/mlir/test/mlir-tblgen/predicate.td
+++ b/mlir/test/mlir-tblgen/predicate.td
@@ -55,7 +55,7 @@ def OpF : NS_Op<"op_for_int_min_val", []> {
// CHECK-LABEL: OpFAdaptor::verify
// CHECK: (::llvm::cast<::mlir::IntegerAttr>(tblgen_attr).getInt() >= 10)
-// CHECK-NEXT: "attribute 'attr' failed to satisfy constraint: 32-bit signless integer attribute whose minimum value is 10"
+// CHECK-NEXT: attribute 'attr' failed to satisfy constraint: 32-bit signless integer attribute whose minimum value is 10"
def OpFX : NS_Op<"op_for_int_max_val", []> {
let arguments = (ins ConfinedAttr<I32Attr, [IntMaxValue<10>]>:$attr);
@@ -63,7 +63,7 @@ def OpFX : NS_Op<"op_for_int_max_val", []> {
// CHECK-LABEL: OpFXAdaptor::verify
// CHECK: (::llvm::cast<::mlir::IntegerAttr>(tblgen_attr).getInt() <= 10)
-// CHECK-NEXT: "attribute 'attr' failed to satisfy constraint: 32-bit signless integer attribute whose maximum value is 10"
+// CHECK-NEXT: attribute 'attr' failed to satisfy constraint: 32-bit signless integer attribute whose maximum value is 10"
def OpG : NS_Op<"op_for_arr_min_count", []> {
let arguments = (ins ConfinedAttr<ArrayAttr, [ArrayMinCount<8>]>:$attr);
@@ -71,7 +71,7 @@ def OpG : NS_Op<"op_for_arr_min_count", []> {
// CHECK-LABEL: OpGAdaptor::verify
// CHECK: (::llvm::cast<::mlir::ArrayAttr>(tblgen_attr).size() >= 8)
-// CHECK-NEXT: "attribute 'attr' failed to satisfy constraint: array attribute with at least 8 elements"
+// CHECK-NEXT: attribute 'attr' failed to satisfy constraint: array attribute with at least 8 elements"
def OpH : NS_Op<"op_for_arr_value_at_index", []> {
let arguments = (ins ConfinedAttr<ArrayAttr, [IntArrayNthElemEq<0, 8>]>:$attr);
@@ -79,7 +79,7 @@ def OpH : NS_Op<"op_for_arr_value_at_index", []> {
// CHECK-LABEL: OpHAdaptor::verify
// CHECK: (((::llvm::cast<::mlir::ArrayAttr>(tblgen_attr).size() > 0)) && ((::llvm::cast<::mlir::IntegerAttr>(::llvm::cast<::mlir::ArrayAttr>(tblgen_attr)[0]).getInt() == 8)))))
-// CHECK-NEXT: "attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be 8"
+// CHECK-NEXT: attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be 8"
def OpI: NS_Op<"op_for_arr_min_value_at_index", []> {
let arguments = (ins ConfinedAttr<ArrayAttr, [IntArrayNthElemMinValue<0, 8>]>:$attr);
@@ -87,7 +87,7 @@ def OpI: NS_Op<"op_for_arr_min_value_at_index", []> {
// CHECK-LABEL: OpIAdaptor::verify
// CHECK: (((::llvm::cast<::mlir::ArrayAttr>(tblgen_attr).size() > 0)) && ((::llvm::cast<::mlir::IntegerAttr>(::llvm::cast<::mlir::ArrayAttr>(tblgen_attr)[0]).getInt() >= 8)))))
-// CHECK-NEXT: "attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be at least 8"
+// CHECK-NEXT: attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be at least 8"
def OpJ: NS_Op<"op_for_arr_max_value_at_index", []> {
let arguments = (ins ConfinedAttr<ArrayAttr, [IntArrayNthElemMaxValue<0, 8>]>:$attr);
@@ -95,7 +95,7 @@ def OpJ: NS_Op<"op_for_arr_max_value_at_index", []> {
// CHECK-LABEL: OpJAdaptor::verify
// CHECK: (((::llvm::cast<::mlir::ArrayAttr>(tblgen_attr).size() > 0)) && ((::llvm::cast<::mlir::IntegerAttr>(::llvm::cast<::mlir::ArrayAttr>(tblgen_attr)[0]).getInt() <= 8)))))
-// CHECK-NEXT: "attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be at most 8"
+// CHECK-NEXT: attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be at most 8"
def OpK: NS_Op<"op_for_arr_in_range_at_index", []> {
let arguments = (ins ConfinedAttr<ArrayAttr, [IntArrayNthElemInRange<0, 4, 8>]>:$attr);
@@ -103,7 +103,7 @@ def OpK: NS_Op<"op_for_arr_in_range_at_index", []> {
// CHECK-LABEL: OpKAdaptor::verify
// 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)))))
-// CHECK-NEXT: "attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be at least 4 and at most 8"
+// CHECK-NEXT: attribute 'attr' failed to satisfy constraint: array attribute whose 0-th element must be at least 4 and at most 8"
def OpL: NS_Op<"op_for_TCopVTEtAreSameAt", [
PredOpTrait<"operands indexed at 0, 2, 3 should all have "
@@ -121,7 +121,7 @@ def OpL: NS_Op<"op_for_TCopVTEtAreSameAt", [
// CHECK: ::llvm::all_equal(::llvm::map_range(
// CHECK-SAME: ::mlir::ArrayRef<unsigned>({0, 2, 3}),
// CHECK-SAME: [this](unsigned i) { return getElementTypeOrSelf(this->getOperand(i)); }))
-// CHECK: "failed to verify that operands indexed at 0, 2, 3 should all have the same type"
+// CHECK: failed to verify that operands indexed at 0, 2, 3 should all have the same type"
def OpM : NS_Op<"op_for_AnyTensorOf", []> {
let arguments = (ins TensorOf<[F32, I32]>:$x);
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 8ea4eb7b3eeca..752afa67182d2 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -17,6 +17,7 @@
#include "OpGenHelpers.h"
#include "mlir/TableGen/Argument.h"
#include "mlir/TableGen/Attribute.h"
+#include "mlir/TableGen/Builder.h"
#include "mlir/TableGen/Class.h"
#include "mlir/TableGen/CodeGenHelpers.h"
#include "mlir/TableGen/Format.h"
@@ -24,21 +25,38 @@
#include "mlir/TableGen/Interfaces.h"
#include "mlir/TableGen/Operator.h"
#include "mlir/TableGen/Property.h"
+#include "mlir/TableGen/Region.h"
#include "mlir/TableGen/SideEffects.h"
+#include "mlir/TableGen/Successor.h"
#include "mlir/TableGen/Trait.h"
#include "llvm/ADT/BitVector.h"
#include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/Sequence.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Casting.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/TableGen/Error.h"
#include "llvm/TableGen/Record.h"
#include "llvm/TableGen/T...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
RobSpringer
reviewed
Oct 22, 2025
RobSpringer
left a comment
RobSpringer
left a comment
There was a problem hiding this comment.
LGTM, but it's up to you to determine how much that matters. :)
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 : i16)
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.
joker-eph
reviewed
Nov 6, 2025
joker-eph
reviewed
Nov 6, 2025
joker-eph
reviewed
Nov 6, 2025
joker-eph
reviewed
Nov 6, 2025
| std::string result2 = buildErrorStreamingString( | ||
| "here {{reformat($_self)}} is block", ctx, ErrorStreamType::InString); | ||
| EXPECT_THAT(result2, StrEq("here \" << reformat(this_thing) << \" is block")); | ||
| } |
Collaborator
There was a problem hiding this comment.
Why do we need this C++ unit-test?
The mlir/test/mlir-tblgen/constraint-unique.td should cover this API already.
Member
Author
There was a problem hiding this comment.
This was one where it was just more obvious that I'm testing both kind of ErrorStreamType while the other one its a bit indirect which path was followed. So made it just much simpler to do simple coverage testing.
joker-eph
reviewed
Nov 6, 2025
|
|
||
| def AnAttrPred : CPred<"attrPred($_self, $_op)">; | ||
| def AnAttr : Attr<AnAttrPred, "an attribute">; | ||
| def AnAttr : Attr<AnAttrPred, "an attribute (got {{reformat($_self)}})">; |
Collaborator
There was a problem hiding this comment.
We're missing an equivalent test for the op case I think?
Member
Author
There was a problem hiding this comment.
Not sure I follow, this is used by an attribute constraint for an op (e.g., this is via OpDefinitionsGen invocation of CodeGenHelper).
Collaborator
There was a problem hiding this comment.
I'm trying to see the result of buildErrorStreamingString invoked with InString and InsideOpError , and make the C++ unit-test obsolete, I don't think it should exists really.
Member
Author
There was a problem hiding this comment.
The tests here show that (both forms are used and CHECK'd in the constraints file), but I think you also wanted to see an interpolation one for both kinds. Added and removed the unit test.
Collaborator
There was a problem hiding this comment.
Thanks! The change around the " everywhere made it hard to figure out the coverage: the UI just showed many line change where only the " was the diff and I couldn't figure it out.
joker-eph
reviewed
Nov 6, 2025
joker-eph
approved these changes
Nov 6, 2025
vinay-deshmukh
pushed a commit
to vinay-deshmukh/llvm-project
that referenced
this pull request
Nov 8, 2025
…lvm#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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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):
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.