Skip to content

Conversation

@VadimCurca
Copy link
Contributor

When a CompositePass is created programmatically, it incorrectly prepends an extra any to the inner pipeline string. For example:

passManager.nestAny().addPass(createCompositeFixedPointPass(
  "Pass1AndPass2",
  [](OpPassManager &nestedPassManger) {
    nestedPassManger.addPass(createPass1());
    nestedPassManger.addPass(createPass2());
  },

This would result in the following pipeline string:

any(composite-fixed-point-pass{max-iterations=3 name=Pass1AndPass2
pipeline=any(pass1,pass2)})

This commit fixes this issue, resulting in the pipeline string:

any(composite-fixed-point-pass{max-iterations=3 name=Pass1AndPass2
pipeline=pass1,pass2})

When a `CompositePass` is created programmatically, it incorrectly
prepends an extra `any` to the inner pipeline string. For example:
```c++
passManager.nestAny().addPass(createCompositeFixedPointPass(
  "Pass1AndPass2",
  [](OpPassManager &nestedPassManger) {
    nestedPassManger.addPass(createPass1());
    nestedPassManger.addPass(createPass2());
  },
```
This would result in the following pipeline string:
```
any(composite-fixed-point-pass{max-iterations=3 name=Pass1AndPass2
pipeline=any(pass1,pass2)})
```

This commit fixes this issue, resulting in the pipeline string:
```
any(composite-fixed-point-pass{max-iterations=3 name=Pass1AndPass2
pipeline=pass1,pass2})
```
@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels May 14, 2025
@llvmbot
Copy link
Member

llvmbot commented May 14, 2025

@llvm/pr-subscribers-mlir-core

Author: Vadim Curcă (VadimCurca)

Changes

When a CompositePass is created programmatically, it incorrectly prepends an extra any to the inner pipeline string. For example:

passManager.nestAny().addPass(createCompositeFixedPointPass(
  "Pass1AndPass2",
  [](OpPassManager &nestedPassManger) {
    nestedPassManger.addPass(createPass1());
    nestedPassManger.addPass(createPass2());
  },

This would result in the following pipeline string:

any(composite-fixed-point-pass{max-iterations=3 name=Pass1AndPass2
pipeline=any(pass1,pass2)})

This commit fixes this issue, resulting in the pipeline string:

any(composite-fixed-point-pass{max-iterations=3 name=Pass1AndPass2
pipeline=pass1,pass2})

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

2 Files Affected:

  • (modified) mlir/lib/Transforms/CompositePass.cpp (+3-1)
  • (modified) mlir/test/Transforms/composite-pass.mlir (+5-1)
diff --git a/mlir/lib/Transforms/CompositePass.cpp b/mlir/lib/Transforms/CompositePass.cpp
index b388a28da6424..16e276e3f41b7 100644
--- a/mlir/lib/Transforms/CompositePass.cpp
+++ b/mlir/lib/Transforms/CompositePass.cpp
@@ -35,7 +35,9 @@ struct CompositeFixedPointPass final
     populateFunc(dynamicPM);
 
     llvm::raw_string_ostream os(pipelineStr);
-    dynamicPM.printAsTextualPipeline(os);
+    llvm::interleave(
+        dynamicPM, [&](mlir::Pass &pass) { pass.printAsTextualPipeline(os); },
+        [&]() { os << ","; });
   }
 
   LogicalResult initializeOptions(
diff --git a/mlir/test/Transforms/composite-pass.mlir b/mlir/test/Transforms/composite-pass.mlir
index 829470c2c9aa6..75587edd5b96d 100644
--- a/mlir/test/Transforms/composite-pass.mlir
+++ b/mlir/test/Transforms/composite-pass.mlir
@@ -1,6 +1,10 @@
-// RUN: mlir-opt %s --log-actions-to=- --test-composite-fixed-point-pass -split-input-file | FileCheck %s
+// RUN: mlir-opt %s --log-actions-to=- --test-composite-fixed-point-pass -split-input-file --dump-pass-pipeline 2>&1 | FileCheck %s --check-prefixes=CHECK,PIPELINE
 // RUN: mlir-opt %s --log-actions-to=- --composite-fixed-point-pass='name=TestCompositePass pipeline=any(canonicalize,cse)' -split-input-file | FileCheck %s
 
+// Ensure the composite pass correctly prints its options.
+// PIPELINE:      builtin.module(composite-fixed-point-pass{max-iterations=10 name=TestCompositePass
+// PIPELINE-SAME: pipeline=canonicalize{ max-iterations=10 max-num-rewrites=-1 region-simplify=normal test-convergence=false top-down=true},cse})
+
 // CHECK-LABEL: running `TestCompositePass`
 //       CHECK: running `Canonicalizer`
 //       CHECK: running `CSE`

@llvmbot
Copy link
Member

llvmbot commented May 14, 2025

@llvm/pr-subscribers-mlir

Author: Vadim Curcă (VadimCurca)

Changes

When a CompositePass is created programmatically, it incorrectly prepends an extra any to the inner pipeline string. For example:

passManager.nestAny().addPass(createCompositeFixedPointPass(
  "Pass1AndPass2",
  [](OpPassManager &amp;nestedPassManger) {
    nestedPassManger.addPass(createPass1());
    nestedPassManger.addPass(createPass2());
  },

This would result in the following pipeline string:

any(composite-fixed-point-pass{max-iterations=3 name=Pass1AndPass2
pipeline=any(pass1,pass2)})

This commit fixes this issue, resulting in the pipeline string:

any(composite-fixed-point-pass{max-iterations=3 name=Pass1AndPass2
pipeline=pass1,pass2})

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

2 Files Affected:

  • (modified) mlir/lib/Transforms/CompositePass.cpp (+3-1)
  • (modified) mlir/test/Transforms/composite-pass.mlir (+5-1)
diff --git a/mlir/lib/Transforms/CompositePass.cpp b/mlir/lib/Transforms/CompositePass.cpp
index b388a28da6424..16e276e3f41b7 100644
--- a/mlir/lib/Transforms/CompositePass.cpp
+++ b/mlir/lib/Transforms/CompositePass.cpp
@@ -35,7 +35,9 @@ struct CompositeFixedPointPass final
     populateFunc(dynamicPM);
 
     llvm::raw_string_ostream os(pipelineStr);
-    dynamicPM.printAsTextualPipeline(os);
+    llvm::interleave(
+        dynamicPM, [&](mlir::Pass &pass) { pass.printAsTextualPipeline(os); },
+        [&]() { os << ","; });
   }
 
   LogicalResult initializeOptions(
diff --git a/mlir/test/Transforms/composite-pass.mlir b/mlir/test/Transforms/composite-pass.mlir
index 829470c2c9aa6..75587edd5b96d 100644
--- a/mlir/test/Transforms/composite-pass.mlir
+++ b/mlir/test/Transforms/composite-pass.mlir
@@ -1,6 +1,10 @@
-// RUN: mlir-opt %s --log-actions-to=- --test-composite-fixed-point-pass -split-input-file | FileCheck %s
+// RUN: mlir-opt %s --log-actions-to=- --test-composite-fixed-point-pass -split-input-file --dump-pass-pipeline 2>&1 | FileCheck %s --check-prefixes=CHECK,PIPELINE
 // RUN: mlir-opt %s --log-actions-to=- --composite-fixed-point-pass='name=TestCompositePass pipeline=any(canonicalize,cse)' -split-input-file | FileCheck %s
 
+// Ensure the composite pass correctly prints its options.
+// PIPELINE:      builtin.module(composite-fixed-point-pass{max-iterations=10 name=TestCompositePass
+// PIPELINE-SAME: pipeline=canonicalize{ max-iterations=10 max-num-rewrites=-1 region-simplify=normal test-convergence=false top-down=true},cse})
+
 // CHECK-LABEL: running `TestCompositePass`
 //       CHECK: running `Canonicalizer`
 //       CHECK: running `CSE`

@Dinistro Dinistro requested a review from Hardcode84 May 14, 2025 11:17
@gysit gysit merged commit cdabce0 into llvm:main May 14, 2025
14 checks passed
@VadimCurca VadimCurca deleted the vadimc/fix_composite_pass_pipeline_str branch May 14, 2025 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mlir:core MLIR Core Infrastructure mlir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants