[mlir][reducer] make opt-reduction pass clone topOp after check (NFC)

[linuxlonelyeagle]

To avoid potential memory leaks, this PR defers the ModuleOp cloning until after the verification check. If the check fails, the moduleVariant might not be properly deallocated(original implementation), leading to a memory leak. Therefore, this PR ensures that the clone operation is only performed after a successful check. It is part of #189353.

[llvmbot]

@llvm/pr-subscribers-mlir

Author: lonely eagle (linuxlonelyeagle)

Changes

To avoid potential memory leaks, this PR defers the ModuleOp cloning until after the verification check. If the check fails, the moduleVariant might not be properly deallocated(original implementation), leading to a memory leak. Therefore, this PR ensures that the clone operation is only performed after a successful check

---

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

1 Files Affected:

• (modified) mlir/lib/Reducer/OptReductionPass.cpp (+1-3)

diff --git a/mlir/lib/Reducer/OptReductionPass.cpp b/mlir/lib/Reducer/OptReductionPass.cpp
index 4bdc7b17567d9..feb28e1b9beb8 100644
--- a/mlir/lib/Reducer/OptReductionPass.cpp
+++ b/mlir/lib/Reducer/OptReductionPass.cpp
@@ -47,8 +47,6 @@ void OptReductionPass::runOnOperation() {
   Tester test(testerName, testerArgs);
 
   ModuleOp module = this->getOperation();
-  ModuleOp moduleVariant = module.clone();
-
   OpPassManager passManager("builtin.module");
   if (failed(parsePassPipeline(optPass, passManager))) {
     module.emitError() << "\nfailed to parse pass pipeline";
@@ -60,7 +58,7 @@ void OptReductionPass::runOnOperation() {
     module.emitError() << "\nthe original input is not interested";
     return signalPassFailure();
   }
-
+  ModuleOp moduleVariant = module.clone();
   // Temporarily push the variant under the main module and execute the pipeline
   // on it.
   module.getBody()->push_back(moduleVariant);

[llvmbot]

@llvm/pr-subscribers-mlir-core

Author: lonely eagle (linuxlonelyeagle)

Changes

To avoid potential memory leaks, this PR defers the ModuleOp cloning until after the verification check. If the check fails, the moduleVariant might not be properly deallocated(original implementation), leading to a memory leak. Therefore, this PR ensures that the clone operation is only performed after a successful check

---

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

1 Files Affected:

• (modified) mlir/lib/Reducer/OptReductionPass.cpp (+1-3)

diff --git a/mlir/lib/Reducer/OptReductionPass.cpp b/mlir/lib/Reducer/OptReductionPass.cpp
index 4bdc7b17567d9..feb28e1b9beb8 100644
--- a/mlir/lib/Reducer/OptReductionPass.cpp
+++ b/mlir/lib/Reducer/OptReductionPass.cpp
@@ -47,8 +47,6 @@ void OptReductionPass::runOnOperation() {
   Tester test(testerName, testerArgs);
 
   ModuleOp module = this->getOperation();
-  ModuleOp moduleVariant = module.clone();
-
   OpPassManager passManager("builtin.module");
   if (failed(parsePassPipeline(optPass, passManager))) {
     module.emitError() << "\nfailed to parse pass pipeline";
@@ -60,7 +58,7 @@ void OptReductionPass::runOnOperation() {
     module.emitError() << "\nthe original input is not interested";
     return signalPassFailure();
   }
-
+  ModuleOp moduleVariant = module.clone();
   // Temporarily push the variant under the main module and execute the pipeline
   // on it.
   module.getBody()->push_back(moduleVariant);

[hockyy]

hmm, good catch, something related to this, our internal MLIR projects did an ASAN build for all of our testcases and found bunch of these kinds of memory leak.

Func cloning and module cloning leaks are the most common. If you have time probably can try doing an ASAN debug and do some regression test.

[hockyy]

lgtm

[llvm-ci]

LLVM Buildbot has detected a new failure on builder ppc64le-flang-rhel-clang running on ppc64le-flang-rhel-test while building mlir at step 6 "test-build-unified-tree-check-flang".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/45695

Here is the relevant piece of the build log for the reference

Step 6 (test-build-unified-tree-check-flang) failure: 1200 seconds without output running [b'ninja', b'check-flang'], attempting to kill
...
PASS: Flang :: Driver/use-module-error.f90 (4328 of 4338)
PASS: Flang :: Driver/slp-vectorize.f90 (4329 of 4338)
PASS: Flang :: Lower/OpenMP/requires-usm.f90 (4330 of 4338)
PASS: Flang :: Driver/mcmodel.f90 (4331 of 4338)
PASS: Flang :: Driver/linker-options.f90 (4332 of 4338)
PASS: Flang :: Driver/omp-driver-offload.f90 (4333 of 4338)
PASS: Flang :: Lower/OpenMP/rtl-flags.f90 (4334 of 4338)
PASS: Flang :: Driver/target-cpu-features.f90 (4335 of 4338)
PASS: Flang :: Driver/options-loongarch.f90 (4336 of 4338)
PASS: Flang :: Driver/fopenmp.f90 (4337 of 4338)
command timed out: 1200 seconds without output running [b'ninja', b'check-flang'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=3083.344807