Skip to content

🐛 Reject nonlinear Hadamard-lifting inputs safely - #2309

Closed
simon1hofmann wants to merge 1 commit into
mainfrom
codex/2255-hadamard-lifting-linearity
Closed

🐛 Reject nonlinear Hadamard-lifting inputs safely#2309
simon1hofmann wants to merge 1 commit into
mainfrom
codex/2255-hadamard-lifting-linearity

Conversation

@simon1hofmann

@simon1hofmann simon1hofmann commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖

Summary

  • Validate QCO linearity before greedy Hadamard lifting mutates the IR.
  • Reject unused Pauli and CNOT outputs through pass failure.
  • Verify that failure leaves the input unchanged.

Part of #2255.

Validation

  • Hadamard-lifting focused tests: 12 passed.
  • Repository pre-commit hooks: passed.
  • Clang-Tidy 22.1.8 on changed C++ sources: passed.

AI assistance: Codex extracted this focused change from the contract audit branch, rebased it onto current main, and ran the listed validation.

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. (Not applicable: no user-facing documentation change is needed.)
  • I have added entries to the changelog for any noteworthy additions, changes, fixes, or removals. (Not applicable: this focused fix is labeled skip-changelog.)
  • I have added migration instructions to the upgrade guide (if needed). (Not 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).
  • I have disclosed AI assistance in the PR description.
  • I confirm that I have personally reviewed and understood all AI-generated content, and accept full responsibility for it.

Validate quantum SSA linearity before applying greedy lifting patterns, leaving invalid input untouched.

Assisted-by: GPT-5.6 via Codex
@simon1hofmann simon1hofmann added c++ Anything related to C++ code code quality Code quality improvements fix Fix for something that isn't working MLIR Anything related to MLIR skip-changelog Changes that do not need to show up in the changelog labels Sep 1, 2026
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@burgholzer burgholzer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I also have my doubts with this one.

Feels to me like it would be reasonable to adjust the audit prompt. It feels like it is generating a lot of changes that we actually do not want.

Comment on lines +232 to +235
if (failed(qco::verifyLinearity(op))) {
signalPassFailure();
return;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm. I am not sure we should actually be adding this in front of every pass.
Passes can expect valid IR. They should not have to verify that it is valid.
I'd rather drop that in cases like here.
Does that make sense?

Comment on lines +87 to +96
TEST_F(QCOHadamardLiftingTest, HandlesUnusedPauliOutput) {
auto input = parseSourceString<ModuleOp>(R"mlir(
module {
func.func @main() {
%q = qco.static 0 : !qco.qubit
%unused = qco.x %q : !qco.qubit -> !qco.qubit
return
}
}
)mlir",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is invalid QCO IR.
We should really enforce the invariant that passes may assume IR to be valid.
We may check in tests that this is actually true before and after passes, but the corner case checking in all of our passes can be reduced.
Even more so can the amount of tests that check these invalid IR programs.
This extends well beyond this PR.

@simon1hofmann

Copy link
Copy Markdown
Contributor Author

🤖 AI text below 🤖

Agreed. These tests use invalid QCO IR, and Hadamard lifting may assume verified linearity. The pass-local validation and invalid-input regressions are outside the revised audit, so I am closing this PR unmerged. The broader audit criteria and disposition are recorded in #2287.

simon1hofmann added a commit that referenced this pull request Sep 1, 2026
Move the invalid-QCO finding from pending withdrawal to closed and refresh the focused audit counts.

Assisted-by: GPT-5.6 via Codex
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 code quality Code quality improvements fix Fix for something that isn't working MLIR Anything related to MLIR skip-changelog Changes that do not need to show up in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants