Skip to content

Conversation

@WillFroom
Copy link
Contributor

Prior to this change pipelineInitializationKey would never be updated so initialize would always be called even if the pipeline didn't change

@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Jul 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 28, 2025

@llvm/pr-subscribers-mlir-core

Author: Will Froom (WillFroom)

Changes

Prior to this change pipelineInitializationKey would never be updated so initialize would always be called even if the pipeline didn't change


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

1 Files Affected:

  • (modified) mlir/lib/Pass/Pass.cpp (+1-1)
diff --git a/mlir/lib/Pass/Pass.cpp b/mlir/lib/Pass/Pass.cpp
index 0db9808b722a7..7094c8e279f2d 100644
--- a/mlir/lib/Pass/Pass.cpp
+++ b/mlir/lib/Pass/Pass.cpp
@@ -901,7 +901,7 @@ LogicalResult PassManager::run(Operation *op) {
     if (failed(initialize(context, impl->initializationGeneration + 1)))
       return failure();
     initializationKey = newInitKey;
-    pipelineKey = pipelineInitializationKey;
+    pipelineInitializationKey = pipelineKey;
   }
 
   // Construct a top level analysis manager for the pipeline.

@llvmbot
Copy link
Member

llvmbot commented Jul 28, 2025

@llvm/pr-subscribers-mlir

Author: Will Froom (WillFroom)

Changes

Prior to this change pipelineInitializationKey would never be updated so initialize would always be called even if the pipeline didn't change


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

1 Files Affected:

  • (modified) mlir/lib/Pass/Pass.cpp (+1-1)
diff --git a/mlir/lib/Pass/Pass.cpp b/mlir/lib/Pass/Pass.cpp
index 0db9808b722a7..7094c8e279f2d 100644
--- a/mlir/lib/Pass/Pass.cpp
+++ b/mlir/lib/Pass/Pass.cpp
@@ -901,7 +901,7 @@ LogicalResult PassManager::run(Operation *op) {
     if (failed(initialize(context, impl->initializationGeneration + 1)))
       return failure();
     initializationKey = newInitKey;
-    pipelineKey = pipelineInitializationKey;
+    pipelineInitializationKey = pipelineKey;
   }
 
   // Construct a top level analysis manager for the pipeline.

@WillFroom WillFroom force-pushed the fix-PassManager-pipelineInitializationKey branch from b7fa80f to 4a61668 Compare July 28, 2025 13:31
@jpienaar jpienaar requested a review from ftynse July 28, 2025 14:15
@WillFroom WillFroom merged commit 4b1d5b8 into llvm:main Jul 28, 2025
9 checks passed
@joker-eph
Copy link
Collaborator

Thanks for the fix!
Can you provide a test for this?

@WillFroom
Copy link
Contributor Author

Can you provide a test for this?

I can, could you point me towards any existing test that I could add to?

@jackalcooper
Copy link
Contributor

Can you provide a test for this?

I can, could you point me towards any existing test that I could add to?

in mlir/test/CAPI/pass.c there is this test

  // Run a pass with `initialize` set

It can be adapted to running the pm multiple times and check the initializeCallCount is still 1.
In my down-stream project I had a similar test to check the initializeCallCount=2 . After your fix got merged the test broke because now the count is 1(correctly).
I was quite confused why the count was not 1 when I added the test, so thanks for the fix!

@WillFroom
Copy link
Contributor Author

Thanks for the pointer @jackalcooper!

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