Skip to content

Conversation

@indietyp
Copy link
Member

@indietyp indietyp commented Jan 3, 2026

🌟 What is the purpose of this PR?

This PR refactors the MIR optimization pipeline by extracting the canonicalization logic from PreInline into a reusable Canonicalization pass. This improves code organization and enables the same optimization sequence to be used in multiple contexts.

🔍 What does this change?

  • Extracts the canonicalization logic from PreInline into a new Canonicalization pass
  • Makes PreInline a thin wrapper around Canonicalization with pre-inlining specific configuration
  • Improves the Changed enum's implementation with optimized bitwise operations
  • Adds a new overlay method to GlobalTransformState to combine results from multiple passes
  • Adds a Miri test command for the changed_bitor functionality

Pre-Merge Checklist 🚀

🚢 Has this modified a publishable library?

This PR:

  • does not modify any publishable blocks or libraries, or modifications do not need publishing

📜 Does this require a change to the docs?

The changes in this PR:

  • are internal and do not require a docs change

🕸️ Does this require a change to the Turbo Graph?

The changes in this PR:

  • do not affect the execution graph

🛡 What tests cover this?

  • Added a Miri test for the changed_bitor functionality
  • Existing MIR optimization tests cover the refactored functionality

@vercel vercel bot temporarily deployed to Preview – petrinaut January 3, 2026 17:09 Inactive
@cursor
Copy link

cursor bot commented Jan 3, 2026

PR Summary

Centralizes MIR canonicalization logic and streamlines pre-inlining.

  • New transform/canonicalization.rs: fixpoint driver running AR → IS → FS/CP → DSE → CFG with configurable iterations (CanonicalizationConfig)
  • PreInline rewritten to delegate to Canonicalization (configured with fewer iterations)
  • Changed enum: optimized BitOr using unchecked conversion and #[inline] for faster |= on slices
  • GlobalTransformState: new overlay(&DefIdSlice<Changed>) to merge per-pass results
  • Exports Canonicalization and config via transform/mod.rs
  • Adds test:miri script for changed_bitor

Written by Cursor Bugbot for commit 0fd9d8f. This will update automatically on new commits. Configure here.

@github-actions github-actions bot added area/libs Relates to first-party libraries/crates/packages (area) type/eng > backend Owned by the @backend team labels Jan 3, 2026
Copy link
Member Author

indietyp commented Jan 3, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@indietyp indietyp force-pushed the bm/be-269-hashql-move-out-most-of-preinline-into-a-canonicalization branch from 76ea4a0 to b3793a2 Compare January 3, 2026 17:11
@vercel vercel bot temporarily deployed to Preview – petrinaut January 3, 2026 17:11 Inactive
@augmentcode
Copy link

augmentcode bot commented Jan 3, 2026

🤖 Augment PR Summary

Summary: Refactors the HashQL MIR optimization pipeline by extracting the canonicalization fixpoint logic into a reusable pass.

Changes:

  • Introduces a new Canonicalization global transform pass with a configurable iteration limit.
  • Reworks PreInline into a thin wrapper around Canonicalization (pre-inlining tuned config).
  • Optimizes Changed bitwise OR by using an unchecked discriminant conversion for better codegen/vectorization.
  • Adds GlobalTransformState::overlay to merge per-pass/per-iteration change tracking into a single state.
  • Wires the new module export and adds a test:miri script for the changed_bitor test.

Technical Notes: Canonicalization runs a pre-pass (CP+CFG) and then iterates AR → IS → (FS/CP) → DSE → CFG until fixpoint or max_iterations.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 3, 2026

Merging this PR will not alter performance

✅ 17 untouched benchmarks


Comparing bm/be-269-hashql-move-out-most-of-preinline-into-a-canonicalization (0312fee) with main (bdb020e)

Open in CodSpeed

@codecov
Copy link

codecov bot commented Jan 3, 2026

Codecov Report

❌ Patch coverage is 94.85714% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.74%. Comparing base (4655bbd) to head (0fd9d8f).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
.../hashql/mir/src/pass/transform/canonicalization.rs 95.54% 5 Missing and 2 partials ⚠️
libs/@local/hashql/mir/src/pass/mod.rs 81.81% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8239      +/-   ##
==========================================
+ Coverage   59.72%   59.74%   +0.01%     
==========================================
  Files        1214     1215       +1     
  Lines      115245   115312      +67     
  Branches     5062     5066       +4     
==========================================
+ Hits        68832    68894      +62     
- Misses      45611    45615       +4     
- Partials      802      803       +1     
Flag Coverage Δ
rust.hashql-compiletest 46.65% <ø> (ø)
rust.hashql-mir 89.68% <94.85%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@indietyp indietyp force-pushed the bm/be-268-hashql-rename-preinlining-to-preinline branch from 1120ba1 to 17bacd7 Compare January 17, 2026 16:17
@indietyp indietyp force-pushed the bm/be-269-hashql-move-out-most-of-preinline-into-a-canonicalization branch from e6d0bdc to 5c164f4 Compare January 17, 2026 16:17
@indietyp indietyp force-pushed the bm/be-268-hashql-rename-preinlining-to-preinline branch from 17bacd7 to 4df23e6 Compare January 17, 2026 17:55
@indietyp indietyp force-pushed the bm/be-269-hashql-move-out-most-of-preinline-into-a-canonicalization branch from 5c164f4 to 1ac5573 Compare January 17, 2026 17:55
@graphite-app graphite-app bot changed the base branch from bm/be-268-hashql-rename-preinlining-to-preinline to graphite-base/8239 January 19, 2026 09:34
@indietyp indietyp force-pushed the bm/be-269-hashql-move-out-most-of-preinline-into-a-canonicalization branch from 1ac5573 to 0312fee Compare January 19, 2026 09:55
@vercel vercel bot temporarily deployed to Preview – petrinaut January 19, 2026 09:55 Inactive
@indietyp indietyp force-pushed the bm/be-269-hashql-move-out-most-of-preinline-into-a-canonicalization branch from 0312fee to 0fd9d8f Compare January 19, 2026 09:55
@vercel vercel bot temporarily deployed to Preview – petrinaut January 19, 2026 09:55 Inactive
@graphite-app graphite-app bot changed the base branch from graphite-base/8239 to main January 19, 2026 09:56
@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 19, 2026

Merge activity

  • Jan 19, 9:56 AM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.
  • Jan 19, 9:56 AM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

@indietyp indietyp added this pull request to the merge queue Jan 19, 2026
Merged via the queue into main with commit 7b09441 Jan 19, 2026
47 checks passed
@indietyp indietyp deleted the bm/be-269-hashql-move-out-most-of-preinline-into-a-canonicalization branch January 19, 2026 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/libs Relates to first-party libraries/crates/packages (area) type/eng > backend Owned by the @backend team

Development

Successfully merging this pull request may close these issues.

3 participants