[mlir-reduce] Improve diagnostic message and clean build dependency

[rriddle]

Looks good, thanks!

mlir/lib/Reducer/ReductionTreePass.cpp

87

nit: diagnostics should start with a lower case.

157–160

?

mlir/lib/Tools/mlir-reduce/MlirReduceMain.cpp

53

Should we just register all of the passes? Is there a reason to do just the one?

[Chia-hungDuan]

Addressed review comments, added a OptionCategory for mlir-reduce and hid the unrelated options
Now only mlir passes will be shown.

mlir/lib/Reducer/OptReductionPass.cpp

void OptReductionPass::runOnOperation() {
 
   Tester test(testerName, testerArgs);
 
   ModuleOp module = this->getOperation();
   ModuleOp moduleVariant = module.clone();
 
   PassManager passManager(module.getContext());
   if (failed(parsePassPipeline(optPass, passManager))) {
-    LLVM_DEBUG(llvm::dbgs() << "\nFailed to parse pass pipeline");
-    return;
+    module.emitError() << "\nFailed to parse pass pipeline";
+    return signalPassFailure();
   }
 
   std::pair<Tester::Interestingness, int> original = test.isInteresting(module);
   if (original.first != Tester::Interestingness::True) {
-    LLVM_DEBUG(llvm::dbgs() << "\nThe original input is not interested");
-    return;
+    module.emitError() << "\nThe original input is not interested";
+    return signalPassFailure();
   }
 
   if (failed(passManager.run(moduleVariant))) {
-    LLVM_DEBUG(llvm::dbgs() << "\nFailed to run pass pipeline");
-    return;
+    module.emitError() << "\nFailed to run pass pipeline";
+    return signalPassFailure();
   }
 
   std::pair<Tester::Interestingness, int> reduced =
       test.isInteresting(moduleVariant);
 
   if (reduced.first == Tester::Interestingness::True &&
       reduced.second < original.second) {
     module.getBody()->clear();

mlir/lib/Reducer/ReductionTreePass.cpp

 /// We will apply the reducer patterns to the operations in the ranges specified
 /// by ReductionNode. Note that we are not able to remove an operation without
 /// replacing it with another valid operation. However, The validity of module
 /// reduction is based on the Tester provided by the user and that means certain
 /// invalid module is still interested by the use. Thus we provide an
 /// alternative way to remove operations, which is using `eraseOpNotInRange` to
 /// erase the operations not in the range specified by ReductionNode.
 template <typename IteratorType>
-static void findOptimal(ModuleOp module, Region &region,
+static LogicalResult findOptimal(ModuleOp module, Region &region,
                                  const FrozenRewritePatternSet &patterns,
                                  const Tester &test, bool eraseOpNotInRange) {
   std::pair<Tester::Interestingness, size_t> initStatus =
       test.isInteresting(module);
   // While exploring the reduction tree, we always branch from an interesting
   // node. Thus the root node must be interesting.
   if (initStatus.first != Tester::Interestingness::True)
-    return;
+    return module.emitWarning() << "Uninterested module will not be reduced";
 
   llvm::SpecificBumpPtrAllocator<ReductionNode> allocator;
 
   std::vector<ReductionNode::Range> ranges{
       {0, std::distance(region.op_begin(), region.op_end())}};
 
   ReductionNode *root = allocator.Allocate();
   new (root) ReductionNode(nullptr, std::move(ranges), allocator);
while (!trace.empty()) {
     applyPatterns(region, patterns, top->getStartRanges(), eraseOpNotInRange);
   }
 
   if (test.isInteresting(module).first != Tester::Interestingness::True)
     llvm::report_fatal_error("Reduced module is not interesting");
   if (test.isInteresting(module).second != smallestNode->getSize())
     llvm::report_fatal_error(
         "Reduced module doesn't have consistent size with smallestNode");
+  return success();
 }
 
 template <typename IteratorType>
-static void findOptimal(ModuleOp module, Region &region,
+static LogicalResult findOptimal(ModuleOp module, Region &region,
                                  const FrozenRewritePatternSet &patterns,
                                  const Tester &test) {
   // We separate the reduction process into 2 steps, the first one is to erase
   // redundant operations and the second one is to apply the reducer patterns.
 
   // In the first phase, we don't apply any patterns so that we only select the
   // range of operations to keep to the module stay interesting.
-  findOptimal<IteratorType>(module, region, /*patterns=*/{}, test,
-                            /*eraseOpNotInRange=*/true);
+  if (failed(findOptimal<IteratorType>(module, region, /*patterns=*/{}, test,
+                                       /*eraseOpNotInRange=*/true)))
+    return failure();
   // In the second phase, we suppose that no operation is redundant, so we try
   // to rewrite the operation into simpler form.
-  findOptimal<IteratorType>(module, region, patterns, test,
-                            /*eraseOpNotInRange=*/false);
+  if (failed(findOptimal<IteratorType>(module, region, patterns, test,
+                                       /*eraseOpNotInRange=*/false)))
+    return failure();
+  return success();
 }
 
 namespace {
 
 //===----------------------------------------------------------------------===//
 // Reduction Pattern Interface Collection
 //===----------------------------------------------------------------------===//
 
public:
   ReductionTreePass(const ReductionTreePass &pass) = default;
 
   LogicalResult initialize(MLIRContext *context) override;
 
   /// Runs the pass instance in the pass pipeline.
   void runOnOperation() override;
 
 private:
-  void reduceOp(ModuleOp module, Region &region);
+  LogicalResult reduceOp(ModuleOp module, Region &region);
 
   FrozenRewritePatternSet reducerPatterns;
 };
 
 } // end anonymous namespace
 
 LogicalResult ReductionTreePass::initialize(MLIRContext *context) {
   RewritePatternSet patterns(context);
void ReductionTreePass::runOnOperation() {
   SmallVector<Operation *, 8> workList;
   workList.push_back(getOperation());
 
   do {
     Operation *op = workList.pop_back_val();
 
     for (Region &region : op->getRegions())
       if (!region.empty())
-        reduceOp(module, region);
+        if (failed(reduceOp(module, region)))
+          return signalPassFailure();
 
     for (Region &region : op->getRegions())
       for (Operation &op : region.getOps())
         if (op.getNumRegions() != 0)
           workList.push_back(&op);
   } while (!workList.empty());
 }
 
-void ReductionTreePass::reduceOp(ModuleOp module, Region &region) {
+LogicalResult ReductionTreePass::reduceOp(ModuleOp module, Region &region) {
   Tester test(testerName, testerArgs);
   switch (traversalModeId) {
   case TraversalMode::SinglePath:
-    findOptimal<ReductionNode::iterator<TraversalMode::SinglePath>>(
+    return findOptimal<ReductionNode::iterator<TraversalMode::SinglePath>>(
         module, region, reducerPatterns, test);
-    break;
   default:
-    llvm_unreachable("Unsupported mode");
+    return module.emitError() << "Unsupported traversal mode detected";
   }
 }
 
 std::unique_ptr<Pass> mlir::createReductionTreePass() {
   return std::make_unique<ReductionTreePass>();
 }

mlir/lib/Tools/mlir-reduce/CMakeLists.txt

-set(LLVM_OPTIONAL_SOURCES
-  MlirReduceMain.cpp
-)
-
-set(LLVM_LINK_COMPONENTS
-  Support
-  )
-
 add_mlir_library(MLIRReduceLib
   MlirReduceMain.cpp
 
   LINK_LIBS PUBLIC
   MLIRIR
   MLIRParser
   MLIRPass
   MLIRReduce
   MLIRSupport
+  MLIRTransforms
   )

mlir/lib/Tools/mlir-reduce/MlirReduceMain.cpp

 #include "mlir/IR/PatternMatch.h"
 #include "mlir/Parser.h"
 #include "mlir/Pass/Pass.h"
 #include "mlir/Pass/PassManager.h"
 #include "mlir/Reducer/Passes.h"
 #include "mlir/Rewrite/FrozenRewritePatternSet.h"
 #include "mlir/Support/FileUtilities.h"
 #include "mlir/Support/LogicalResult.h"
+#include "mlir/Transforms/Passes.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/ToolOutputFile.h"
 
 using namespace mlir;
 
 // Parse and verify the input MLIR file.
 static LogicalResult loadModule(MLIRContext &context, OwningModuleRef &module,
                                 StringRef inputFilename) {
LogicalResult mlir::mlirReduceMain(int argc, char **argv,
 
   static llvm::cl::opt<std::string> outputFilename(
       "o", llvm::cl::desc("Output filename for the reduced test case"),
       llvm::cl::init("-"));
 
   llvm::InitLLVM y(argc, argv);
 
   registerReducerPasses();
-  registerMLIRContextCLOptions();
-  registerPassManagerCLOptions();
+  registerSymbolDCEPass();
 
   PassPipelineCLParser parser("", "Reduction Passes to Run");
   llvm::cl::ParseCommandLineOptions(argc, argv,
                                     "MLIR test case reduction tool.\n");
 
   std::string errorMessage;
 
   auto output = openOutputFile(outputFilename, &errorMessage);

mlir/tools/mlir-reduce/CMakeLists.txt

 get_property(dialect_libs GLOBAL PROPERTY MLIR_DIALECT_LIBS)
-get_property(conversion_libs GLOBAL PROPERTY MLIR_CONVERSION_LIBS)
-
-if(MLIR_INCLUDE_TESTS)
-  set(test_libs
-    MLIRTestDialect
-    )
-endif()
 
 set(LIBS
   ${dialect_libs}
-  ${conversion_libs}
-  ${test_libs}
   MLIRDialect
   MLIRIR
   MLIRPass
   MLIRReduceLib
+  MLIRTestDialect
   )
 
 add_llvm_tool(mlir-reduce
   mlir-reduce.cpp
 
   DEPENDS
   ${LIBS}
   )
 
 target_link_libraries(mlir-reduce PRIVATE ${LIBS})
 llvm_update_compile_flags(mlir-reduce)
 
 mlir_check_all_link_libraries(mlir-reduce)

mlir/tools/mlir-reduce/mlir-reduce.cpp

 // up the testing environment. It  outputs the most reduced test case variant
 // after executing the reduction passes.
 //
 //===----------------------------------------------------------------------===//
 
 #include "mlir/IR/Dialect.h"
 #include "mlir/IR/MLIRContext.h"
 #include "mlir/InitAllDialects.h"
-#include "mlir/InitAllPasses.h"
 #include "mlir/Tools/mlir-reduce/MlirReduceMain.h"
 
 using namespace mlir;
 
 namespace mlir {
 namespace test {
 #ifdef MLIR_INCLUDE_TESTS
 void registerTestDialect(DialectRegistry &);
 #endif
 } // namespace test
 } // namespace mlir
 
 int main(int argc, char **argv) {
-  registerAllPasses();
-
   DialectRegistry registry;
   registerAllDialects(registry);
 #ifdef MLIR_INCLUDE_TESTS
   test::registerTestDialect(registry);
 #endif
   MLIRContext context(registry);
 
   return failed(mlirReduceMain(argc, argv, context));
 }