Skip to content

Merge repeated Func bound/align_bounds/align_extent calls per Var - #9410

Open
mcourteaux wants to merge 3 commits into
mainfrom
mcourteaux/merge-func-bounds
Open

Merge repeated Func bound/align_bounds/align_extent calls per Var#9410
mcourteaux wants to merge 3 commits into
mainfrom
mcourteaux/merge-func-bounds

Conversation

@mcourteaux

Copy link
Copy Markdown
Contributor

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, with a user_warning when a call overwrites a field a previous call already set. Every consumer now sees at most one Bound per Var, so call order no longer matters.

Breaking changes

None.

Checklist

  • Tests added or updated (not required for docs, CI config, or typo fixes)
  • Documentation updated (if public API changed)
  • Python bindings updated (if public API changed)
  • Benchmarks are included here if the change is intended to affect performance.
  • Commits include AI attribution where applicable (see Code of Conduct)

@mcourteaux
mcourteaux force-pushed the mcourteaux/merge-func-bounds branch from 2b8bcef to 5dbb22f Compare August 31, 2026 13:00
@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.05%. Comparing base (d9debf9) to head (df3e828).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/Inline.cpp 0.00% 1 Missing and 1 partial ⚠️
src/Schedule.cpp 89.47% 1 Missing and 1 partial ⚠️
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.
📢 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.

Comment thread src/Func.cpp Outdated
Comment thread src/Func.cpp Outdated
Comment thread src/Func.cpp Outdated
@abadams

abadams commented Sep 1, 2026

Copy link
Copy Markdown
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.

mcourteaux and others added 3 commits September 2, 2026 10:40
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
mcourteaux force-pushed the mcourteaux/merge-func-bounds branch from 8eefe12 to df3e828 Compare September 2, 2026 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants