Skip to content

🐛 Fix equivalence checking of LLVM::CallOps#1475

Merged
denialhaag merged 2 commits intomainfrom
fix-equivalence-checking
Jan 21, 2026
Merged

🐛 Fix equivalence checking of LLVM::CallOps#1475
denialhaag merged 2 commits intomainfrom
fix-equivalence-checking

Conversation

@denialhaag
Copy link
Member

@denialhaag denialhaag commented Jan 21, 2026

Description

This PR fixes the equivalence checking of LLVM::CallOps.

Without this change, the following test passes:

TEST_F(CompilerPipelineTest, CX) {
  ::qc::QuantumComputation comp;
  comp.addQubitRegister(2, "q");
  comp.cx(0, 1);

  const auto module = importQuantumCircuit(comp);
  ASSERT_TRUE(module);
  ASSERT_TRUE(runPipeline(module.get()).succeeded());

  const auto qc = buildQCIR([](mlir::qc::QCProgramBuilder& b) {
    auto reg = b.allocQubitRegister(2, "q");
    b.cx(reg[0], reg[1]);
  });
  const auto qco = buildQCOIR([](qco::QCOProgramBuilder& b) {
    auto reg = b.allocQubitRegister(2, "q");
    b.cx(reg[0], reg[1]);
  });
  const auto qir = buildQIR([](qir::QIRProgramBuilder& b) {
    auto reg = b.allocQubitRegister(2);
    b.cy(reg[0], reg[1]); // ERROR: Must be cx instead of cy
  });

  verifyAllStages({
      .qcImport = qc.get(),
      .qcoConversion = qco.get(),
      .optimization = qco.get(),
      .qcConversion = qc.get(),
      .qirConversion = qir.get(),
  });
}

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.

@denialhaag denialhaag self-assigned this Jan 21, 2026
@denialhaag denialhaag added fix Fix for something that isn't working MLIR Anything related to MLIR labels Jan 21, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 21, 2026

📝 Walkthrough

Summary by CodeRabbit

  • Tests

    • Improved equivalence checks in compiler validation to more accurately detect differences in call operations, enhancing test reliability.
  • Documentation

    • Updated changelog to reference additional related PRs for the new dialect infrastructure.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Adds a structural equivalence check for LLVM::CallOp in the operation comparison: when the left-hand operation is a CallOp, the right-hand must also be a CallOp with an identical callee. Also updates CHANGELOG.md PR link entries.

Changes

Cohort / File(s) Summary
LLVM CallOp Equivalence Check
mlir/unittests/Compiler/test_compiler_pipeline.cpp
Added logic to require matching callee identity when comparing LLVM::CallOp operations; complements existing constant/operand/result checks.
Changelog updates
CHANGELOG.md
Extended "Added" entry to reference additional PRs and added corresponding PR links in the changelog.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • burgholzer

Poem

🐰 I hopped through code to make a call,
Left and right must mirror all,
Callees matched in neat array,
Tests now check them true as day,
Hooray, the pipeline bounces well!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description includes a summary of the fix and demonstrates the problem with a concrete test case, but the checklist shows incomplete documentation and test additions. Clarify why test additions and documentation updates were not needed, or complete them if they should have been included per the template requirements.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: fixing equivalence checking for LLVM::CallOp operations.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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 and usage tips.

@denialhaag denialhaag requested a review from burgholzer January 21, 2026 17:56
@codecov
Copy link

codecov bot commented Jan 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@denialhaag denialhaag merged commit 0b1fc7c into main Jan 21, 2026
30 checks passed
@denialhaag denialhaag deleted the fix-equivalence-checking branch January 21, 2026 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Fix for something that isn't working MLIR Anything related to MLIR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants