Skip to content

[mlir][target] RAII wrap moduleToObject timer to ensure call clear function #136142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

MikaOvO
Copy link
Contributor

@MikaOvO MikaOvO commented Apr 17, 2025

As title, we need to call Timer::clear to avoid extra log like this:

===-------------------------------------------------------------------------===
                           ...
===-------------------------------------------------------------------------===
  Total Execution Time: 0.0000 seconds (0.0000 wall clock)

   ---Wall Time---  --- Name ---
        -----       ....
        -----       Total

@llvmbot
Copy link
Member

llvmbot commented Apr 17, 2025

@llvm/pr-subscribers-mlir

Author: Zichen Lu (MikaOvO)

Changes

As title, we need to call Timer::clear to avoid extra log like this:

===-------------------------------------------------------------------------===
                           ...
===-------------------------------------------------------------------------===
  Total Execution Time: 0.0000 seconds (0.0000 wall clock)

   ---Wall Time---  --- Name ---
        -----       ....
        -----       Total

Full diff: https://github.com/llvm/llvm-project/pull/136142.diff

1 Files Affected:

  • (modified) mlir/lib/Target/LLVM/NVVM/Target.cpp (+19-11)
diff --git a/mlir/lib/Target/LLVM/NVVM/Target.cpp b/mlir/lib/Target/LLVM/NVVM/Target.cpp
index 5a2b829fbd1bd..ea0c2b6663fbb 100644
--- a/mlir/lib/Target/LLVM/NVVM/Target.cpp
+++ b/mlir/lib/Target/LLVM/NVVM/Target.cpp
@@ -669,10 +669,6 @@ NVPTXSerializer::compileToBinaryNVPTX(const std::string &ptxCode) {
 
 std::optional<SmallVector<char, 0>>
 NVPTXSerializer::moduleToObject(llvm::Module &llvmModule) {
-  llvm::Timer moduleToObjectTimer(
-      "moduleToObjectTimer",
-      "Timer for perf llvm-ir -> isa and isa -> binary.");
-  moduleToObjectTimer.startTimer();
   // Return LLVM IR if the compilation target is `offload`.
 #define DEBUG_TYPE "serialize-to-llvm"
   LLVM_DEBUG({
@@ -691,6 +687,18 @@ NVPTXSerializer::moduleToObject(llvm::Module &llvmModule) {
   return std::nullopt;
 #endif // LLVM_HAS_NVPTX_TARGET
 
+  auto timerDeleter = [](llvm::Timer *timer) {
+    if (timer != nullptr) {
+      timer->clear();
+      delete timer;
+    }
+  };
+  std::unique_ptr<llvm::Timer, decltype(timerDeleter)> moduleToObjectTimer(
+      new llvm::Timer("moduleToObjectTimer",
+                      "Timer for perf llvm-ir -> isa and isa -> binary."),
+      timerDeleter);
+  moduleToObjectTimer->startTimer();
+
   // Emit PTX code.
   std::optional<llvm::TargetMachine *> targetMachine =
       getOrCreateTargetMachine();
@@ -706,9 +714,9 @@ NVPTXSerializer::moduleToObject(llvm::Module &llvmModule) {
     return std::nullopt;
   }
 
-  moduleToObjectTimer.stopTimer();
-  llvmToISATimeInMs = moduleToObjectTimer.getTotalTime().getWallTime() * 1000;
-  moduleToObjectTimer.clear();
+  moduleToObjectTimer->stopTimer();
+  llvmToISATimeInMs = moduleToObjectTimer->getTotalTime().getWallTime() * 1000;
+  moduleToObjectTimer->clear();
   if (isaCallback)
     isaCallback(serializedISA.value());
 
@@ -729,7 +737,7 @@ NVPTXSerializer::moduleToObject(llvm::Module &llvmModule) {
   }
 
   std::optional<SmallVector<char, 0>> result;
-  moduleToObjectTimer.startTimer();
+  moduleToObjectTimer->startTimer();
   // Compile to binary.
 #if MLIR_ENABLE_NVPTXCOMPILER
   result = compileToBinaryNVPTX(*serializedISA);
@@ -737,9 +745,9 @@ NVPTXSerializer::moduleToObject(llvm::Module &llvmModule) {
   result = compileToBinary(*serializedISA);
 #endif // MLIR_ENABLE_NVPTXCOMPILER
 
-  moduleToObjectTimer.stopTimer();
-  isaToBinaryTimeInMs = moduleToObjectTimer.getTotalTime().getWallTime() * 1000;
-  moduleToObjectTimer.clear();
+  moduleToObjectTimer->stopTimer();
+  isaToBinaryTimeInMs =
+      moduleToObjectTimer->getTotalTime().getWallTime() * 1000;
   return result;
 }
 

@llvmbot
Copy link
Member

llvmbot commented Apr 17, 2025

@llvm/pr-subscribers-mlir-llvm

Author: Zichen Lu (MikaOvO)

Changes

As title, we need to call Timer::clear to avoid extra log like this:

===-------------------------------------------------------------------------===
                           ...
===-------------------------------------------------------------------------===
  Total Execution Time: 0.0000 seconds (0.0000 wall clock)

   ---Wall Time---  --- Name ---
        -----       ....
        -----       Total

Full diff: https://github.com/llvm/llvm-project/pull/136142.diff

1 Files Affected:

  • (modified) mlir/lib/Target/LLVM/NVVM/Target.cpp (+19-11)
diff --git a/mlir/lib/Target/LLVM/NVVM/Target.cpp b/mlir/lib/Target/LLVM/NVVM/Target.cpp
index 5a2b829fbd1bd..ea0c2b6663fbb 100644
--- a/mlir/lib/Target/LLVM/NVVM/Target.cpp
+++ b/mlir/lib/Target/LLVM/NVVM/Target.cpp
@@ -669,10 +669,6 @@ NVPTXSerializer::compileToBinaryNVPTX(const std::string &ptxCode) {
 
 std::optional<SmallVector<char, 0>>
 NVPTXSerializer::moduleToObject(llvm::Module &llvmModule) {
-  llvm::Timer moduleToObjectTimer(
-      "moduleToObjectTimer",
-      "Timer for perf llvm-ir -> isa and isa -> binary.");
-  moduleToObjectTimer.startTimer();
   // Return LLVM IR if the compilation target is `offload`.
 #define DEBUG_TYPE "serialize-to-llvm"
   LLVM_DEBUG({
@@ -691,6 +687,18 @@ NVPTXSerializer::moduleToObject(llvm::Module &llvmModule) {
   return std::nullopt;
 #endif // LLVM_HAS_NVPTX_TARGET
 
+  auto timerDeleter = [](llvm::Timer *timer) {
+    if (timer != nullptr) {
+      timer->clear();
+      delete timer;
+    }
+  };
+  std::unique_ptr<llvm::Timer, decltype(timerDeleter)> moduleToObjectTimer(
+      new llvm::Timer("moduleToObjectTimer",
+                      "Timer for perf llvm-ir -> isa and isa -> binary."),
+      timerDeleter);
+  moduleToObjectTimer->startTimer();
+
   // Emit PTX code.
   std::optional<llvm::TargetMachine *> targetMachine =
       getOrCreateTargetMachine();
@@ -706,9 +714,9 @@ NVPTXSerializer::moduleToObject(llvm::Module &llvmModule) {
     return std::nullopt;
   }
 
-  moduleToObjectTimer.stopTimer();
-  llvmToISATimeInMs = moduleToObjectTimer.getTotalTime().getWallTime() * 1000;
-  moduleToObjectTimer.clear();
+  moduleToObjectTimer->stopTimer();
+  llvmToISATimeInMs = moduleToObjectTimer->getTotalTime().getWallTime() * 1000;
+  moduleToObjectTimer->clear();
   if (isaCallback)
     isaCallback(serializedISA.value());
 
@@ -729,7 +737,7 @@ NVPTXSerializer::moduleToObject(llvm::Module &llvmModule) {
   }
 
   std::optional<SmallVector<char, 0>> result;
-  moduleToObjectTimer.startTimer();
+  moduleToObjectTimer->startTimer();
   // Compile to binary.
 #if MLIR_ENABLE_NVPTXCOMPILER
   result = compileToBinaryNVPTX(*serializedISA);
@@ -737,9 +745,9 @@ NVPTXSerializer::moduleToObject(llvm::Module &llvmModule) {
   result = compileToBinary(*serializedISA);
 #endif // MLIR_ENABLE_NVPTXCOMPILER
 
-  moduleToObjectTimer.stopTimer();
-  isaToBinaryTimeInMs = moduleToObjectTimer.getTotalTime().getWallTime() * 1000;
-  moduleToObjectTimer.clear();
+  moduleToObjectTimer->stopTimer();
+  isaToBinaryTimeInMs =
+      moduleToObjectTimer->getTotalTime().getWallTime() * 1000;
   return result;
 }
 

"Timer for perf llvm-ir -> isa and isa -> binary."),
timerDeleter);
moduleToObjectTimer->startTimer();

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we just do the following:

auto clear = llvm::make_scope_exit([&]() { moduleToObjectTimer.clear(); });

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modified. Could you help merge this? Thanks.

@MikaOvO MikaOvO force-pushed the mikaovo/raii_for_module_to_object_timer branch from 71dae28 to cdbba68 Compare April 18, 2025 03:24
@MikaOvO MikaOvO force-pushed the mikaovo/raii_for_module_to_object_timer branch from cdbba68 to ff92237 Compare April 18, 2025 05:14
@joker-eph joker-eph merged commit 1d19006 into llvm:main Apr 18, 2025
11 checks passed
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…function (llvm#136142)

As title, we need to call `Timer::clear` to avoid extra log like this:
```
===-------------------------------------------------------------------------===
                           ...
===-------------------------------------------------------------------------===
  Total Execution Time: 0.0000 seconds (0.0000 wall clock)

   ---Wall Time---  --- Name ---
        -----       ....
        -----       Total
```
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…function (llvm#136142)

As title, we need to call `Timer::clear` to avoid extra log like this:
```
===-------------------------------------------------------------------------===
                           ...
===-------------------------------------------------------------------------===
  Total Execution Time: 0.0000 seconds (0.0000 wall clock)

   ---Wall Time---  --- Name ---
        -----       ....
        -----       Total
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants