Skip to content

Commit 5d15b5a

Browse files
maksleventalmakslevental
committed
use enum for verifyDiagnostics
1 parent af3fbaa commit 5d15b5a

File tree

9 files changed

+160
-89
lines changed

9 files changed

+160
-89
lines changed

‎mlir/examples/transform-opt/mlir-transform-opt.cpp

Lines changed: 32 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -38,17 +38,20 @@ struct MlirTransformOptCLOptions {
3838
cl::desc("Allow operations coming from an unregistered dialect"),
3939
cl::init(false)};
4040

41-
cl::opt<bool> verifyDiagnostics{
42-
"verify-diagnostics",
43-
cl::desc("Check that emitted diagnostics match expected-* lines "
44-
"on the corresponding line"),
45-
cl::init(false)};
46-
cl::opt<bool> verifyOnlyExpectedDiagnostics{
47-
"verify-only-expected-diagnostics",
48-
cl::desc("Check that emitted diagnostics match only specified expected-* "
49-
"lines "
50-
"on the corresponding line"),
51-
cl::init(false)};
41+
cl::opt<mlir::SourceMgrDiagnosticVerifierHandler::Level> verifyDiagnostics{
42+
"verify-diagnostics", llvm::cl::ValueOptional,
43+
cl::desc("Check that emitted diagnostics match "
44+
"expected-* lines on the corresponding line"),
45+
cl::values(
46+
clEnumValN(
47+
mlir::SourceMgrDiagnosticVerifierHandler::Level::All, "all",
48+
"Check all diagnostics (expected, unexpected, near-misses)"),
49+
clEnumValN(
50+
mlir::SourceMgrDiagnosticVerifierHandler::Level::All, "",
51+
"Check all diagnostics (expected, unexpected, near-misses)"),
52+
clEnumValN(
53+
mlir::SourceMgrDiagnosticVerifierHandler::Level::OnlyExpected,
54+
"only-expected", "Check only expected diagnostics"))};
5255

5356
cl::opt<std::string> payloadFilename{cl::Positional, cl::desc("<input file>"),
5457
cl::init("-")};
@@ -108,14 +111,17 @@ class DiagnosticHandlerWrapper {
108111

109112
/// Constructs the diagnostic handler of the specified kind of the given
110113
/// source manager and context.
111-
DiagnosticHandlerWrapper(Kind kind, llvm::SourceMgr &mgr,
112-
mlir::MLIRContext *context,
113-
bool verifyOnlyExpectedDiagnostics = false) {
114-
if (kind == Kind::EmitDiagnostics)
114+
DiagnosticHandlerWrapper(
115+
Kind kind, llvm::SourceMgr &mgr, mlir::MLIRContext *context,
116+
std::optional<mlir::SourceMgrDiagnosticVerifierHandler::Level> level =
117+
{}) {
118+
if (kind == Kind::EmitDiagnostics) {
115119
handler = new mlir::SourceMgrDiagnosticHandler(mgr, context);
116-
else
117-
handler = new mlir::SourceMgrDiagnosticVerifierHandler(
118-
mgr, context, verifyOnlyExpectedDiagnostics);
120+
} else {
121+
assert(level.has_value() && "expected level");
122+
handler =
123+
new mlir::SourceMgrDiagnosticVerifierHandler(mgr, context, *level);
124+
}
119125
}
120126

121127
/// This object is non-copyable but movable.
@@ -158,10 +164,10 @@ class TransformSourceMgr {
158164
public:
159165
/// Constructs the source manager indicating whether diagnostic messages will
160166
/// be verified later on.
161-
explicit TransformSourceMgr(bool verifyDiagnostics,
162-
bool verifyOnlyExpectedDiagnostics)
163-
: verifyDiagnostics(verifyDiagnostics),
164-
verifyOnlyExpectedDiagnostics(verifyOnlyExpectedDiagnostics) {}
167+
explicit TransformSourceMgr(
168+
std::optional<mlir::SourceMgrDiagnosticVerifierHandler::Level>
169+
verifyDiagnostics)
170+
: verifyDiagnostics(verifyDiagnostics) {}
165171

166172
/// Deconstructs the source manager. Note that `checkResults` must have been
167173
/// called on this instance before deconstructing it.
@@ -190,7 +196,7 @@ class TransformSourceMgr {
190196
if (verifyDiagnostics) {
191197
diagHandlers.emplace_back(
192198
DiagnosticHandlerWrapper::Kind::VerifyDiagnostics, mgr, &context,
193-
verifyOnlyExpectedDiagnostics);
199+
verifyDiagnostics);
194200
} else {
195201
diagHandlers.emplace_back(DiagnosticHandlerWrapper::Kind::EmitDiagnostics,
196202
mgr, &context);
@@ -215,10 +221,8 @@ class TransformSourceMgr {
215221

216222
private:
217223
/// Indicates whether diagnostic message verification is requested.
218-
const bool verifyDiagnostics;
219-
/// Indicates whether *only specified* diagnostic message verification is
220-
/// requested.
221-
const bool verifyOnlyExpectedDiagnostics;
224+
const std::optional<mlir::SourceMgrDiagnosticVerifierHandler::Level>
225+
verifyDiagnostics;
222226

223227
/// Indicates that diagnostic message verification has taken place, and the
224228
/// deconstruction is therefore safe.
@@ -262,8 +266,7 @@ static llvm::LogicalResult processPayloadBuffer(
262266
context.allowUnregisteredDialects(clOptions->allowUnregisteredDialects);
263267
mlir::ParserConfig config(&context);
264268
TransformSourceMgr sourceMgr(
265-
/*verifyDiagnostics=*/clOptions->verifyDiagnostics,
266-
clOptions->verifyOnlyExpectedDiagnostics);
269+
/*verifyDiagnostics=*/clOptions->verifyDiagnostics);
267270

268271
// Parse the input buffer that will be used as transform payload.
269272
mlir::OwningOpRef<mlir::Operation *> payloadRoot =

‎mlir/include/mlir/IR/Diagnostics.h

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -626,12 +626,12 @@ struct SourceMgrDiagnosticVerifierHandlerImpl;
626626
/// corresponding line of the source file.
627627
class SourceMgrDiagnosticVerifierHandler : public SourceMgrDiagnosticHandler {
628628
public:
629-
SourceMgrDiagnosticVerifierHandler(
630-
llvm::SourceMgr &srcMgr, MLIRContext *ctx, raw_ostream &out,
631-
bool verifyOnlyExpectedDiagnostics = false);
632-
SourceMgrDiagnosticVerifierHandler(
633-
llvm::SourceMgr &srcMgr, MLIRContext *ctx,
634-
bool verifyOnlyExpectedDiagnostics = false);
629+
enum class Level { None = 0, All, OnlyExpected };
630+
SourceMgrDiagnosticVerifierHandler(llvm::SourceMgr &srcMgr, MLIRContext *ctx,
631+
raw_ostream &out,
632+
Level level = Level::All);
633+
SourceMgrDiagnosticVerifierHandler(llvm::SourceMgr &srcMgr, MLIRContext *ctx,
634+
Level level = Level::All);
635635
~SourceMgrDiagnosticVerifierHandler();
636636

637637
/// Returns the status of the handler and verifies that all expected
@@ -648,9 +648,7 @@ class SourceMgrDiagnosticVerifierHandler : public SourceMgrDiagnosticHandler {
648648

649649
std::unique_ptr<detail::SourceMgrDiagnosticVerifierHandlerImpl> impl;
650650

651-
/// Set whether to check that emitted diagnostics match *only specified*
652-
/// `expected-*` lines on the corresponding line.
653-
bool verifyOnlyExpectedDiagnostics = false;
651+
Level level = Level::All;
654652
};
655653

656654
//===----------------------------------------------------------------------===//

‎mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -185,20 +185,19 @@ class MlirOptMainConfig {
185185

186186
/// Set whether to check that emitted diagnostics match `expected-*` lines on
187187
/// the corresponding line. This is meant for implementing diagnostic tests.
188-
MlirOptMainConfig &verifyDiagnostics(bool verify) {
188+
MlirOptMainConfig &
189+
verifyDiagnostics(SourceMgrDiagnosticVerifierHandler::Level verify) {
189190
verifyDiagnosticsFlag = verify;
190191
return *this;
191192
}
192-
bool shouldVerifyDiagnostics() const { return verifyDiagnosticsFlag; }
193193

194-
/// Set whether to check that emitted diagnostics match *only specified*
195-
/// `expected-*` lines on the corresponding line.
196-
MlirOptMainConfig &verifyOnlyExpectedDiagnostics(bool verify) {
197-
verifyOnlyExpectedDiagnosticsFlag = verify;
198-
return *this;
194+
bool shouldVerifyDiagnostics() const {
195+
return verifyDiagnosticsFlag !=
196+
SourceMgrDiagnosticVerifierHandler::Level::None;
199197
}
200-
bool shouldVerifyOnlyExpectedDiagnostics() const {
201-
return verifyOnlyExpectedDiagnosticsFlag;
198+
199+
SourceMgrDiagnosticVerifierHandler::Level verifyDiagnosticsLevel() const {
200+
return verifyDiagnosticsFlag;
202201
}
203202

204203
/// Set whether to run the verifier after each transformation pass.
@@ -289,10 +288,8 @@ class MlirOptMainConfig {
289288

290289
/// Set whether to check that emitted diagnostics match `expected-*` lines on
291290
/// the corresponding line. This is meant for implementing diagnostic tests.
292-
bool verifyDiagnosticsFlag = false;
293-
/// Set whether to check that emitted diagnostics match *only specified*
294-
/// `expected-*` lines on the corresponding line.
295-
bool verifyOnlyExpectedDiagnosticsFlag = false;
291+
SourceMgrDiagnosticVerifierHandler::Level verifyDiagnosticsFlag =
292+
SourceMgrDiagnosticVerifierHandler::Level::None;
296293

297294
/// Run the verifier after each transformation pass.
298295
bool verifyPassesFlag = true;

‎mlir/lib/IR/Diagnostics.cpp

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -795,11 +795,9 @@ SourceMgrDiagnosticVerifierHandlerImpl::computeExpectedDiags(
795795
}
796796

797797
SourceMgrDiagnosticVerifierHandler::SourceMgrDiagnosticVerifierHandler(
798-
llvm::SourceMgr &srcMgr, MLIRContext *ctx, raw_ostream &out,
799-
bool verifyOnlyExpectedDiagnostics)
798+
llvm::SourceMgr &srcMgr, MLIRContext *ctx, raw_ostream &out, Level level)
800799
: SourceMgrDiagnosticHandler(srcMgr, ctx, out),
801-
impl(new SourceMgrDiagnosticVerifierHandlerImpl()),
802-
verifyOnlyExpectedDiagnostics(verifyOnlyExpectedDiagnostics) {
800+
impl(new SourceMgrDiagnosticVerifierHandlerImpl()), level(level) {
803801
// Compute the expected diagnostics for each of the current files in the
804802
// source manager.
805803
for (unsigned i = 0, e = mgr.getNumBuffers(); i != e; ++i)
@@ -817,10 +815,8 @@ SourceMgrDiagnosticVerifierHandler::SourceMgrDiagnosticVerifierHandler(
817815
}
818816

819817
SourceMgrDiagnosticVerifierHandler::SourceMgrDiagnosticVerifierHandler(
820-
llvm::SourceMgr &srcMgr, MLIRContext *ctx,
821-
bool verifyOnlyExpectedDiagnostics)
822-
: SourceMgrDiagnosticVerifierHandler(srcMgr, ctx, llvm::errs(),
823-
verifyOnlyExpectedDiagnostics) {}
818+
llvm::SourceMgr &srcMgr, MLIRContext *ctx, Level level)
819+
: SourceMgrDiagnosticVerifierHandler(srcMgr, ctx, llvm::errs(), level) {}
824820

825821
SourceMgrDiagnosticVerifierHandler::~SourceMgrDiagnosticVerifierHandler() {
826822
// Ensure that all expected diagnostics were handled.
@@ -890,7 +886,7 @@ void SourceMgrDiagnosticVerifierHandler::process(FileLineColLoc loc,
890886
}
891887
}
892888

893-
if (verifyOnlyExpectedDiagnostics)
889+
if (level == Level::OnlyExpected)
894890
return;
895891

896892
// Otherwise, emit an error for the near miss.

‎mlir/lib/Tools/mlir-opt/MlirOptMain.cpp

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -168,19 +168,26 @@ struct MlirOptMainConfigCLOptions : public MlirOptMainConfig {
168168
cl::desc("Split marker to use for merging the ouput"),
169169
cl::location(outputSplitMarkerFlag), cl::init(kDefaultSplitMarker));
170170

171-
static cl::opt<bool, /*ExternalStorage=*/true> verifyDiagnostics(
172-
"verify-diagnostics",
173-
cl::desc("Check that emitted diagnostics match "
174-
"expected-* lines on the corresponding line"),
175-
cl::location(verifyDiagnosticsFlag), cl::init(false));
176-
static cl::opt<bool, /*ExternalStorage=*/true>
177-
verifyOnlyExpectedDiagnostics{
178-
"verify-only-expected-diagnostics",
179-
cl::desc("Check that emitted diagnostics match only specified "
180-
"expected-* "
181-
"lines "
182-
"on the corresponding line"),
183-
cl::location(verifyOnlyExpectedDiagnosticsFlag), cl::init(false)};
171+
static cl::opt<SourceMgrDiagnosticVerifierHandler::Level,
172+
/*ExternalStorage=*/true>
173+
verifyDiagnostics{
174+
"verify-diagnostics", llvm::cl::ValueOptional,
175+
cl::desc("Check that emitted diagnostics match "
176+
"expected-* lines on the corresponding line"),
177+
cl::location(verifyDiagnosticsFlag),
178+
cl::values(
179+
clEnumValN(SourceMgrDiagnosticVerifierHandler::Level::All,
180+
"all",
181+
"Check all diagnostics (expected, unexpected, "
182+
"near-misses)"),
183+
// Implicit value: when passed with no arguments, e.g.
184+
// `--verify-diagnostics` or `--verify-diagnostics=`.
185+
clEnumValN(SourceMgrDiagnosticVerifierHandler::Level::All, "",
186+
"Check all diagnostics (expected, unexpected, "
187+
"near-misses)"),
188+
clEnumValN(
189+
SourceMgrDiagnosticVerifierHandler::Level::OnlyExpected,
190+
"only-expected", "Check only expected diagnostics"))};
184191

185192
static cl::opt<bool, /*ExternalStorage=*/true> verifyPasses(
186193
"verify-each",
@@ -551,7 +558,7 @@ static LogicalResult processBuffer(raw_ostream &os,
551558
}
552559

553560
SourceMgrDiagnosticVerifierHandler sourceMgrHandler(
554-
*sourceMgr, &context, config.shouldVerifyOnlyExpectedDiagnostics());
561+
*sourceMgr, &context, config.verifyDiagnosticsLevel());
555562

556563
// Do any processing requested by command line flags. We don't care whether
557564
// these actions succeed or fail, we only care what diagnostics they produce

‎mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -73,18 +73,21 @@ LogicalResult mlir::mlirTranslateMain(int argc, char **argv,
7373
"default marker and process each chunk independently"),
7474
llvm::cl::init("")};
7575

76-
static llvm::cl::opt<bool> verifyDiagnostics(
77-
"verify-diagnostics",
78-
llvm::cl::desc("Check that emitted diagnostics match "
79-
"expected-* lines on the corresponding line"),
80-
llvm::cl::init(false));
81-
static llvm::cl::opt<bool> verifyOnlyExpectedDiagnostics{
82-
"verify-only-expected-diagnostics",
83-
llvm::cl::desc(
84-
"Check that emitted diagnostics match only specified expected-* "
85-
"lines "
86-
"on the corresponding line"),
87-
llvm::cl::init(false)};
76+
static llvm::cl::opt<SourceMgrDiagnosticVerifierHandler::Level>
77+
verifyDiagnostics{
78+
"verify-diagnostics", llvm::cl::ValueOptional,
79+
llvm::cl::desc("Check that emitted diagnostics match "
80+
"expected-* lines on the corresponding line"),
81+
llvm::cl::values(
82+
clEnumValN(
83+
SourceMgrDiagnosticVerifierHandler::Level::All, "all",
84+
"Check all diagnostics (expected, unexpected, near-misses)"),
85+
clEnumValN(
86+
SourceMgrDiagnosticVerifierHandler::Level::All, "",
87+
"Check all diagnostics (expected, unexpected, near-misses)"),
88+
clEnumValN(
89+
SourceMgrDiagnosticVerifierHandler::Level::OnlyExpected,
90+
"only-expected", "Check only expected diagnostics"))};
8891

8992
static llvm::cl::opt<bool> errorDiagnosticsOnly(
9093
"error-diagnostics-only",
@@ -157,17 +160,17 @@ LogicalResult mlir::mlirTranslateMain(int argc, char **argv,
157160

158161
MLIRContext context;
159162
context.allowUnregisteredDialects(allowUnregisteredDialects);
160-
context.printOpOnDiagnostic(!verifyDiagnostics);
163+
context.printOpOnDiagnostic(!bool(verifyDiagnostics.getNumOccurrences()));
161164
auto sourceMgr = std::make_shared<llvm::SourceMgr>();
162165
sourceMgr->AddNewSourceBuffer(std::move(ownedBuffer), SMLoc());
163166

164-
if (verifyDiagnostics) {
167+
if (verifyDiagnostics.getNumOccurrences()) {
165168
// In the diagnostic verification flow, we ignore whether the
166169
// translation failed (in most cases, it is expected to fail) and we do
167170
// not filter non-error diagnostics even if `errorDiagnosticsOnly` is
168171
// set. Instead, we check if the diagnostics were produced as expected.
169172
SourceMgrDiagnosticVerifierHandler sourceMgrHandler(
170-
*sourceMgr, &context, verifyOnlyExpectedDiagnostics);
173+
*sourceMgr, &context, verifyDiagnostics);
171174
(void)(*translationRequested)(sourceMgr, os, &context);
172175
result = sourceMgrHandler.verify();
173176
} else if (errorDiagnosticsOnly) {

‎mlir/test/Pass/full_diagnostics.mlir

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(test-pass-failure{gen-diagnostics}))' -verify-diagnostics
2+
// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(test-pass-failure{gen-diagnostics}))' -verify-diagnostics=all
23

34
// Test that all errors are reported.
45
// expected-error@below {{illegal operation}}

‎mlir/test/Pass/full_diagnostics_only_expected.mlir

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(test-pass-failure{gen-diagnostics}))' -verify-diagnostics=only-expected
2+
3+
// Test that only expected errors are reported.
4+
// reports {{illegal operation}} but unchecked
5+
func.func @TestAlwaysIllegalOperationPass1() {
6+
return
7+
}
8+
9+
// expected-error@+1 {{illegal operation}}
10+
func.func @TestAlwaysIllegalOperationPass2() {
11+
return
12+
}
13+
14+
// reports {{illegal operation}} but unchecked
15+
func.func @TestAlwaysIllegalOperationPass3() {
16+
return
17+
}

‎mlir/test/mlir-translate/verify-only-expected.mlir

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Note: borrowed/copied from mlir/test/Target/LLVMIR/arm-sme-invalid.mlir
2+
3+
// Check that verify-diagnostics=only-expected passes with only one actual `expected-error`
4+
// RUN: mlir-translate %s -verify-diagnostics=only-expected -split-input-file -mlir-to-llvmir
5+
6+
// Check that verify-diagnostics=all fails because we're missing three `expected-error`
7+
// RUN: not mlir-translate %s -verify-diagnostics=all -split-input-file -mlir-to-llvmir 2>&1 | FileCheck %s --check-prefix=CHECK-VERIFY-ALL
8+
// CHECK-VERIFY-ALL: error: unexpected error: 'arm_sme.intr.write.horiz' op failed to verify that all of {predicate, vector} have same shape
9+
// CHECK-VERIFY-ALL-NEXT: "arm_sme.intr.write.horiz"
10+
// CHECK-VERIFY-ALL: error: unexpected error: 'arm_sme.intr.read.horiz' op failed to verify that all of {vector, res} have same element type
11+
// CHECK-VERIFY-ALL-NEXT: %res = "arm_sme.intr.read.horiz"
12+
// CHECK-VERIFY-ALL: error: unexpected error: 'arm_sme.intr.cntsb' op failed to verify that `res` is i64
13+
// CHECK-VERIFY-ALL-NEXT: %res = "arm_sme.intr.cntsb"
14+
15+
llvm.func @arm_sme_vector_to_tile_invalid_types(%tileslice : i32,
16+
%nxv4i1 : vector<[4]xi1>,
17+
%nxv16i8 : vector<[16]xi8>) {
18+
"arm_sme.intr.write.horiz"(%tileslice, %nxv4i1, %nxv16i8) <{tile_id = 0 : i32}> :
19+
(i32, vector<[4]xi1>, vector<[16]xi8>) -> ()
20+
llvm.return
21+
}
22+
23+
// -----
24+
25+
llvm.func @arm_sme_tile_slice_to_vector_invalid_shapes(
26+
%tileslice : i32, %nxv4i1 : vector<[4]xi1>, %nxv16i8 : vector<[16]xi8>
27+
) -> vector<[3]xf32> {
28+
// expected-error @+1 {{failed to verify that all of {vector, predicate, res} have same shape}}
29+
%res = "arm_sme.intr.read.horiz"(%nxv16i8, %nxv4i1, %tileslice) <{tile_id = 0 : i32}> :
30+
(vector<[16]xi8>, vector<[4]xi1>, i32) -> vector<[3]xf32>
31+
llvm.return %res : vector<[3]xf32>
32+
}
33+
34+
// -----
35+
36+
llvm.func @arm_sme_tile_slice_to_vector_invalid_element_types(
37+
%tileslice : i32, %nxv4i1 : vector<[4]xi1>, %nxv4f32 : vector<[4]xf32>
38+
) -> vector<[3]xi32> {
39+
%res = "arm_sme.intr.read.horiz"(%nxv4f32, %nxv4i1, %tileslice) <{tile_id = 0 : i32}> :
40+
(vector<[4]xf32>, vector<[4]xi1>, i32) -> vector<[4]xi32>
41+
llvm.return %res : vector<[4]xi32>
42+
}
43+
44+
// -----
45+
46+
llvm.func @arm_sme_streaming_vl_invalid_return_type() -> i32 {
47+
%res = "arm_sme.intr.cntsb"() : () -> i32
48+
llvm.return %res : i32
49+
}

0 commit comments

Comments
 (0)