Skip to content

Conversation

@indietyp
Copy link
Member

@indietyp indietyp commented Jan 1, 2026

🌟 What is the purpose of this PR?

Add a new PreInlining optimization pass that runs a sequence of transformations to prepare MIR bodies for inlining. This pass orchestrates multiple optimizations in a fixpoint loop to ensure code is maximally simplified before inlining occurs.

🔍 What does this change?

  • Add new PreInlining pass that runs a carefully ordered sequence of optimizations:
    1. Administrative reduction - Removes structural clutter and normalizes shape
    2. Instruction simplification - Constant folding and algebraic simplification
    3. Value propagation (FS/CP alternating) - Propagates values through the code
    4. Dead store elimination - Removes stores made dead by propagation
    5. CFG simplification - Cleans up control flow after local changes
  • Add GlobalTransformState to track per-body changes during global transformations
  • Update AdministrativeReduction to use the new state tracking mechanism
  • Add comprehensive test cases demonstrating the pass's effectiveness on various code patterns
  • Add a new compiletest suite for the pre-inlining pass

🛡 What tests cover this?

  • Added 8 new test cases covering various optimization patterns:
    • Constant folding and branch elimination
    • Cascading conditional simplification
    • Closure inlining with dead branch elimination
    • Dead code elimination after value propagation
    • Instruction simplification with value propagation
    • Nested conditional simplification
    • Nested let binding cleanup
    • Thunk inlining with dead code elimination

❓ How to test this?

  1. Run the MIR tests: cargo test -p hashql_mir
  2. Run the compiletest suite: cargo test -p hashql_compiletest -- mir/pass/transform/pre-inlining

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

@vercel vercel bot temporarily deployed to Preview – petrinaut January 1, 2026 15:08 Inactive
@cursor
Copy link

cursor bot commented Jan 1, 2026

PR Summary

Introduces a new global MIR optimization driver and supporting infra, and wires it into tests/benchmarks.

  • Adds PreInlining global pass running a fixpoint of AdministrativeReductionInstSimplify → alternating ForwardSubstitution/CopyPropagationDeadStoreEliminationCfgSimplify
  • Adds GlobalTransformState (+ owned variant) to track per-body changes; updates AdministrativeReduction to consume it and mark changed bodies
  • New compiletest suite mir/pass/transform/pre-inlining with text/D2 renderers and multiple UI tests; registers suite in runner
  • Benchmarks: replace manual pipeline with PreInlining; assign DefId to bodies
  • Minor CfgSimplify simplify() fast-path/ordering tweak

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

Copy link
Member Author

indietyp commented Jan 1, 2026

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

@augmentcode
Copy link

augmentcode bot commented Jan 1, 2026

🤖 Augment PR Summary

Summary: Introduces a new MIR optimization driver pass (PreInlining) to aggressively simplify MIR bodies before the inlining stage.

Changes:

  • Add transform::PreInlining, which runs a fixpoint pipeline (AR → InstSimplify → FS/CP alternating → DSE → CfgSimplify) plus an initial CP+CFG pre-pass.
  • Add GlobalTransformState (borrowed) and OwnedGlobalTransformState (owned) for per-body Changed tracking in global MIR transforms.
  • Update GlobalTransformPass::run signature to accept &mut GlobalTransformState and adapt call sites/tests.
  • Update AdministrativeReduction to mark per-body changes via the new state and return early when nothing is reducible.
  • Minor CfgSimplify refactor to avoid unnecessary CFG recomputation for non-branching terminators.
  • Add and register a new compiletest suite (mir/pass/transform/pre-inlining) plus multiple UI test cases exercising the new pipeline.

Technical notes: PreInlining maintains an internal “unstable body” set and iterates (bounded by MAX_ITERATIONS) until no further changes occur.

🤖 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. 1 suggestions posted.

Fix All in Augment

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

@codecov
Copy link

codecov bot commented Jan 1, 2026

Codecov Report

❌ Patch coverage is 92.52874% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.20%. Comparing base (719edd0) to head (720ee51).

Files with missing lines Patch % Lines
libs/@local/hashql/mir/src/pass/mod.rs 61.90% 8 Missing ⚠️
...ocal/hashql/mir/src/pass/transform/pre_inlining.rs 96.92% 2 Missing and 2 partials ⚠️
.../hashql/mir/src/pass/transform/cfg_simplify/mod.rs 83.33% 1 Missing ⚠️
Additional details and impacted files
@@                                     Coverage Diff                                      @@
##           bm/be-265-hashql-track-locals-inside-of-copy-propagation    #8233      +/-   ##
============================================================================================
+ Coverage                                                     80.94%   81.20%   +0.26%     
============================================================================================
  Files                                                            81       82       +1     
  Lines                                                         10575    10744     +169     
  Branches                                                        276      283       +7     
============================================================================================
+ Hits                                                           8560     8725     +165     
- Misses                                                         1907     1912       +5     
+ Partials                                                        108      107       -1     
Flag Coverage Δ
rust.hashql-compiletest 46.65% <ø> (ø)
rust.hashql-mir 87.58% <92.52%> (+0.19%) ⬆️

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.

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 1, 2026

Merging this PR will degrade performance by 47.92%

❌ 3 (👁 3) regressed benchmarks
✅ 14 untouched benchmarks

Performance Changes

Benchmark BASE HEAD Efficiency
👁 linear 32.9 µs 40.4 µs -18.56%
👁 diamond 37.3 µs 71.4 µs -47.75%
👁 complex 53.8 µs 103.2 µs -47.92%

Comparing bm/be-266-hashql-implement-pre-inlining-fix-point-loop (3e0b23b) with main (4a6d675)

Open in CodSpeed

@vercel vercel bot temporarily deployed to Preview – petrinaut January 1, 2026 18:00 Inactive
@indietyp indietyp force-pushed the bm/be-266-hashql-implement-pre-inlining-fix-point-loop branch from cf2afea to a941112 Compare January 17, 2026 16:17
@indietyp indietyp force-pushed the bm/be-265-hashql-track-locals-inside-of-copy-propagation branch from dd0c9b9 to a300008 Compare January 17, 2026 16:17
@graphite-app graphite-app bot changed the base branch from bm/be-265-hashql-track-locals-inside-of-copy-propagation to graphite-base/8233 January 17, 2026 16:38
@indietyp indietyp force-pushed the bm/be-266-hashql-implement-pre-inlining-fix-point-loop branch from a941112 to 3e0b23b Compare January 17, 2026 16:56
@vercel vercel bot temporarily deployed to Preview – petrinaut January 17, 2026 16:56 Inactive
@graphite-app graphite-app bot changed the base branch from graphite-base/8233 to main January 17, 2026 16:57
@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 17, 2026

Merge activity

  • Jan 17, 4:57 PM 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 17, 2026
Merged via the queue into main with commit 40bee1c Jan 17, 2026
66 of 79 checks passed
@indietyp indietyp deleted the bm/be-266-hashql-implement-pre-inlining-fix-point-loop branch January 17, 2026 17:21
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) area/tests New or updated tests type/eng > backend Owned by the @backend team

Development

Successfully merging this pull request may close these issues.

3 participants