Skip to content

Conversation

@indietyp
Copy link
Member

🌟 What is the purpose of this PR?

Enhance the Place::local function to be more efficient by making it a const fn that doesn't require an interner parameter, and add a convenient From<Local> implementation for Operand to simplify code that converts locals to operands.

🔍 What does this change?

  • Make Place::local() a const fn that doesn't require an interner parameter
  • Add From<Local> implementation for Operand to simplify code
  • Update all call sites to use the new signature
  • Enhance copy propagation to handle both constant and local value propagation
  • Add tests for copy propagation with local values

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 new tests for copy propagation with local values
  • Existing tests continue to pass with the updated API

@vercel vercel bot temporarily deployed to Preview – petrinaut December 31, 2025 12:14 Inactive
@cursor
Copy link

cursor bot commented Dec 31, 2025

PR Summary

Modernizes constructor APIs and broadens MIR optimizations.

  • Make Place::local() and Target::block() const and remove interner args; switch to Interned::empty() and update all usages (reify, builders, passes, docs/examples)
  • Add From<Local> for Operand to simplify local-to-operand conversion
  • Enhance copy propagation to track and substitute both constants and locals (KnownValue), including block-param consensus; update visitor logic accordingly
  • Adjust CFG simplify simplify_goto signature (drops context param) and update call site
  • Update data-dependency resolution and SSA/administrative reduction to new Place::local usage
  • Add and snapshot new tests for copy propagation of locals and chains (simple_copy, copy_chain, block-param variants)

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

Copy link
Member Author

indietyp commented Dec 31, 2025

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

@indietyp indietyp force-pushed the bm/be-265-hashql-track-locals-inside-of-copy-propagation branch from 5702e6e to d4a3d01 Compare December 31, 2025 12:16
@vercel vercel bot temporarily deployed to Preview – petrinaut December 31, 2025 12:16 Inactive
@indietyp indietyp force-pushed the bm/be-265-hashql-track-locals-inside-of-copy-propagation branch from d4a3d01 to 8e4f5be Compare December 31, 2025 12:19
@indietyp indietyp force-pushed the bm/be-264-hashql-rename-sroa-to-projectionforwarding branch from f3c0958 to 0fc7994 Compare December 31, 2025 12:19
@vercel vercel bot temporarily deployed to Preview – petrinaut December 31, 2025 12:19 Inactive
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 31, 2025

Merging this PR will not alter performance

✅ 17 untouched benchmarks


Comparing bm/be-265-hashql-track-locals-inside-of-copy-propagation (bee6b36) with main (ef9788b)

Open in CodSpeed

@codecov
Copy link

codecov bot commented Dec 31, 2025

Codecov Report

❌ Patch coverage is 96.42857% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.94%. Comparing base (b51e5cd) to head (719edd0).

Files with missing lines Patch % Lines
libs/@local/hashql/mir/src/body/operand.rs 0.00% 3 Missing ⚠️
libs/@local/hashql/mir/src/reify/rvalue.rs 50.00% 1 Missing ⚠️
Additional details and impacted files
@@                                   Coverage Diff                                    @@
##           bm/be-264-hashql-rename-sroa-to-projectionforwarding    #8231      +/-   ##
========================================================================================
+ Coverage                                                 80.77%   80.94%   +0.17%     
========================================================================================
  Files                                                        81       81              
  Lines                                                     10527    10575      +48     
  Branches                                                    282      276       -6     
========================================================================================
+ Hits                                                       8503     8560      +57     
+ Misses                                                     1914     1907       -7     
+ Partials                                                    110      108       -2     
Flag Coverage Δ
rust.hashql-compiletest 46.65% <ø> (ø)
rust.hashql-mir 87.39% <96.42%> (+0.17%) ⬆️

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.

@augmentcode
Copy link

augmentcode bot commented Dec 31, 2025

🤖 Augment PR Summary

Summary: Extends HashQL MIR copy propagation to handle local-to-local copies (not just constants) and simplifies MIR place/operand helpers.

Changes:

  • Place::local(Local) is now a const fn and no longer needs an Interner; it uses a canonical empty projection list.
  • Adds From<Local> for Operand to streamline local-to-operand conversions.
  • Updates call sites across MIR builder, reifier, SSA repair, CFG simplify, and data-dependency resolution to the new Place::local signature.
  • Generalizes CopyPropagation to track a KnownValue (constant or local) and substitute operands, including via block-parameter agreement.
  • Adds new unit tests + snapshots for simple copy, chained copy, and block-param copy/disagreement scenarios.

Technical Notes: The pass remains single-pass (reverse postorder), does not traverse projections, and assumes SSA-like single-assignment locals for soundness.

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

@vercel vercel bot temporarily deployed to Preview – petrinaut December 31, 2025 15:00 Inactive
@graphite-app graphite-app bot requested a review from a team December 31, 2025 15:09
@indietyp indietyp force-pushed the bm/be-264-hashql-rename-sroa-to-projectionforwarding branch from de61ae3 to 5f7b4cc 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
Base automatically changed from bm/be-264-hashql-rename-sroa-to-projectionforwarding to main January 17, 2026 16:37
@indietyp indietyp force-pushed the bm/be-265-hashql-track-locals-inside-of-copy-propagation branch from a300008 to bee6b36 Compare January 17, 2026 16:38
@vercel vercel bot temporarily deployed to Preview – petrinaut January 17, 2026 16:38 Inactive
@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 17, 2026

Merge activity

  • Jan 17, 4:38 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 4a6d675 Jan 17, 2026
47 checks passed
@indietyp indietyp deleted the bm/be-265-hashql-track-locals-inside-of-copy-propagation branch January 17, 2026 16:52
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