Timing tool that adds the flag “--enable-timing" for a detailed report #2801

ParamvirSran · 2024-04-15T21:41:54Z

Detailed timing report printed to the terminal for the entire onnx-mlir execution process, including setup in main() in onnx-mlir.cpp, input file processing before compilation, the passes run in the pass manager, and finally, the lower level compilation/generation stages (LLVM, Object, Shared Library, JNI). This tool will print the detailed execution report as a tree of all the timed stages to the terminal with the (wall time) and (relative percentage of program execution) for each timed stage and the total execution time of the onnx-mlir compiler. I used the DefaultTimingManager from llvm-project “timing.cpp” and timing scopes to time and name essential code sections where work is being done.

Example:

jenkins-droid · 2024-04-15T21:42:07Z

Can one of the admins verify this patch?

tungld · 2024-04-16T01:15:48Z

@jenkins-droid test this please

jenkins-droid · 2024-04-17T01:12:56Z

Can one of the admins verify this patch?

…art a detailed timing report for the entire onnx-mlir execution process, including setup in main() in onnx-mlir.cpp, input file processing before compilation, the passes run in the pass manager, and finally, the lower level compilation/generation stages (LLVM, Object, Shared Library, JNI). This tool will print the detailed execution report as a tree of all the timed stages to the terminal with the (wall time) and (relative percentage of program execution) for each timed stage and the total execution time of the onnx-mlir compiler. I used the DefaultTimingManager from llvm-project “timing.cpp” and timing scopes to time and name essential code sections where work is being done. Signed-off-by: Paramvir Sran <[email protected]>

jenkins-droid · 2024-04-22T00:43:32Z

Can one of the admins verify this patch?

jenkins-droid · 2024-04-22T00:44:31Z

Can one of the admins verify this patch?

tungld

Great feature! Thank you very much for working on this!

Just some comments. They are mainly about changing text messages. Please fix DCO also so that it can be merged.

tungld · 2024-04-22T00:52:44Z

CMakeLists.txt

@@ -7,7 +7,7 @@ project(onnx-mlir)

 option(ONNX_MLIR_BUILD_TESTS "Build ONNX-MLIR test executables. If OFF, just generate build targets." ON)
 option(ONNX_MLIR_CCACHE_BUILD "Set to ON for a ccache enabled build." OFF)
-option(ONNX_MLIR_ENABLE_STABLEHLO "Enable StableHLO support." ON)
+option(ONNX_MLIR_ENABLE_STABLEHLO "Enable StableHLO support." OFF)


For the final commit, please change this back to ON.

tungld · 2024-04-22T00:54:08Z

docs/devcontainer-example/devcontainer.json

        "terminal.integrated.defaultProfile.linux": "bash",
        "cmake.mergedCompileCommands": "${workspaceFolder}/compile_commands.json",
        "cmakeExplorer.suiteDelimiter": ".",
        "cmakeExplorer.debugConfig": "(gdb) Launch",
        "editor.formatOnSave": true,
        "[cpp]": {
          "editor.defaultFormatter": "xaver.clang-format",
-          "editor.tabSize": 2
+          "editor.tabSize":2 


Is this space redundant? It looks like an accidental change.

tungld · 2024-04-22T00:57:41Z

src/Compiler/CompilerOptions.cpp

@@ -546,6 +547,12 @@ static llvm::cl::opt<OptReport, true> optReportOpt("opt-report",
        clEnumVal(Simd, "Provide report on how SIMD is applied to ONNX ops.")),
    llvm::cl::init(OptReport::NoReport), llvm::cl::cat(OnnxMlirOptions));

+static llvm::cl::opt<bool, true> enable_timing("enable-timing",
+    llvm::cl::desc("Enable pass timing (default is false)\n"


maybe Enable compile timing is easier to understand.

tungld · 2024-04-22T01:00:10Z

src/Compiler/CompilerOptions.cpp

@@ -579,6 +586,7 @@ static llvm::cl::opt<bool, true> useOldBufferizationOpt("use-old-bufferization",
    llvm::cl::location(useOldBufferization), llvm::cl::init(true),
    llvm::cl::cat(OnnxMlirOptions));

+


Remove this redundant line.

tungld · 2024-04-22T01:01:34Z

src/Compiler/CompilerUtils.cpp

 #include "llvm/Target/TargetMachine.h"
+#include <fstream>
+#include <memory>
+#include <regex>


Could you include these three lines first? They are from C++, and we often include them first before mlir stuffs.

tungld · 2024-04-22T01:09:33Z

src/onnx-mlir.cpp

  initCompilerConfig();

+  // timing manager reporting enabled via "--enable-timing" compiler flag


typos: timing => Timing

tungld · 2024-04-22T01:09:48Z

src/onnx-mlir.cpp

  initCompilerConfig();

+  // timing manager reporting enabled via "--enable-timing" compiler flag
+  // mlir::applyDefaultTimingManagerCLOptions(timingManager);


Should this be removed?

tungld · 2024-04-22T01:10:10Z

src/onnx-mlir.cpp

+  // mlir::applyDefaultTimingManagerCLOptions(timingManager);
+  timingManager.setEnabled(enableTiming);
+  rootScope = timingManager.getRootScope();
+  auto setupScope = rootScope.nest("[onnx-mlir] Preparing for Compilation");


setupScope => setupTiming.

Perhaps, change the message to [onnx-mlir] Loading Dialects

tungld · 2024-04-22T01:11:29Z

src/onnx-mlir.cpp

@@ -71,7 +74,8 @@ int main(int argc, char *argv[]) {
    LLVM_DEBUG(llvm::dbgs() << "multithreading is disabled\n");
  }
  loadDialects(context);
-
+  setupScope.stop();
+  auto inputFileScope = rootScope.nest("[onnx-mlir] Processing of Input File");


inputFileScope => inputFileTiming

I think [onnx-mlir] Importing Input Model to MLIR is easier to understand.

tungld · 2024-04-22T01:17:00Z

timing.diff

@@ -0,0 +1,143 @@
+diff --git a/src/Compiler/CompilerUtils.cpp b/src/Compiler/CompilerUtils.cpp


Please remove this file.

tungld · 2024-04-22T01:23:50Z

@ParamvirSran follow the instruction here to fix DCO: https://github.com/onnx/onnx-mlir/pull/2801/checks?check_run_id=24080750134.

For Clang Format, you can run git clang-format to format the code, see https://github.com/onnx/onnx-mlir/blob/main/docs/Workflow.md#code-style

Signed-off-by: Paramvir Sran <[email protected]>

jenkins-droid · 2024-04-22T03:19:17Z

Can one of the admins verify this patch?

tungld · 2024-04-22T04:37:13Z

@jenkins-droid test this please

Signed-off-by: Paramvir Sran <[email protected]>

jenkins-droid · 2024-04-22T05:31:32Z

Can one of the admins verify this patch?

Signed-off-by: Paramvir Sran <[email protected]>

jenkins-droid · 2024-04-22T05:57:47Z

Can one of the admins verify this patch?

tungld · 2024-04-22T06:03:12Z

@jenkins-droid test this please

Signed-off-by: Paramvir Sran <[email protected]>

jenkins-droid · 2024-04-22T06:41:37Z

Can one of the admins verify this patch?

tungld · 2024-04-22T06:56:31Z

@jenkins-droid test this please

tungld · 2024-04-22T06:57:59Z

src/Compiler/CompilerUtils.cpp

@@ -146,8 +153,7 @@ int Command::exec(std::string wdir) const {
    llvm::errs() << llvm::join(argsRef, " ") << "\n"
                 << "Error message: " << errMsg << "\n"
                 << "Program path: " << _path << "\n"
-                 << "Command execution failed."
-                 << "\n";
+                 << "Command execution failed." << "\n";


Revert this change, it caused clang-format failed.

tungld · 2024-04-22T06:58:07Z

src/Compiler/CompilerUtils.cpp

@@ -568,8 +578,7 @@ static int compileModuleToJniJar(
 #define NOEXECSTACK                                                            \
  {}
 #else
-#define NOEXECSTACK                                                            \
-  { "-z", "noexecstack" }
+#define NOEXECSTACK {"-z", "noexecstack"}


Revert this change, it caused clang-format failed.

tungld · 2024-04-22T06:58:37Z

src/Dialect/ONNX/ElementsAttr/ElementsAttrBuilder.cpp

-auto ElementsAttrBuilder::getElementsProperties(ElementsAttr elements)
-    -> ElementsProperties {
+auto ElementsAttrBuilder::getElementsProperties(
+    ElementsAttr elements) -> ElementsProperties {


Revert this change, it caused clang-format failed.

tungld · 2024-04-22T06:59:15Z

src/Compiler/CompilerUtils.cpp

@@ -49,6 +53,9 @@
 using namespace mlir;
 using namespace onnx_mlir;

+mlir::DefaultTimingManager timingManager;
+mlir::TimingScope rootScope;


Please change rootScope to rootTimingScope.

Signed-off-by: Paramvir Sran <[email protected]>

jenkins-droid · 2024-04-22T22:53:08Z

Can one of the admins verify this patch?

Signed-off-by: Paramvir Sran <[email protected]>

jenkins-droid · 2024-04-22T23:30:58Z

Can one of the admins verify this patch?

tungld

LGTM! Thanks for adding this useful option!

Signed-off-by: Paramvir Sran <[email protected]>

jenkins-droid · 2024-04-25T02:40:33Z

Can one of the admins verify this patch?

Signed-off-by: Paramvir Sran <[email protected]>

jenkins-droid · 2024-04-25T02:57:48Z

Can one of the admins verify this patch?

tungld · 2024-04-25T03:38:02Z

@jenkins-droid test this please

jenkins-droid · 2024-04-25T05:06:24Z

Jenkins Linux s390x Build #14756 [push] Timing tool that adds th... started at 01:06

jenkins-droid · 2024-04-25T05:06:25Z

Jenkins Linux ppc64le Build #13751 [push] Timing tool that adds th... started at 01:17

jenkins-droid · 2024-04-25T05:06:28Z

Jenkins Linux amd64 Build #14726 [push] Timing tool that adds th... started at 00:06

jenkins-droid · 2024-04-25T06:17:56Z

Jenkins Linux amd64 Build #14726 [push] Timing tool that adds th... passed after 1 hr 11 min

jenkins-droid · 2024-04-25T06:29:58Z

Jenkins Linux s390x Build #14756 [push] Timing tool that adds th... passed after 1 hr 23 min

jenkins-droid · 2024-04-25T07:06:16Z

Jenkins Linux ppc64le Build #13751 [push] Timing tool that adds th... passed after 1 hr 59 min

ParamvirSran force-pushed the pass-timing branch from 7480da9 to b6d7fb0 Compare April 22, 2024 00:43

tungld reviewed Apr 22, 2024

View reviewed changes

Applying changes from Tung

c7f84b1

Signed-off-by: Paramvir Sran <[email protected]>

ParamvirSran force-pushed the pass-timing branch from b6d7fb0 to c7f84b1 Compare April 22, 2024 03:19

clang-formatting src/Compiler/CompilerUtils.cpp

056dd74

Signed-off-by: Paramvir Sran <[email protected]>

Working on improving PR based on Tung's reviewing

3c015f0

Signed-off-by: Paramvir Sran <[email protected]>

Edited PR based on comments from Tung

68f25f2

Signed-off-by: Paramvir Sran <[email protected]>

tungld reviewed Apr 22, 2024

View reviewed changes

Fixed several formatting issues

f95a047

Signed-off-by: Paramvir Sran <[email protected]>

added back linking shared lib timing after accidently removing it

69cba4e

Signed-off-by: Paramvir Sran <[email protected]>

Merge branch 'main' into pass-timing

fff854f

tungld approved these changes Apr 25, 2024

View reviewed changes

clang format on previously changed files

d8f2812

Signed-off-by: Paramvir Sran <[email protected]>

clang-format changing again after failing github action

7f49357

Signed-off-by: Paramvir Sran <[email protected]>

tungld merged commit 060b370 into onnx:main Apr 25, 2024
6 of 7 checks passed

		@@ -579,6 +586,7 @@ static llvm::cl::opt<bool, true> useOldBufferizationOpt("use-old-bufferization",
		llvm::cl::location(useOldBufferization), llvm::cl::init(true),
		llvm::cl::cat(OnnxMlirOptions));

		initCompilerConfig();

		// timing manager reporting enabled via "--enable-timing" compiler flag

		@@ -0,0 +1,143 @@
		diff --git a/src/Compiler/CompilerUtils.cpp b/src/Compiler/CompilerUtils.cpp

Timing tool that adds the flag “--enable-timing" for a detailed report #2801

Timing tool that adds the flag “--enable-timing" for a detailed report #2801

Conversation

ParamvirSran commented Apr 15, 2024

jenkins-droid commented Apr 15, 2024

tungld commented Apr 16, 2024

jenkins-droid commented Apr 17, 2024

jenkins-droid commented Apr 22, 2024

jenkins-droid commented Apr 22, 2024

tungld left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tungld commented Apr 22, 2024

jenkins-droid commented Apr 22, 2024

tungld commented Apr 22, 2024

jenkins-droid commented Apr 22, 2024

jenkins-droid commented Apr 22, 2024

tungld commented Apr 22, 2024

jenkins-droid commented Apr 22, 2024

tungld commented Apr 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenkins-droid commented Apr 22, 2024

jenkins-droid commented Apr 22, 2024

tungld left a comment

Choose a reason for hiding this comment

jenkins-droid commented Apr 25, 2024

jenkins-droid commented Apr 25, 2024

tungld commented Apr 25, 2024

jenkins-droid commented Apr 25, 2024

jenkins-droid commented Apr 25, 2024

jenkins-droid commented Apr 25, 2024

jenkins-droid commented Apr 25, 2024

jenkins-droid commented Apr 25, 2024

jenkins-droid commented Apr 25, 2024