move passes options to their own thing (#1363)

manbearian · tungld · web-flow · commit 1081a5e8b9a7 · 2022-04-21T22:39:20.000+09:00
Signed-off-by: Ian Bearman &lt;ianb@microsoft.com&gt;

Co-authored-by: Tung D. Le &lt;tung@jp.ibm.com&gt;
diff --git a/docs/Options.md b/docs/Options.md
@@ -5,26 +5,16 @@
 Command-line options can be used to alter the default behavior of onnx-mlir, or onnx-mlir-opt, and help user experimenting, debugging or performance tuning. We implemented command-line in ONNX-MLIR based on the command-line utility provided by LLVM. We did not define `Option` or `ListOption` with MLIR pass classes(see discussion). 
  
 ## Organize Options
-Refer [llvm document](https://llvm.org/docs/CommandLine.html) for basic idea of how to define an option. In ONNX-MLIR, options are put into groups (`llvm::cl::OptionCategory`).
-One group of options are only used by onnx-mlir to configure its input or output. These options are defined in src/main.cpp and src/Compiler/CompilerUtils.cpp within OnnxMlirOptions category.
-The rest of options may be used by both onnx-mlir and onnx-mlir-opt to control the behavior of a pass or passes. So far, only one group is defined as an example. 
+Refer [llvm document](https://llvm.org/docs/CommandLine.html) for basic idea of how to define an option. In ONNX-MLIR, options are put into groups (`llvm::cl::OptionCategory`). All command-line options for onnx-mlir are in the `OnnxMlirOptions` group.
 
 ## Code structure
-The head file for options is `src/Support/OMOptions.hpp`. This file should contain the declaration of groups used by both onnx-mlir and onnx-mlir-opt, and options that may be shared by different passes.
-The definition of group and shared options are in `src/Support/OMOptions.cpp`.
+Command-line options should be placed in `src/Compiler/CompilerOptions.cpp` and declared in `src/Compiler/CompilerOptions.hpp`.
 
 ## Define an option
-* Add a declaration of the option in 'src/Support/OMOptions.hpp`
-* In `src/SupportOMOptions.cpp`, define the option with the desirable llvm::cl class and option group
-* For the file to use this option, make sure `src/Support/OMOptions.hpp` is included.
-* Add OMSupport to the list of libraries in CMakefile.txt
+- Add a declaration of the option in `src/Compiler/CompilerOptions.hpp`
+- In `src/Compiler/CompilerOptions.cpp`, define the option
+- Do **not** include `src/Compiler/CompilerOptions.hpp` in new source files; it should only be used in the onnx-mlir and onnn-mlir-opt command-line tools.
+- Do create 'Pass Options' to pass information to specific passses and transformations
 
 ## Define an option local to a transformation
-If an option is only used in one transformation,  it can be defined locally in the file for the transformation. In the file for transformation:
-* Include `src/Support/OMOtions.hpp`
-* Define the option with the desirable llvm::cl class and option group
-* Add OMSupport to the list of libraries in CMakefile.txt
-
-## Discussion
-### MLIR Option support
-MLIR allows to define options within the pass class definition. However, the document said you have to define the same options for PassPipeline and pass a lambda function to assign the options. I thought that this requirement complicated code. I am open to using MLIR if someone know how to easily implement it and know its advantages.
+Use MLIR's Pass Options to configure passes.
diff --git a/include/onnx-mlir/Compiler/OMCompilerTypes.h b/include/onnx-mlir/Compiler/OMCompilerTypes.h
@@ -41,6 +41,14 @@ typedef enum {
   LLVMFlag,         /* Kind for -mllvm string. */
 } OptionKind;
 
+/* Compiler options to describe instrumentation options */
+typedef enum {
+  InstrumentBeforeOp,
+  InstrumentAfterOp,
+  InstrumentReportTime,
+  InstrumentReportMemory
+} InstrumentActions;
+
 #ifdef __cplusplus
 } // namespace onnx_mlir
 #endif
diff --git a/src/Accelerators/NNPA/Compiler/NNPACompilerUtils.cpp b/src/Accelerators/NNPA/Compiler/NNPACompilerUtils.cpp
@@ -73,7 +73,8 @@ void addONNXToZHighPasses(
   pm.addNestedPass<FuncOp>(onnx_mlir::createConstPropONNXToONNXPass());
   // Add instrumentation for Onnx Ops in the same way as onnx-mlir.
   if (instrumentZHighOps == "" || instrumentZHighOps == "NONE")
-    pm.addNestedPass<FuncOp>(onnx_mlir::createInstrumentONNXPass());
+    pm.addNestedPass<FuncOp>(onnx_mlir::createInstrumentONNXPass(
+        instrumentONNXOps, instrumentControlBits.getBits()));
   pm.addPass(onnx_mlir::createONNXToZHighPass(execNodesOnCpu));
   pm.addPass(onnx_mlir::createShapeInferencePass());
   // There are more opportunities for const propagation once all zhigh ops were
diff --git a/src/Accelerators/NNPA/Support/OMNNPAOptions.cpp b/src/Accelerators/NNPA/Support/OMNNPAOptions.cpp
@@ -2,7 +2,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-//===--------------------------- OMOptions.cpp ----------------------------===//
+//===--------------------------- OMNNPAOptions.cpp ------------------------===//
 //
 // Copyright 2019-2021 The IBM Research Authors.
 //
diff --git a/src/Compiler/CMakeLists.txt b/src/Compiler/CMakeLists.txt
@@ -35,7 +35,7 @@ add_onnx_mlir_library(CompilerOptions
 
   INCLUDE_DIRS PRIVATE
   ${FILE_GENERATE_DIR}
-  
+
   INCLUDE_DIRS PUBLIC
   ${ONNX_MLIR_SRC_ROOT}/include
 
@@ -53,7 +53,7 @@ add_onnx_mlir_library(CompilerPasses
 
   INCLUDE_DIRS PRIVATE
   ${FILE_GENERATE_DIR}
-  
+
   INCLUDE_DIRS PUBLIC
   ${ONNX_MLIR_SRC_ROOT}/include
 
@@ -73,6 +73,8 @@ endif()
 add_onnx_mlir_library(CompilerUtils
   CompilerUtils.cpp
 
+  EXCLUDE_FROM_OM_LIBS
+
   DEPENDS
   ExternalUtil
   MLIRIR
diff --git a/src/Compiler/CompilerOptions.cpp b/src/Compiler/CompilerOptions.cpp
@@ -123,32 +123,40 @@ llvm::cl::opt<OptLevel> OptimizationLevel(
         clEnumVal(O3, "Optimization level 3.")),
     llvm::cl::init(O0), llvm::cl::cat(OnnxMlirOptions));
 
-llvm::cl::OptionCategory OMPassOptions("ONNX-MLIR Pass Options",
-    "These are options to provide fine control on passes");
-
 llvm::cl::opt<std::string> instrumentONNXOps("instrument-onnx-ops",
     llvm::cl::desc("Specify onnx ops to be instrumented\n"
                    "\"NONE\" or \"\" for no instrument\n"
                    "\"ALL\" for all ops. \n"
                    "\"op1 op2 ...\" for the specified ops."),
-    llvm::cl::init(""), llvm::cl::cat(OMPassOptions));
+    llvm::cl::init(""), llvm::cl::cat(OnnxMlirOptions));
+
+llvm::cl::bits<InstrumentActions> instrumentControlBits(
+    llvm::cl::desc("Specify what instrumentation actions at runtime:"),
+    llvm::cl::values(
+        clEnumVal(InstrumentBeforeOp, "insert instrument before op"),
+        clEnumVal(InstrumentAfterOp, "insert instrument after op"),
+        clEnumVal(
+            InstrumentReportTime, "instrument runtime reports time usage"),
+        clEnumVal(
+            InstrumentReportMemory, "instrument runtime reports memory usage")),
+    llvm::cl::cat(OnnxMlirOptions));
 
 llvm::cl::opt<bool> enableMemoryBundling("enable-memory-bundling",
     llvm::cl::desc(
         "Enable memory bundling related optimizations (default=false)\n"
         "Set to 'false' if you experience significant compile time."),
-    llvm::cl::init(false), llvm::cl::cat(OMPassOptions));
+    llvm::cl::init(false), llvm::cl::cat(OnnxMlirOptions));
 
 llvm::cl::opt<int> onnxOpTransformThreshold("onnx-op-transform-threshold",
     llvm::cl::desc(
         "Max iteration for dynamic op transform passes (default=3).\n"
         "If set to 0, onnxOpTransformPass will be disabled, and\n"
         "static iteration will be used"),
-    llvm::cl::init(3), llvm::cl::cat(OMPassOptions));
+    llvm::cl::init(3), llvm::cl::cat(OnnxMlirOptions));
 
 llvm::cl::opt<bool> onnxOpTransformReport("onnx-op-transform-report",
     llvm::cl::desc("Report diagnostic info for op transform passes."),
-    llvm::cl::init(false), llvm::cl::cat(OMPassOptions));
+    llvm::cl::init(false), llvm::cl::cat(OnnxMlirOptions));
 
 // Configuration states associated with certain options.
 // For example, when maccel is specified, NNPA can register
diff --git a/src/Compiler/CompilerOptions.hpp b/src/Compiler/CompilerOptions.hpp
@@ -25,7 +25,6 @@ extern const std::string OnnxMlirEnvOptionName;
 namespace onnx_mlir {
 extern llvm::cl::OptionCategory OnnxMlirOptions;
 
-extern llvm::cl::opt<std::string> instrumentONNXOps;
 extern llvm::cl::opt<bool> invokeOnnxVersionConverter;
 extern llvm::cl::opt<bool> preserveLocations;
 extern llvm::cl::opt<bool> printIR;
@@ -45,8 +44,8 @@ extern llvm::cl::opt<std::string> Xopt;
 extern llvm::cl::opt<std::string> Xllc;
 extern llvm::cl::opt<std::string> mllvm;
 
-extern llvm::cl::OptionCategory OMPassOptions;
 extern llvm::cl::opt<std::string> instrumentONNXOps;
+extern llvm::cl::bits<InstrumentActions> instrumentControlBits;
 extern llvm::cl::opt<bool> enableMemoryBundling;
 extern llvm::cl::opt<int> onnxOpTransformThreshold;
 extern llvm::cl::opt<bool> onnxOpTransformReport;
diff --git a/src/Compiler/CompilerPasses.cpp b/src/Compiler/CompilerPasses.cpp
@@ -53,7 +53,8 @@ void addONNXToMLIRPasses(mlir::PassManager &pm) {
 
   if (onnxOpTransformThreshold > 0) {
     // Dynamic iterate in ONNXOpTransformPass
-    pm.addPass(onnx_mlir::createONNXOpTransformPass(onnxOpTransformThreshold));
+    pm.addPass(onnx_mlir::createONNXOpTransformPass(
+        onnxOpTransformThreshold, onnxOpTransformReport));
   } else {
     // Statically add extra passes
     for (int i = 0; i < repeatOnnxTransform; i++) {
@@ -75,7 +76,8 @@ void addONNXToKrnlPasses(mlir::PassManager &pm, int optLevel, bool enableCSE) {
   // Verify ONNX ops before lowering to Krnl.
   pm.addNestedPass<FuncOp>(onnx_mlir::createONNXPreKrnlVerifyPass());
   // Add instrumentation for Onnx Ops
-  pm.addNestedPass<FuncOp>(onnx_mlir::createInstrumentONNXPass());
+  pm.addNestedPass<FuncOp>(onnx_mlir::createInstrumentONNXPass(
+      instrumentONNXOps, instrumentControlBits.getBits()));
   pm.addPass(onnx_mlir::createLowerToKrnlPass(optLevel));
   // An additional pass of canonicalization is helpful because lowering
   // from ONNX dialect to Standard dialect exposes additional canonicalization
diff --git a/src/Pass/Passes.hpp b/src/Pass/Passes.hpp
@@ -24,7 +24,8 @@ namespace onnx_mlir {
 
 /// Pass for ONNX graph level optimization
 std::unique_ptr<mlir::Pass> createONNXOpTransformPass();
-std::unique_ptr<mlir::Pass> createONNXOpTransformPass(int threshold);
+std::unique_ptr<mlir::Pass> createONNXOpTransformPass(
+    int threshold, bool report);
 
 /// Pass for rewriting inside frontend dialect.
 std::unique_ptr<mlir::Pass> createDecomposeONNXToONNXPass();
@@ -39,6 +40,8 @@ std::unique_ptr<mlir::Pass> createElideConstantValuePass();
 
 /// Pass for instrument the Onnx ops
 std::unique_ptr<mlir::Pass> createInstrumentONNXPass();
+std::unique_ptr<mlir::Pass> createInstrumentONNXPass(
+    llvm::StringRef ops, int actions);
 
 /// Pass for verifying Onnx ops before lowering to Krnl
 std::unique_ptr<mlir::Pass> createONNXPreKrnlVerifyPass();
diff --git a/src/Support/OMOptions.cpp b/src/Support/OMOptions.cpp
diff --git a/src/Transform/ONNX/CMakeLists.txt b/src/Transform/ONNX/CMakeLists.txt
@@ -43,8 +43,10 @@ add_onnx_mlir_library(OMShapeInference
 add_onnx_mlir_library(OMInstrumentONNX
   InstrumentONNXPass.cpp
 
+  INCLUDE_DIRS PUBLIC
+  ${ONNX_MLIR_SRC_ROOT}/include
+
   LINK_LIBS PUBLIC
-  CompilerOptions
   OMONNXOps
   OMKrnlOps
   MLIRPass
@@ -54,7 +56,6 @@ add_onnx_mlir_library(OMOpTransform
   ONNXOpTransformPass.cpp
 
   LINK_LIBS PUBLIC
-  CompilerOptions
   OMONNXOps
   MLIRPass
   OMONNXRewrite
diff --git a/src/Transform/ONNX/InstrumentONNXPass.cpp b/src/Transform/ONNX/InstrumentONNXPass.cpp
diff --git a/src/Transform/ONNX/ONNXOpTransformPass.cpp b/src/Transform/ONNX/ONNXOpTransformPass.cpp
