Skip to content

Fix quadratic bounds blow-up at the source - #9448

Merged
abadams merged 4 commits into
mainfrom
abadams/fix_quadratic_bounds_inference
Sep 16, 2026
Merged

abadams merged 4 commits into
mainfrom
abadams/fix_quadratic_bounds_inference

Conversation

@abadams

@abadams abadams commented Sep 14, 2026

Copy link
Copy Markdown
Member

Alternative to #9442

Bounds inference scales quadratically in the number of update definitions of a Func. This is because all stages are considered to depend on all earlier stages. This makes bgu's bounds inference stage very slow.

The root cause is the quadratic loop over all (consumer, producer) pairs. This can't be made linear, because there could genuinely be a quadratic number of relationships between stages. But we can skip or neuter iterations of this loop to at least not send quadratic amounts of IR downstream.

The general problem is cases where A depends on B, B depends on C, and A also directly depends on C, and the transitive dependence of A on C via B is equivalent to the direct dependence.

This PR skips once instance of this. It skips the dependence of one update stage on earlier update stages along a particular axis if the very next update stage would have the same dependence on earlier update stages along that axis because the var is pure in both. I.e. this situation:

f(x, y) = ...

... some number of update defs ...

f(x, 0) += 3; // update def 37
f(x, 2) += 4; // update def 38

There's no need to consider update def 37's dependence on earlier stages, because it's going to be the same as update def 38's.

This is more powerful than the existing optimization because it still kicks in if most but not all of the update defs are pure in x.

Bounds inference scales quadratically in the number of update
definitions of a Func. This is because all stages are considered to
depend on all earlier stages. This makes bgu's bounds inference stage
very slow.

The root cause is the quadratic loop over all (consumer, producer)
pairs. This can't be made linear, because there could genuinely be a
quadratic number of relationships between stages. But we can skip or
neuter iterations of this loop to at least not send quadratic amounts of
IR downstream.

The general problem is cases where A depends on B, B depends on C, and A
also directly depends on C, and the transitive dependence of A on C via
B is equivalent to the direct dependence.

This PR skips once instance of this. It skips the dependence of one
update stage on earlier update stages along a particular axis if the
very next update stage would have the same dependence on earlier update
stages along that axis because the var is pure in both. I.e. this
situation:

f(x, y) = ...

... some number of update defs ...

f(x, 0) += 3; // update def 37
f(x, 2) += 4; // update def 38

There's no need to consider update def 37's dependence on earlier
stages, because it's going to be the same as update def 38's.

This is more powerful than the existing optimization because it still
kicks in if most but not all of the update defs are pure in x.

@alexreinking alexreinking left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: debug prints could be better

Comment thread src/BoundsInference.cpp
Comment on lines +938 to 949
if (!b[k].is_bounded()) {
std::ostringstream err;
if (consumer.stage == 0) {
err << "The pure definition ";
} else {
err << "Update definition number " << (consumer.stage - 1);
}
err << " of Function " << consumer.name
<< " calls function " << producer.name
<< " in an unbounded way in dimension " << k << "\n";
user_error << err.str();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

user_assert(b[k].is_bounded()) << [&] {
    std::ostringstream err;
    // ...
    return err.str();
}();

Comment thread src/BoundsInference.cpp Outdated
@codecov

codecov Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 62.50000% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.13%. Comparing base (d10a1f2) to head (5896dd8).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
src/BoundsInference.cpp 60.00% 24 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9448      +/-   ##
==========================================
+ Coverage   70.02%   70.13%   +0.10%     
==========================================
  Files         261      261              
  Lines       79761    79850      +89     
  Branches    19443    19459      +16     
==========================================
+ Hits        55855    56003     +148     
  Misses      18004    18004              
+ Partials     5902     5843      -59     

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

@mcourteaux

mcourteaux commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Merged main into this PR, and benchmarked it with 3 reps for every generator of the apps.

report-host-fix-quadratic-bounds-dark

@abadams Somehow the impact on gaussian_blur seems real. Producer consumer order stays the same, but it seems to generate quite some different code compared to main. It looks like a genuine regression: the code generated on main looks like it underwent loop partitioning or perhaps some unrolling of produce down_y, where the PR version didn't.

@mcourteaux

Copy link
Copy Markdown
Contributor

New commit (all good!):

report-host-fix-quadratic-bounds-dark

@abadams

abadams commented Sep 16, 2026

Copy link
Copy Markdown
Member Author

Failure is the MAX_PATH windows nonsense

@abadams
abadams merged commit d226bfc into main Sep 16, 2026
27 of 28 checks passed
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