Merge repeated Func bound/align_bounds/align_extent calls per Var - #9410
Open
mcourteaux wants to merge 3 commits into
Open
Merge repeated Func bound/align_bounds/align_extent calls per Var#9410mcourteaux wants to merge 3 commits into
mcourteaux wants to merge 3 commits into
Conversation
mcourteaux
force-pushed
the
mcourteaux/merge-func-bounds
branch
from
August 31, 2026 13:00
2b8bcef to
5dbb22f
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9410 +/- ##
==========================================
+ Coverage 69.95% 70.05% +0.09%
==========================================
Files 261 261
Lines 79402 79424 +22
Branches 19360 19365 +5
==========================================
+ Hits 55546 55639 +93
+ Misses 17932 17929 -3
+ Partials 5924 5856 -68 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
abadams
reviewed
Aug 31, 2026
abadams
reviewed
Aug 31, 2026
Member
|
LGTM but the linux-32 failure looks like it might be real. Dropping a bound could slow that down. Could also be a flake though. I'll rerun it. |
Func::bound(), align_bounds(), and align_extent() each appended an independent Bound entry to FuncSchedule::bounds(), even when a Bound for that Var already existed. With two entries for the same Var, every consumer of bounds() (BoundsInference, ScheduleFunctions, AllocationBoundsInference) had to guess how to combine them, and did so inconsistently -- most notably, BoundsInference's LetStmt-based composition ends up applying the entries in the reverse of the order they were pushed, which is why the call order between bound_extent() and align_bounds() mattered and, when reversed, made bounds inference derive a too-small region (a runtime "do not cover required region" failure), not just leftover unsimplified expressions. Fold repeated calls for the same Var into a single Bound instead. Each of the four Bound fields (min, extent, modulus, remainder) is merged independently: a call that leaves a field undefined never touches whatever an earlier call set for that field, and a call that does set a field overwrites it, with a user_warning if it had already been set to something different. Every consumer now sees at most one Bound per Var, so call order no longer matters. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Change Schedule::bounds to be a map instead of a vector. Deserialization keeps it backwards compatible and loads in a vector.
mcourteaux
force-pushed
the
mcourteaux/merge-func-bounds
branch
from
September 2, 2026 08:41
8eefe12 to
df3e828
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Func::bound(),align_bounds(), andalign_extent()each appended an independentBoundentry toFuncSchedule::bounds(), even when aBoundfor thatVaralready existed. With two entries for the sameVar, every consumer ofbounds()(BoundsInference,ScheduleFunctions,AllocationBoundsInference) had to guess how to combine them, and did so inconsistently -- most notably,BoundsInference'sLetStmt-based composition ends up applying the entries in the reverse of the order they were pushed, which is why the call order betweenbound_extent()andalign_bounds()mattered and, when reversed, made bounds inference derive a too-small region (a runtime "do not cover required region" failure), not just leftover unsimplified expressions.Fold repeated calls for the same
Varinto a singleBoundinstead, with auser_warningwhen a call overwrites a field a previous call already set. Every consumer now sees at most oneBoundper Var, so call order no longer matters.Breaking changes
None.
Checklist