Skip to content

Commit e769225

Browse files
gryppgrypp
committed
address comments
1 parent a9a0b54 commit e769225

File tree

5 files changed

+73
-44
lines changed

5 files changed

+73
-44
lines changed

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

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -231,29 +231,49 @@ class MlirOptMainConfig {
231231
bool shouldEmitRemarks() const {
232232
// Emit all remarks only when no filters are specified.
233233
const bool hasFilters =
234-
!remarksAllFlag.empty() || !remarksPassedFlag.empty() ||
235-
!remarksFailedFlag.empty() || !remarksMissedFlag.empty() ||
236-
!remarksAnalyseFlag.empty();
234+
!getRemarksAllFilter().empty() || !getRemarksPassedFilter().empty() ||
235+
!getRemarksFailedFilter().empty() ||
236+
!getRemarksMissedFilter().empty() || !getRemarksAnalyseFilter().empty();
237237
return hasFilters;
238238
}
239239

240240
/// Reproducer file generation (no crash required).
241241
StringRef getReproducerFilename() const { return generateReproducerFileFlag; }
242242

243-
/// Remark options
244-
RemarkFormat remarkFormatFlag;
245-
std::string remarksAllFlag = "";
246-
std::string remarksPassedFlag = "";
247-
std::string remarksFailedFlag = "";
248-
std::string remarksMissedFlag = "";
249-
std::string remarksAnalyseFlag = "";
243+
/// Set the reproducer output filename
244+
RemarkFormat getRemarkFormat() const { return remarkFormatFlag; }
245+
/// Set the remark format to use.
246+
std::string getRemarksAllFilter() const { return remarksAllFilterFlag; }
247+
/// Set the remark output file.
248+
std::string getRemarksOutputFile() const { return remarksOutputFileFlag; }
249+
/// Set the remark passed filters.
250+
std::string getRemarksPassedFilter() const { return remarksPassedFilterFlag; }
251+
/// Set the remark failed filters.
252+
std::string getRemarksFailedFilter() const { return remarksFailedFilterFlag; }
253+
/// Set the remark missed filters.
254+
std::string getRemarksMissedFilter() const { return remarksMissedFilterFlag; }
255+
/// Set the remark analyse filters.
256+
std::string getRemarksAnalyseFilter() const {
257+
return remarksAnalyseFilterFlag;
258+
}
250259

251260
protected:
252261
/// Allow operation with no registered dialects.
253262
/// This option is for convenience during testing only and discouraged in
254263
/// general.
255264
bool allowUnregisteredDialectsFlag = false;
256265

266+
/// Remark format
267+
RemarkFormat remarkFormatFlag;
268+
/// Remark file to output to
269+
std::string remarksOutputFileFlag = "";
270+
/// Remark filters
271+
std::string remarksAllFilterFlag = "";
272+
std::string remarksPassedFilterFlag = "";
273+
std::string remarksFailedFilterFlag = "";
274+
std::string remarksMissedFilterFlag = "";
275+
std::string remarksAnalyseFilterFlag = "";
276+
257277
/// Configuration for the debugging hooks.
258278
tracing::DebugConfig debugConfig;
259279

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

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -228,23 +228,35 @@ struct MlirOptMainConfigCLOptions : public MlirOptMainConfig {
228228
static cl::opt<std::string, /*ExternalStorage=*/true> remarksAll(
229229
"remarks",
230230
cl::desc("Show all remarks: passed, missed, failed, analysis"),
231-
cl::location(remarksAllFlag), cl::init(""), cl::cat(remarkCategory));
231+
cl::location(remarksAllFilterFlag), cl::init(""),
232+
cl::cat(remarkCategory));
233+
234+
static cl::opt<std::string, /*ExternalStorage=*/true> remarksFile(
235+
"remarks-output-file",
236+
cl::desc(
237+
"Output file for yaml and bitstream remark formats. Default is "
238+
"mlir-remarks.yaml or mlir-remarks.bitstream"),
239+
cl::location(remarksOutputFileFlag), cl::init(""),
240+
cl::cat(remarkCategory));
232241

233242
static cl::opt<std::string, /*ExternalStorage=*/true> remarksPassed(
234243
"remarks-passed", cl::desc("Show passed remarks"),
235-
cl::location(remarksPassedFlag), cl::init(""), cl::cat(remarkCategory));
244+
cl::location(remarksPassedFilterFlag), cl::init(""),
245+
cl::cat(remarkCategory));
236246

237247
static cl::opt<std::string, /*ExternalStorage=*/true> remarksFailed(
238248
"remarks-failed", cl::desc("Show failed remarks"),
239-
cl::location(remarksFailedFlag), cl::init(""), cl::cat(remarkCategory));
249+
cl::location(remarksFailedFilterFlag), cl::init(""),
250+
cl::cat(remarkCategory));
240251

241252
static cl::opt<std::string, /*ExternalStorage=*/true> remarksMissed(
242253
"remarks-missed", cl::desc("Show missed remarks"),
243-
cl::location(remarksMissedFlag), cl::init(""), cl::cat(remarkCategory));
254+
cl::location(remarksMissedFilterFlag), cl::init(""),
255+
cl::cat(remarkCategory));
244256

245257
static cl::opt<std::string, /*ExternalStorage=*/true> remarksAnalyse(
246258
"remarks-analyse", cl::desc("Show analysis remarks"),
247-
cl::location(remarksAnalyseFlag), cl::init(""),
259+
cl::location(remarksAnalyseFilterFlag), cl::init(""),
248260
cl::cat(remarkCategory));
249261

250262
/// Set the callback to load a pass plugin.
@@ -516,30 +528,34 @@ performActions(raw_ostream &os,
516528
};
517529

518530
remark::RemarkCategories cats{
519-
combine(config.remarksAllFlag, config.remarksPassedFlag),
520-
combine(config.remarksAllFlag, config.remarksMissedFlag),
521-
combine(config.remarksAllFlag, config.remarksAnalyseFlag),
522-
combine(config.remarksAllFlag, config.remarksFailedFlag)};
531+
combine(config.getRemarksAllFilter(), config.getRemarksPassedFilter()),
532+
combine(config.getRemarksAllFilter(), config.getRemarksMissedFilter()),
533+
combine(config.getRemarksAllFilter(), config.getRemarksAnalyseFilter()),
534+
combine(config.getRemarksAllFilter(), config.getRemarksFailedFilter())};
523535

524536
mlir::MLIRContext &ctx = *context;
525537

526-
switch (config.remarkFormatFlag) {
538+
switch (config.getRemarkFormat()) {
527539
case REMARK_FORMAT_STDOUT:
528540
if (failed(mlir::remark::enableOptimizationRemarks(
529541
ctx, nullptr, cats, true /*printAsEmitRemarks*/)))
530542
return failure();
531543
break;
532544

533545
case REMARK_FORMAT_YAML: {
534-
constexpr llvm::StringLiteral file{"mlir-remarks.yaml"};
546+
std::string file = config.getRemarksOutputFile().empty()
547+
? "mlir-remarks.yaml"
548+
: config.getRemarksOutputFile();
535549
if (failed(mlir::remark::enableOptimizationRemarksWithLLVMStreamer(
536550
ctx, file, llvm::remarks::Format::YAML, cats)))
537551
return failure();
538552
break;
539553
}
540554

541555
case REMARK_FORMAT_BITSTREAM: {
542-
constexpr llvm::StringLiteral file{"mlir-remarks.bitstream"};
556+
std::string file = config.getRemarksOutputFile().empty()
557+
? "mlir-remarks.bitstream"
558+
: config.getRemarksOutputFile();
543559
if (failed(mlir::remark::enableOptimizationRemarksWithLLVMStreamer(
544560
ctx, file, llvm::remarks::Format::Bitstream, cats)))
545561
return failure();

‎mlir/test/Pass/remarks.mlir‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
// RUN: mlir-opt %s --test-remark-pipeline --remarks-passed="category" 2>&1 | FileCheck %s -check-prefix=CHECK-PASSED
2-
// RUN: mlir-opt %s --test-remark-pipeline --remarks-missed="category" 2>&1 | FileCheck %s -check-prefix=CHECK-MISSED
3-
// RUN: mlir-opt %s --test-remark-pipeline --remarks-failed="category" 2>&1 | FileCheck %s -check-prefix=CHECK-FAILED
4-
// RUN: mlir-opt %s --test-remark-pipeline --remarks-analyse="category" 2>&1 | FileCheck %s -check-prefix=CHECK-ANALYSIS
5-
// RUN: mlir-opt %s --test-remark-pipeline --remarks="category" 2>&1 | FileCheck %s -check-prefix=CHECK-ALL
6-
// RUN: mlir-opt %s --test-remark-pipeline --remarks="category-1" 2>&1 | FileCheck %s -check-prefix=CHECK-ALL1
1+
// RUN: mlir-opt %s --test-remark --remarks-passed="category" 2>&1 | FileCheck %s -check-prefix=CHECK-PASSED
2+
// RUN: mlir-opt %s --test-remark --remarks-missed="category" 2>&1 | FileCheck %s -check-prefix=CHECK-MISSED
3+
// RUN: mlir-opt %s --test-remark --remarks-failed="category" 2>&1 | FileCheck %s -check-prefix=CHECK-FAILED
4+
// RUN: mlir-opt %s --test-remark --remarks-analyse="category" 2>&1 | FileCheck %s -check-prefix=CHECK-ANALYSIS
5+
// RUN: mlir-opt %s --test-remark --remarks="category" 2>&1 | FileCheck %s -check-prefix=CHECK-ALL
6+
// RUN: mlir-opt %s --test-remark --remarks="category-1" 2>&1 | FileCheck %s -check-prefix=CHECK-ALL1
77
module @foo {
88
"test.op"() : () -> ()
99
"test.op2"() : () -> ()
@@ -46,3 +46,5 @@ module @foo {
4646
// CHECK-ALL1: remarks.mlir:9:3: remark: [Passed] test-remark | Category:category-1-passed | Reason="because we are testing the remark pipeline", Remark="This is a test passed remark", Suggestion="try using the remark pipeline feature"
4747
// CHECK-ALL1: remarks.mlir:7:1: remark: [Missed] test-remark | Category:category-1-missed | Reason="because we are testing the remark pipeline", Remark="This is a test missed remark", Suggestion="try using the remark pipeline feature"
4848
// CHECK-ALL1: remarks.mlir:7:1: remark: [Passed] test-remark | Category:category-1-passed | Reason="because we are testing the remark pipeline", Remark="This is a test passed remark", Suggestion="try using the remark pipeline feature"
49+
// CHECK-ALL1-NOT: remarks.mlir:8:3: remark: [Failure]
50+
// CHECK-ALL1-NOT: remarks.mlir:8:3: remark: [Analysis]

‎mlir/test/lib/Pass/TestRemarksPipeline.cpp‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,16 @@ using namespace mlir;
2020

2121
namespace {
2222

23-
class TestRemarkPipelinePass
24-
: public PassWrapper<TestRemarkPipelinePass, OperationPass<>> {
23+
class TestRemarkPass : public PassWrapper<TestRemarkPass, OperationPass<>> {
2524
public:
26-
MLIR_DEFINE_EXPLICIT_INTERNAL_INLINE_TYPE_ID(TestRemarkPipelinePass)
25+
MLIR_DEFINE_EXPLICIT_INTERNAL_INLINE_TYPE_ID(TestRemarkPass)
2726

28-
StringRef getArgument() const final { return "test-remark-pipeline"; }
27+
StringRef getArgument() const final { return "test-remark"; }
2928
StringRef getDescription() const final {
3029
return "Tests the remark pipeline feature";
3130
}
32-
void getDependentDialects(DialectRegistry &registry) const override {
33-
OpPassManager pm(ModuleOp::getOperationName(),
34-
OpPassManager::Nesting::Implicit);
3531

36-
pm.getDependentDialects(registry);
37-
}
38-
39-
TestRemarkPipelinePass() = default;
32+
TestRemarkPass() = default;
4033

4134
void runOnOperation() override {
4235

@@ -73,8 +66,6 @@ class TestRemarkPipelinePass
7366

7467
namespace mlir {
7568
namespace test {
76-
void registerTestRemarkPipelinePass() {
77-
PassRegistration<TestRemarkPipelinePass>();
78-
}
69+
void registerTestRemarkPass() { PassRegistration<TestRemarkPass>(); }
7970
} // namespace test
8071
} // namespace mlir

‎mlir/tools/mlir-opt/mlir-opt.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ void registerTestDiagnosticsPass();
9797
void registerTestDiagnosticsMetadataPass();
9898
void registerTestDominancePass();
9999
void registerTestDynamicPipelinePass();
100-
void registerTestRemarkPipelinePass();
100+
void registerTestRemarkPass();
101101
void registerTestEmulateNarrowTypePass();
102102
void registerTestFooAnalysisPass();
103103
void registerTestComposeSubView();
@@ -244,7 +244,7 @@ void registerTestPasses() {
244244
mlir::test::registerTestDiagnosticsMetadataPass();
245245
mlir::test::registerTestDominancePass();
246246
mlir::test::registerTestDynamicPipelinePass();
247-
mlir::test::registerTestRemarkPipelinePass();
247+
mlir::test::registerTestRemarkPass();
248248
mlir::test::registerTestEmulateNarrowTypePass();
249249
mlir::test::registerTestFooAnalysisPass();
250250
mlir::test::registerTestComposeSubView();

0 commit comments

Comments
 (0)