Skip to content

BE-304: HashQL: Terminator placement based on capabilities#8359

Open
indietyp wants to merge 9 commits intobm/be-303-hashql-split-basic-blocks-depending-on-largest-availablefrom
bm/be-304-hashql-terminator-coloring-based-on-capabilities
Open

BE-304: HashQL: Terminator placement based on capabilities#8359
indietyp wants to merge 9 commits intobm/be-303-hashql-split-basic-blocks-depending-on-largest-availablefrom
bm/be-304-hashql-terminator-coloring-based-on-capabilities

Conversation

@indietyp
Copy link
Member

@indietyp indietyp commented Feb 5, 2026

🌟 What is the purpose of this PR?

Adds terminator placement analysis to the MIR execution planning pipeline. This complements statement placement by analyzing control flow edges rather than individual statements, determining which backend transitions are valid at each edge and their associated transfer costs.

🔍 What does this change?

  • Introduces TransMatrix for encoding valid (source → destination) backend transitions per edge
  • Adds TerminatorCostVec to collect transition matrices for all edges in a body
  • Implements TerminatorPlacement analysis that computes costs based on:
    • Liveness analysis (which locals cross edges)
    • SCC detection (loop membership for Postgres restrictions)
    • Terminator-specific rules (GraphRead, SwitchInt, Goto)
  • Moves execution/ module from pass/analysis/ to pass/ (organizational)
  • Adds helper methods to BodyFootprint for computing average transfer costs

Pre-Merge Checklist 🚀

🚢 Has this modified a publishable library?

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

📜 Does this require a change to the docs?

  • are internal and do not require a docs change

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

  • do not affect the execution graph

🛡 What tests cover this?

New unit tests in terminator_placement/tests.rs covering loops, self-loops, SwitchInt, GraphRead, and Postgres restrictions
Snapshot test for terminator placement output

❓ How to test this?

cargo nextest run --package hashql-mir terminator_placement

@vercel
Copy link

vercel bot commented Feb 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hash Ready Ready Preview, Comment Feb 6, 2026 3:46pm
petrinaut Ready Ready Preview Feb 6, 2026 3:46pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
hashdotdesign Ignored Ignored Preview Feb 6, 2026 3:46pm
hashdotdesign-tokens Ignored Ignored Preview Feb 6, 2026 3:46pm

@cursor
Copy link

cursor bot commented Feb 5, 2026

PR Summary

Medium Risk
Introduces new control-flow transition/cost logic (loop/SCC and liveness-driven) that will affect execution planning decisions; while well-tested, incorrect cost/transition constraints could lead to suboptimal or invalid backend switching.

Overview
Adds a new MIR terminator placement analysis that computes per-control-flow-edge backend transition legality and costs, using liveness + SCC/loop detection and terminator-specific rules (Goto, SwitchInt, GraphRead) to constrain transitions (including stricter Postgres restrictions) and derive transfer costs from size estimates.

Refactors execution-related code out of pass::analysis into a new top-level pass::execution module, updates imports/call sites, and extends size-estimation utilities (Estimate::eval, Footprint::average, range/unit helpers) plus Cost helpers (MAX, saturating_add, TargetId::abbreviation) to support overflow-safe transfer-cost calculation. Also adds list support to the MIR rvalue builder macro and introduces comprehensive unit/snapshot tests for terminator placement.

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

Copy link
Member Author

indietyp commented Feb 5, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

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

@codecov
Copy link

codecov bot commented Feb 5, 2026

Codecov Report

❌ Patch coverage is 95.30387% with 34 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.09%. Comparing base (cf5e803) to head (9a0ea84).

Files with missing lines Patch % Lines
...mir/src/pass/execution/terminator_placement/mod.rs 94.00% 13 Missing and 2 partials ⚠️
.../mir/src/pass/analysis/size_estimation/estimate.rs 47.05% 9 Missing ⚠️
...hql/mir/src/pass/analysis/size_estimation/range.rs 75.00% 4 Missing ⚠️
...r/src/pass/execution/terminator_placement/tests.rs 99.24% 2 Missing and 1 partial ⚠️
...mir/src/pass/analysis/size_estimation/footprint.rs 88.23% 1 Missing and 1 partial ⚠️
...shql/mir/src/pass/analysis/size_estimation/unit.rs 91.66% 1 Missing ⚠️
Additional details and impacted files
@@                                          Coverage Diff                                           @@
##           bm/be-303-hashql-split-basic-blocks-depending-on-largest-available    #8359      +/-   ##
======================================================================================================
+ Coverage                                                               83.87%   84.09%   +0.22%     
======================================================================================================
  Files                                                                     344      346       +2     
  Lines                                                                   50913    51637     +724     
  Branches                                                                 1308     1319      +11     
======================================================================================================
+ Hits                                                                    42701    43423     +722     
+ Misses                                                                   7746     7744       -2     
- Partials                                                                  466      470       +4     
Flag Coverage Δ
rust.hashql-compiletest 29.69% <ø> (ø)
rust.hashql-mir 90.79% <95.30%> (+0.36%) ⬆️

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 Feb 5, 2026

🤖 Augment PR Summary

Summary: Adds terminator-edge backend transition planning to the HashQL MIR execution planner.
Why: Complements statement placement by deciding where backend switches are valid and how costly they are.
Key changes:

  • New TransMatrix + TerminatorCostVec to store (source→dest) transition validity/cost per terminator edge.
  • New TerminatorPlacement analysis combining liveness, SCC/loop detection, and size-estimation to compute transfer costs and enforce backend rules (GraphRead/SwitchInt/Goto, Postgres restrictions).
  • Execution modules reorganized from pass/analysis/execution to pass/execution and exports updated.
  • Size-estimation extended with Estimate::eval, Footprint::average, and range/unit helpers used for transfer-cost calculations.
  • Cost enhanced with MAX sentinel and saturating_add to avoid overflow while accumulating costs.
  • MIR builder rvalue! gains list support, enabling tests to construct list values.
  • New unit + snapshot tests covering loops/self-loops, SwitchInt, GraphRead, and unbounded transfer costs.

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

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 5, 2026

Merging this PR will not alter performance

✅ 21 untouched benchmarks
⏩ 20 skipped benchmarks1


Comparing bm/be-304-hashql-terminator-coloring-based-on-capabilities (9a0ea84) with bm/be-303-hashql-split-basic-blocks-depending-on-largest-available (cf5e803)

Open in CodSpeed

Footnotes

  1. 20 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@graphite-app graphite-app bot requested review from a team February 5, 2026 11:54
@indietyp indietyp force-pushed the bm/be-304-hashql-terminator-coloring-based-on-capabilities branch from c04d28b to 49dcd92 Compare February 6, 2026 08:29
@indietyp indietyp force-pushed the bm/be-303-hashql-split-basic-blocks-depending-on-largest-available branch from 613ef47 to 77f0621 Compare February 6, 2026 08:29
@indietyp indietyp force-pushed the bm/be-304-hashql-terminator-coloring-based-on-capabilities branch from 49dcd92 to 76697d7 Compare February 6, 2026 09:37
@indietyp indietyp force-pushed the bm/be-303-hashql-split-basic-blocks-depending-on-largest-available branch from 77f0621 to 02e1336 Compare February 6, 2026 09:37
@indietyp indietyp force-pushed the bm/be-303-hashql-split-basic-blocks-depending-on-largest-available branch from 02e1336 to ef6aeec Compare February 6, 2026 10:04
@indietyp indietyp force-pushed the bm/be-304-hashql-terminator-coloring-based-on-capabilities branch from 76697d7 to 4298d22 Compare February 6, 2026 10:04
TimDiekmann
TimDiekmann previously approved these changes Feb 6, 2026
@indietyp indietyp force-pushed the bm/be-304-hashql-terminator-coloring-based-on-capabilities branch from 4298d22 to 9a0ea84 Compare February 6, 2026 15:40
@indietyp indietyp force-pushed the bm/be-303-hashql-split-basic-blocks-depending-on-largest-available branch from ef6aeec to cf5e803 Compare February 6, 2026 15:40
@github-actions github-actions bot dismissed TimDiekmann’s stale review February 6, 2026 15:40

Your organization requires reapproval when changes are made, so Graphite has dismissed approvals. See the output of git range-diff at https://github.com/hashintel/hash/actions/runs/21756355054

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.

2 participants