🐛 Verify modifier bodies at the dialect boundary

[simon1hofmann]

🤖 AI text below 🤖

Description

Reject invalid operations at the QC and QCO modifier boundary instead of adding pass-local guardrails to measurement lifting.

Modifier bodies may contain operations implementing UnitaryOpInterface and pure, speculatable, qubit-free classical support computations. Effectful or unknown operations, register access, and non-unitary operations carrying qubits are rejected by the dialect verifiers. Measurement lifting can consequently assume valid IR and no longer needs its redundant memory-effect checks.

The QIR conversion also rejects controlled global phases before mutating or inlining the control region. This preserves the existing diagnostic for valid multi-block classical computations that produce a non-hoistable phase angle.

Part of #2255.

Validation

• QC IR: 339/339 tests passed
• QCO IR: 494/494 tests passed
• QC-to-QIR adaptive: 149/149 tests passed
• QCO optimizations: 194/194 tests passed
• Total affected suites: 1,176/1,176 tests passed
• nox -s lint
• Changed-file C++ lint: zero clang-format and clang-tidy findings

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.
• The QC and QCO modifier documentation describes the verifier contract.
• No changelog entry is needed for this unreleased MLIR behavior; the skip-changelog label is applied.
• No upgrade-guide migration instructions are needed.
• The changes follow the project's style guidelines and introduce no new warnings.
• The changes are fully tested and pass the CI checks. Local targeted, full-suite, and lint checks pass; CI is pending.
• 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).
• I have disclosed AI assistance in the PR description. Implementation, tests, and PR preparation were assisted by Codex.
• I confirm that I have personally reviewed and understood all AI-generated content, and accept full responsibility for it.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[DRovara]

That's an edge case I didn't think about, but I also have some concerns about it.

In particular, I'm not very convinced by the example test case. I assume the @observe function should represent a function that contains a measurement. But it's not really reasonable to invert a measurement.

In fact, I would say the contents of an qco.inv modifier should always be unitary. That means the only way to have actual memory effects in there is to have a gphase operation, because that adds a global phase with no targeted qubits (but actually, in that case, I don't even know if the inv modifier allows 0 target qubits.

Similarly, qco.ctrl also only makes sense for unitaries.

What are your thoughts on that?

[simon1hofmann]

That's an edge case I didn't think about, but I also have some concerns about it.

In particular, I'm not very convinced by the example test case. I assume the @observe function should represent a function that contains a measurement. But it's not really reasonable to invert a measurement.

In fact, I would say the contents of an qco.inv modifier should always be unitary. That means the only way to have actual memory effects in there is to have a gphase operation, because that adds a global phase with no targeted qubits (but actually, in that case, I don't even know if the inv modifier allows 0 target qubits.

Similarly, qco.ctrl also only makes sense for unitaries.

What are your thoughts on that?

Thanks a lot for the review, you're right, the func.call example is invalid modifier content. I will retarget this PR to reject unknown/effectful non-unitary operations in both modifier verifiers while permitting pure classical support and UnitaryOpInterface operations.

The audit produced a lot of edge cases that are quite hypothetical.

[DRovara]

Hi @simon1hofmann, thanks for the update! Looks quite reasonable, I think it's good to make these verifications in advance.

I do have some minor concerns on the correctness/completeness of the implementation though, but I'm not 100% sure on any of them.

[coderabbitai]

Important

Approval pending

CodeRabbit has no unresolved comments, but it has not reviewed the latest commit.

Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.

• 🔍 Trigger review

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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