Skip to content

⚡ Avoid redundant target-compilation liveness analysis - #2180

Merged
simon1hofmann merged 1 commit into
mainfrom
codex/target-compilation-liveness-scaling
Sep 2, 2026
Merged

⚡ Avoid redundant target-compilation liveness analysis#2180
simon1hofmann merged 1 commit into
mainfrom
codex/target-compilation-liveness-scaling

Conversation

@simon1hofmann

@simon1hofmann simon1hofmann commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖

Summary

Remove the final module-wide remove-dead-values pass from target compilation. The second QCO cleanup immediately before native synthesis already establishes global liveness, while native synthesis replaces one- and two-qubit unitary operations with native equivalents and reconnects their results.

Post-synthesis CSE and final target-conformance verification remain unchanged.

Motivation

The production-shaped profile in #2134 measured the final RemoveDeadValues pass at 7.77 seconds of a 29.93-second target-compilation run (about 26%), including 4.09 seconds in liveness analysis, for hwb10 targeting FakeTorino.

This PR removes that measured cleanup stage without changing placement, routing, native-synthesis choices, or target-conformance checks. It addresses the cleanup portion of #2134.

Safety and testing

The added end-to-end regression compiles a program containing a private helper call, dynamic qco.if, and one- and two-qubit synthesis. It then reruns RemoveDeadValues and requires the IR to remain byte-for-byte unchanged, protecting the intended fixed point.

  • mqt-core-mlir-unittests-compiler: 140/140 passed.
  • uvx nox -s lint: passed.
  • uvx nox -s cpp-lint with LLVM/Clang 22: 0 findings.
  • git diff --check: passed.

Stack

AI assistance: Codex assisted with diagnosis, implementation, review, testing, stack maintenance, and this description.

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.

@simon1hofmann simon1hofmann added this to the MLIR Support milestone Aug 20, 2026
@simon1hofmann simon1hofmann added enhancement Improvement of existing feature c++ Anything related to C++ code MLIR Anything related to MLIR labels Aug 20, 2026
@simon1hofmann simon1hofmann self-assigned this Aug 20, 2026
@codecov

codecov Bot commented Aug 20, 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 force-pushed the codex/target-compilation-liveness-scaling branch from d74e283 to 08515ac Compare August 24, 2026 13:59
@burgholzer burgholzer self-assigned this Aug 24, 2026
@mergify mergify Bot added the conflict label Aug 28, 2026
@simon1hofmann
simon1hofmann force-pushed the codex/target-compilation-liveness-scaling branch from 08515ac to 6d15343 Compare September 1, 2026 16:03
@mergify mergify Bot removed the conflict label Sep 1, 2026
@simon1hofmann
simon1hofmann force-pushed the codex/target-compilation-liveness-scaling branch from 6d15343 to 7d85391 Compare September 1, 2026 18:40
@simon1hofmann
simon1hofmann marked this pull request as ready for review September 1, 2026 18:50
@simon1hofmann

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: e6c3f789-42c4-44a6-8869-d8caf20c707e

📥 Commits

Reviewing files that changed from the base of the PR and between 79c7688 and 7d85391.

📒 Files selected for processing (2)
  • mlir/lib/Compiler/TargetCompilation.cpp
  • mlir/unittests/Compiler/test_compiler_pipeline.cpp

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Bug Fixes

    • Improved target compilation cleanup to ensure generated operations are fully eliminated when no longer needed.
    • Prevented unnecessary changes to compiled program output during dead-value cleanup.
  • Tests

    • Added coverage verifying that target compilation reaches a stable state for programs with conditional control flow and private functions.

Walkthrough

Changes

Target compilation cleanup

Layer / File(s) Summary
Native synthesis cleanup pipeline
mlir/lib/Compiler/TargetCompilation.cpp
The pipeline no longer runs createRemoveDeadValuesPass() after native synthesis. CSE remains responsible for removing trivially dead local operations.
Dead-value cleanup fixed-point test
mlir/unittests/Compiler/test_compiler_pipeline.cpp
A test compiles a QCO program and verifies that a subsequent RemoveDeadValues pass leaves the IR unchanged.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 7d853

This change removes a redundant compiler cleanup pass while preserving the remaining pipeline checks and adding fixed-point regression coverage; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: burgholzer, denialhaag

Poem

A rabbit hops through native code,
CSE clears the unused load,
The IR rests, unchanged and bright,
Fixed-point checks confirm it right,
Then carrots mark the clean-up road.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: removing redundant liveness analysis during target compilation.
Description check ✅ Passed The description explains the change, motivation, safety impact, testing, issue context, stack, AI assistance, and checklist status. It does not explicitly list dependencies or use the template's `Fixe…
Full details: Description check

Explanation

The description explains the change, motivation, safety impact, testing, issue context, stack, AI assistance, and checklist status. It does not explicitly list dependencies or use the template's Fixes #(issue) line, but the required information is otherwise sufficiently complete.


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.

@simon1hofmann

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

Base automatically changed from codex/classical-control-mapping to main September 2, 2026 13:23
@simon1hofmann
simon1hofmann force-pushed the codex/target-compilation-liveness-scaling branch from 7d85391 to 697c0a0 Compare September 2, 2026 13:23
Drop the post-synthesis RemoveDeadValues pass because the second QCO cleanup establishes global liveness and native synthesis preserves value flow. Keep CSE for local cleanup and add an end-to-end fixed-point regression.

Assisted-by: Codex
Signed-off-by: Simon Hofmann <simon.t.hofmann@tum.de>
@simon1hofmann
simon1hofmann force-pushed the codex/target-compilation-liveness-scaling branch from 697c0a0 to 2da97cd Compare September 2, 2026 14:11

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

LGTM 👍🏻 it's likely that we are running these passes too often.

@simon1hofmann
simon1hofmann merged commit 30b5d54 into main Sep 2, 2026
26 checks passed
@simon1hofmann
simon1hofmann deleted the codex/target-compilation-liveness-scaling branch September 2, 2026 15:58
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.

2 participants