Skip to content

✨ Map scf::WhileOps using the restore strategy#1911

Open
MatthiasReumann wants to merge 13 commits into
mainfrom
feat/scf-while-ops-mapping
Open

✨ Map scf::WhileOps using the restore strategy#1911
MatthiasReumann wants to merge 13 commits into
mainfrom
feat/scf-while-ops-mapping

Conversation

@MatthiasReumann

@MatthiasReumann MatthiasReumann commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Description

This pull request adds mapping support for the scf::WhileOp.

  • ✨ Add scf::WhileOp to supported SCF ops in Mapping.cpp
  • 🐛 Fix WireIterator for scf::WhileOp
  • ♻️ Simplify isExecutable function in test_mapping.cpp
  • ♻️ Simplify dispatch function in Mapping.cpp

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:

  • Any agent that created, edited, or submitted GitHub content was explicitly authorized for that scope, as required by our AI Usage Guidelines.
  • Every agent-authored or agent-edited public text body begins with the visible disclosure 🤖 *AI text below* 🤖 (titles are exempt).
  • 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 this to the MLIR Support milestone Jul 15, 2026
@MatthiasReumann MatthiasReumann self-assigned this Jul 15, 2026
@MatthiasReumann MatthiasReumann added c++ Anything related to C++ code MLIR Anything related to MLIR labels Jul 15, 2026
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.37838% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...lir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp 98.8% 2 Missing ⚠️
mlir/lib/Dialect/QCO/Utils/WireIterator.cpp 92.3% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@MatthiasReumann
MatthiasReumann marked this pull request as ready for review July 17, 2026 06:50
@MatthiasReumann

Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

An error occurred during the review process. Please try again later.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch feat/scf-while-ops-mapping

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp`:
- Around line 107-127: Update the initialization in the mapping test around
initArgs, initialHardwareOrder, and localM so SCF hardware indices are derived
from qubit-only init operands, matching the filtered qubitArgs order. Filter
scf::WhileOp getInits() and scf::ForOp getInits() by QubitType before mapping
through m, while preserving qco::IfOp behavior and ensuring mixed-type loops do
not access missing DenseMap entries.
- Line 738: Add routing-relevant gates to the while after-region test around the
identity callback in the mapping test, so recursive after-region routing and
terminator realignment are exercised. Extend the test inputs and assertions to
verify the mapped values and control flow remain correctly aligned through the
after-region rather than merely returning args unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ce75cc65-da3f-4bff-a384-61ca652f1b74

📥 Commits

Reviewing files that changed from the base of the PR and between 559fde4 and 0c05d8e.

📒 Files selected for processing (4)
  • mlir/include/mlir/Dialect/QCO/Utils/Drivers.h
  • mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp
  • mlir/lib/Dialect/QCO/Utils/WireIterator.cpp
  • mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp

Comment thread mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp
Comment thread mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp Outdated
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.

1 participant