Skip to content

✨ Enhance QCOProgramBuilder with tracked qubit and tensor structures - #2017

Open
simon1hofmann wants to merge 7 commits into
mainfrom
linear_tracking_helpers
Open

✨ Enhance QCOProgramBuilder with tracked qubit and tensor structures#2017
simon1hofmann wants to merge 7 commits into
mainfrom
linear_tracking_helpers

Conversation

@simon1hofmann

@simon1hofmann simon1hofmann commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖

Description

  • Replace DenseMap<Value, QubitInfo/TensorInfo> with DenseSet<Qubit/Tensor> keyed on the SSA Value, keeping register metadata on the handle itself.
  • Return thin Qubit handles from allocQubit() / staticQubit() (implicitly convertible to Value); add small getType() / getDefiningOp() forwards so common call sites stay ergonomic.
  • Leave gate / modifier / SCF APIs on Value / ValueRange.

Fixes #1628

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).
  • 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.

- Introduced `Qubit` and `Tensor` structs to encapsulate SSA values along with their register information.
- Updated methods `allocQubit` and `staticQubit` to return `Qubit` instances instead of raw `Value`.
- Modified internal tracking of valid qubits and tensors to utilize `DenseSet` for improved management.
- Enhanced unit tests to validate the new structures and their integration within the builder.

* Updated CHANGELOG to reflect changes related to PR #2017.
@simon1hofmann simon1hofmann self-assigned this Aug 7, 2026
@simon1hofmann simon1hofmann added enhancement Improvement of existing feature c++ Anything related to C++ code MLIR Anything related to MLIR labels Aug 7, 2026
@simon1hofmann simon1hofmann added this to the MLIR Support milestone Aug 7, 2026
@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@simon1hofmann

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Improved quantum program building with tracked qubit and tensor metadata, including register identifiers and indices.
    • Enhanced interoperability between tracked qubits and their underlying values.
    • Improved qubit allocation, reuse validation, tensor lifecycle management, and finalization.
  • Tests

    • Added coverage for tracked qubit conversions, metadata, reuse behavior, and operation integration.
    • Updated index-switch and measurement-lifting tests for improved builder APIs.
  • Documentation

    • Updated the unreleased changelog with the corresponding pull request reference.

Walkthrough

QCOProgramBuilder now returns tracked Qubit handles and stores qubit and tensor metadata in dense sets. Builder operations and tests use these wrappers across allocation, replacement, extraction, initialization, and finalization.

Changes

QCO tracking refactor

Layer / File(s) Summary
Tracked wrapper contracts
mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h
Added Qubit and Tensor wrappers. Allocation APIs now return tracked qubits. Dense sets replace metadata maps.
Builder tracking and finalization
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp
Updated allocation, replacement, tensor creation, extraction, initialization, sinking, and finalization to preserve wrapper metadata.
API validation and release metadata
mlir/unittests/Dialect/QCO/IR/test_qco_ir.cpp, mlir/unittests/Dialect/QCO/Transforms/Optimizations/test_qco_measurement_lifting.cpp, CHANGELOG.md
Updated tests for tracked-handle conversions and single-value APIs. Added the PR reference to the changelog.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

Suggested labels: refactor

Suggested reviewers: burgholzer, denialhaag

Poem

I’m a rabbit with qubits tucked tight,
Dense sets keep register IDs right.
Handles hop through each builder call,
Tensors remember their place through it all.
Tests confirm reuse stays controlled.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding tracked qubit and tensor structures to QCOProgramBuilder.
Description check ✅ Passed The description includes the change summary, issue reference, checklist, tests, changelog, and AI disclosure details; only minor checklist items remain incomplete.
Linked Issues check ✅ Passed The changes satisfy issue #1628 by introducing Qubit and Tensor handles, replacing metadata maps with DenseSet tracking, and avoiding QubitRegister tracking.
Out of Scope Changes check ✅ Passed The implementation, tests, and changelog updates directly support issue #1628 and the stated QCOProgramBuilder API and tracking changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch linear_tracking_helpers

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: 1

🤖 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/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h`:
- Around line 173-206: Add Doxygen documentation to the public Qubit and Tensor
constructors, implicit Value conversion operators, getType(), and both
getDefiningOp() overloads. Describe the conversion direction and state that
getDefiningOp() may return a null operation when the value has no defining
operation; document both the non-templated and templated accessors consistently.
🪄 Autofix

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 Plus

Run ID: 5bd9e4b3-942b-45fa-9da5-145c0f5b89bb

📥 Commits

Reviewing files that changed from the base of the PR and between 52cf1e7 and 8506415.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h
  • mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp
  • mlir/unittests/Dialect/QCO/IR/test_qco_ir.cpp
  • mlir/unittests/Dialect/QCO/Transforms/Optimizations/test_qco_measurement_lifting.cpp

Comment thread mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h Outdated
@simon1hofmann

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@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: 1

🤖 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/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h`:
- Around line 179-180: Make the Qubit(Value, int64_t, Value) constructor
explicit to prevent implicit Value-to-Qubit conversions from discarding regId
and regIndex. Preserve explicit assignment-from-Value behavior only in
intentional metadata-reset paths such as updateQubitTracking.
🪄 Autofix

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 Plus

Run ID: 43432451-ea74-41ed-b509-2ea0de49393d

📥 Commits

Reviewing files that changed from the base of the PR and between 8506415 and 75970c6.

📒 Files selected for processing (1)
  • mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h

Comment thread mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h Outdated
@simon1hofmann

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp (1)

242-250: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🤖 AI text below 🤖

Validate the tensor before dereferencing find.

checkQubitType checks the tensor type, but it does not verify tracking membership. If an untracked tensor reaches prepareInitArg, validTensors.find(initArg) returns end(), and ->regId dereferences an invalid iterator. Call validateTensorValue(initArg) before the lookup.

Suggested fix
   if (isa<QubitType>(initArg.getType())) {
     return initArg;
   }

+  validateTensorValue(initArg);
   const auto regId = validTensors.find(initArg)->regId;
🤖 Prompt for 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.

In `@mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp` around lines 242 - 250,
In prepareInitArg, validate initArg with validateTensorValue before
dereferencing the validTensors.find(initArg) result. Preserve the existing
register-ID lookup and initialization flow after validation.
🤖 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.

Outside diff comments:
In `@mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp`:
- Around line 242-250: In prepareInitArg, validate initArg with
validateTensorValue before dereferencing the validTensors.find(initArg) result.
Preserve the existing register-ID lookup and initialization flow after
validation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0d14e32f-eae8-465c-8518-24546e21887d

📥 Commits

Reviewing files that changed from the base of the PR and between 75970c6 and 24c21b0.

📒 Files selected for processing (3)
  • mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h
  • mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp
  • mlir/unittests/Dialect/QCO/IR/test_qco_ir.cpp

@simon1hofmann
simon1hofmann marked this pull request as ready for review August 7, 2026 12:32

@denialhaag denialhaag 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.

Apart from the two small comments below, this LGTM! Thanks for the improvements to the QCOProgramBuilder, @simon1hofmann! 🙂

Comment thread mlir/unittests/Dialect/QCO/IR/test_qco_ir.cpp Outdated
Comment thread mlir/unittests/Dialect/QCO/IR/test_qco_ir.cpp Outdated
@denialhaag
denialhaag requested a review from burgholzer August 7, 2026 14:44
@denialhaag

Copy link
Copy Markdown
Member

Just as a reminder for ourselves: Depending on which of #2014 and #2017 gets merged second, we can mark it as fixing #1614. 🎉

@denialhaag denialhaag mentioned this pull request Aug 7, 2026
12 tasks
simon1hofmann and others added 3 commits August 7, 2026 21:29
Co-authored-by: Daniel Haag <121057143+denialhaag@users.noreply.github.com>
Signed-off-by: simon1hofmann <119581649+simon1hofmann@users.noreply.github.com>
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 enhancement Improvement of existing feature MLIR Anything related to MLIR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🎨 Improve linear-type-tracking helpers in QCOProgramBuilder

2 participants