Skip to content

Add RemoveSubsequentEquivalentCtrl Canonicalization Pattern#1686

Draft
MatthiasReumann wants to merge 5 commits intomainfrom
feat/remove-subseq-eq-ctrls
Draft

Add RemoveSubsequentEquivalentCtrl Canonicalization Pattern#1686
MatthiasReumann wants to merge 5 commits intomainfrom
feat/remove-subseq-eq-ctrls

Conversation

@MatthiasReumann
Copy link
Copy Markdown
Collaborator

@MatthiasReumann MatthiasReumann commented May 5, 2026

Description

This pull request add a RemoveSubsequentEquivalentCtrl pattern to the canonicalization of the CtrlOp, which is currently not implemented.

This pattern will probably also be used in the "Writing Your First Optimization Pass" section in the Getting Started guide. (#1555)

Checklist

  • The pull request only contains commits that are focused and relevant to this change.
  • I have added appropriate tests that cover the new/changed functionality.
  • I have updated the documentation to reflect these changes.
  • I have added entries to the changelog for any noteworthy additions, changes, fixes, or removals.
  • I have added migration instructions to the upgrade guide (if needed).
  • The changes follow the project's style guidelines and introduce no new warnings.
  • The changes are fully tested and pass the CI checks.
  • I have reviewed my own code changes.

If PR contains AI-assisted content:

  • I have disclosed the use of AI tools in the PR description as per our AI Usage Guidelines.
  • AI-assisted commits include an Assisted-by: [Model Name] via [Tool Name] footer.
  • I confirm that I have personally reviewed and understood all AI-generated content, and accept full responsibility for it.

@MatthiasReumann MatthiasReumann added c++ Anything related to C++ code MLIR Anything related to MLIR labels May 5, 2026
@MatthiasReumann MatthiasReumann self-assigned this May 5, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2026

Codecov Report

❌ Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
mlir/lib/Dialect/QCO/IR/Modifiers/CtrlOp.cpp 96.4% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@burgholzer
Copy link
Copy Markdown
Member

Just a brief comment here: We should really consider lifting the constraint that Ctrl modifiers (and modifiers in general) must only contain a single unitary operation. If we would allow multiple operations in a modifier, then we would get this optimization basically for free based on the current canonicalization and a canonicalization that pulls together separate CtrlOp with shared controls.
Something to think about a little.

@MatthiasReumann
Copy link
Copy Markdown
Collaborator Author

Just a brief comment here: We should really consider lifting the constraint that Ctrl modifiers (and modifiers in general) must only contain a single unitary operation. If we would allow multiple operations in a modifier, then we would get this optimization basically for free based on the current canonicalization and a canonicalization that pulls together separate CtrlOp with shared controls. Something to think about a little.

I haven't thought it through thoroughly, but this sounds very reasonable - at least for the optimization introduced in this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Anything related to C++ code MLIR Anything related to MLIR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants