Shared bounds tracker machinery. - #9399
Open
mcourteaux wants to merge 18 commits into
Open
mcourteaux wants to merge 18 commits into
mcourteaux wants to merge 18 commits into
Conversation
mcourteaux
commented
Aug 27, 2026
| // happen to be multiples of c0 -- because c0 > 0 means both | ||
| // "+c2" and "/c0" distribute over min. | ||
| rewrite((min(x * c0 + c1, y) + c2) / c0 - x, min((y + c2) / c0 - x, fold((c1 + c2) / c0)), c0 > 0) || | ||
| rewrite((min(y, x * c0 + c1) + c2) / c0 - x, min((y + c2) / c0 - x, fold((c1 + c2) / c0)), c0 > 0) || |
Contributor
Author
There was a problem hiding this comment.
@abadams I think these can be in the general Simplify_Sub? They are genuine simplifications.
mcourteaux
commented
Aug 27, 2026
| PROPERTIES | ||
| EXPORT_COMPILE_COMMANDS NO | ||
| POSITION_INDEPENDENT_CODE ON | ||
| ) |
Contributor
Author
There was a problem hiding this comment.
@alexreinking Drive-by fix for PIC on the initmod.
Member
There was a problem hiding this comment.
Open a separate PR for this, please.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9399 +/- ##
==========================================
+ Coverage 69.95% 70.19% +0.24%
==========================================
Files 261 263 +2
Lines 79596 79858 +262
Branches 19400 19470 +70
==========================================
+ Hits 55678 56053 +375
+ Misses 18004 17980 -24
+ Partials 5914 5825 -89 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
|
I like the idea of deduplicating bounds tracking logic, but I think this PR inflated lowering times by getting much more aggressive with bounds, based on our offline discussions. Is my memory correct? If this is still a WIP please mark it as a draft. |
mcourteaux
marked this pull request as draft
September 2, 2026 09:25
…cations/AllocationBoundsInference Introduces BoundsTracker, a struct that accumulates enclosing pure LetStmt/Let bindings and dominating facts while a mutator descends a Stmt tree, and uses them to find constant bounds far more reliably than a bare find_constant_bound() call. In addition to a Scope<Interval> fast path, find_constant_bound_aggressive()/find_constant_bounds_aggressive() fall back to wrapping an expression in all pending pure lets, inlining them with substitute_in_all_lets, and re-simplifying under the dominating facts -- generalizing the trick bound_constant_extent_loops has always used to find constant loop extents. Migrates all three targeted passes onto it: - BoundConstantExtentLoops: same two-tier (exact vs guarded upper bound) unroll/vectorize logic, now expressed via find_constant_bounds_aggressive()'s interval collapse-to-a-point check instead of a separate ad hoc IntImm check. - BoundSmallAllocations: Frame/visit_let chain now binds through tracker.push_let(); find_constant_bound() call sites upgraded to find_constant_bound_aggressive() so allocation/realize extents get the same aggressive treatment. - AllocationBoundsInference: gains LetStmt/For tracking it never had before, and runs the box_touched() result through tracker.simplify_with_context() before CSE, so a Realize's per- dimension min/max can be simplified using enclosing let context that box_touched (called with an empty scope) can't see on its own. KNOWN ISSUE (not yet resolved): correctness_unroll_loop_with_implied_constant_bounds segfaults via infinite recursion inside Simplify's fact/var_info substitution machinery, triggered from BoundConstantExtentLoops's aggressive fallback when two dominating facts (a bounds-query check and a 4-way equality conjunction "three_channels") are both active for the same simplify() call. Confirmed via debug instrumentation that BoundsTracker builds the same wrapped expression and fact list the original hand-rolled implementation would have; the crash is inside the Simplify engine's own var_info replacement logic (Simplify_Exprs.cpp:270-282), not in BoundsTracker's bookkeeping. Root cause not yet isolated -- needs a minimal standalone repro against Simplify() directly (bypassing BoundsTracker/Lower.cpp entirely) to determine whether this is a latent pre-existing Simplify bug that BoundConstantExtentLoops previously never triggered, or a subtle behavioral difference between BoundsTracker's fact accumulation and the original vector-based one. All other targeted correctness tests (bounds, bound_small_allocations, unroll, vectorize, realize, extern, sliding_window, partition_loops, split, etc.) pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019EGMmdqNC6mTcMSCDBFbwV
…okup BoundLoops::visit(For*) took bounds.max.as<IntImm>() as a raw pointer while `bounds` was a stack-local Interval about to go out of scope. If that Interval's Expr was the only thing keeping the underlying IntImm node's refcount alive, the node could be freed as soon as `bounds` was destroyed, leaving `e` dangling. The freed memory would typically get reused shortly after (while unwinding through further LetStmt/IfThenElse processing), corrupting the IntImm embedded in the constructed For loop and manifesting later as an infinite Add/Sub/Variable recursion inside Simplify -- reported by the user as a segfault in correctness_unroll_loop_with_implied_constant_bounds, reproduced and fixed with their help using an ASan build. Fixed by copying the Expr into `extent_upper` (already a function-scoped local used for the guarded-upper-bound case) before extracting the raw IntImm pointer from it, so the node stays referenced for the rest of the function regardless of which branch is taken. Also fixes the CMake issue that blocked building an ASan config in the first place: Halide_initmod (the object library holding the runtime's embedded bitcode blobs, linked into the shared Halide target) never had POSITION_INDEPENDENT_CODE set, unlike the Halide target itself. This happened to link fine in optimized builds, where x86-64 codegen tends to use RIP-relative addressing regardless, but failed with absolute 32-bit relocation errors in unoptimized/Debug/ASan builds. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019EGMmdqNC6mTcMSCDBFbwV
Both DetermineAllocStride and LowerWarpShuffles maintained their own Scope<Interval> bounds, populated on For loops only when the loop's min and max were literal constants (is_const(op->min) && is_const(op->max)), and never updated by LetStmt/Let at all -- so any allocation size or stride computation that depended on a let-bound intermediate value (very common after earlier lowering passes hoist bounds calculations into lets) had no way to resolve to a constant. All uses of `bounds` in this file only ever feed it into simplify() or reduce_expr() (itself simplify()-based), never bounds_of_expr_in_scope() directly -- and Simplify's own internal bounds representation is constant-only anyway (it converts via as_const_int at ingestion), so BoundsTracker's constant-collapsing scope loses nothing here, unlike SlidingWindow/HexagonOptimize which need genuinely symbolic interval tracking BoundsTracker doesn't provide (left unmigrated). Adds two small BoundsTracker capabilities needed by this pass: - interval_scope(): exposes the underlying Scope<Interval> for passes that feed it directly to simplify()/similar rather than going through find_constant_bound(). - push_interval(): pushes an already-computed Interval directly, for LowerWarpShuffles::visit(IfThenElse*)'s lane-masking case, which narrows an existing binding rather than deriving a new one. Verified with a full correctness suite run under an actual CUDA JIT target (HL_TARGET/HL_JIT_TARGET=host-cuda, reconfigured via -DHalide_TARGET=host-cuda since ctest bakes the target into each test's ENVIRONMENT property at configure time rather than inheriting it from the shell). One unrelated pre-existing failure (correctness_gpu_register_at_block_level, in PromoteGPURegisters/ MultiRamp, which runs before LowerWarpShuffles in the pipeline) was confirmed to reproduce identically against an unmodified origin/main build with the same target override, so it predates this change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019EGMmdqNC6mTcMSCDBFbwV
…Tracker SimplifyCorrelatedDifferences doesn't just back find_constant_bounds() (via the exported bound_correlated_differences() on a single Expr) -- simplify_correlated_differences() is also run directly as a whole-tree lowering pass in Lower.cpp, several times. Give it a BoundsTracker so it gathers the same constant-bounds context the other migrated passes do, and use it in cancel_correlated_subexpression()'s final simplify() call. This complements rather than replaces the pass's existing `lets` tracking (used to wrap terms for CSE before solve_expression): `lets` deliberately excludes pure lets that are constant w.r.t. the current loop_var, since the monotonicity analysis doesn't need them, but the final simplify() can still benefit from resolving them, and from dominating assert conditions this pass previously never looked at at all (new visit(Block*) override, peeling leading asserts the same way BoundConstantExtentLoops peels dominating if-conditions). Deliberately uses interval_scope()/known_facts() fed straight into simplify(), not find_constant_bound_aggressive()'s more powerful wrap-every-pending-let-and-resimplify path: this pass is already documented as quadratic in loop nesting depth and runs across the whole tree multiple times, so paying that cost on every correlated-difference site would be a real compile-time risk. (Chased what looked like a ~9x compile-time regression down this path during development -- it turned out to be an unrelated Debug-vs-RelWithDebInfo build type mismatch between the two binaries being compared, not anything caused by this change or the earlier migrations; a same-build-type comparison confirms no regression.) Verified with a full correctness suite run (CUDA JIT target enabled): 462/463 passed, with the one failure being the same pre-existing, unrelated GPU register-allocation issue already confirmed to reproduce against an unmodified origin/main build. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019EGMmdqNC6mTcMSCDBFbwV
A producer compute_at a plain (non-aligned) split tile of its consumer, with the tail handled by PredicateStores, still needs bounds inference to find a compile-time-constant bound for the tile's extent in order to unroll it -- PredicateStores only predicates the consumer's store, not the loads that feed it, so the producer's required region for a boundary tile stays tied to the consumer's declared extent rather than becoming an unconditional full tile. Combined with align_bounds() rounding that region's extent up to a multiple of the tile factor, the resulting extent expression is a ceiling-divide of a min-clamped quantity minus the unclamped multiple the clamp is anchored to: (min(x*c0 + c1, y) + c2)/c0 - x. No existing rule covered it, so bounds inference found no bound at all and unrolling failed outright, even though the region provably fits in one tile. Add that as an exact identity (not just a bound) to SimplifyCorrelatedDifferences's PartiallyCancelDifferences: c0 > 0 means both "+c2" and "/c0" distribute over min, so it reduces to min((y+c2)/c0 - x, (c1+c2)/c0) unconditionally, not just when c1/c2 happen to be multiples of c0. Also push the enclosing loop's own range into BoundConstantExtentLoops' BoundsTracker before recursing into its body, and resimplify the extent with that context before giving up -- matching the pattern BoundSmallAllocations, AllocationBoundsInference, and LowerWarpShuffles already use. Not load-bearing for the new test (the SimplifyCorrelatedDifferences rule alone already finds the bound), but the same class of gap for any nested extent that only resolves once an enclosing loop's bound is in scope.
An unrolled producer tile inside a PredicateStores split gets an extent of the form (min(x*c + c, y) + c)/c*c - x*c, where the enclosing tile loop's own max (a ceiling-divide of y) is exactly what bounds y from below and makes the ceiling-divide exact. BoundConstantExtentLoops could only find the upper bound, so it unrolled to the split factor and wrapped the body in a guard that is always true. Two gaps, both on BoundsTracker's deliberately-expensive slow path: simplify_with_context inlines the enclosing lets, which is what makes the loop variable appear on both sides of the extent's subtraction -- but nothing cancelled it back out. Run bound_correlated_differences and re-simplify, keeping the result only when it actually shrank (it can grow the expression). That turns the extent into min((y + c)/c - x, 1)*c. That form is monotonic in the loop variable, so its extremes over the loop are reached at the ends of the loop's range -- but push_for only recorded a constants-only Interval, which drops the fact that the loop's max mentions y too. Record the range symbolically as well, and have find_constant_bounds_aggressive substitute the endpoints into an expression is_monotonic() says is monotonic in that variable. Substitution keeps y correlated between the expression and the loop bound where per-node interval arithmetic can't, so the extent comes out as exactly [c, c]. This reuses Monotonic.h's existing analysis rather than teaching the simplifier a new kind of fact, and costs nothing until the cheap paths have already failed to find a bound. split_predicate_stores_compute_at now checks for no guard at all inside the unrolled tile, rather than at most one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018SvYp54SUAXaJ49MizJg63
…_aggressive The single-Direction form was missing the loop-monotonicity fallback the Interval form has, so the two disagreed about how hard they tried. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H8YeexA67LffkteXd9ekdy
…seful as we're not trying to find any upper bound.
Pushing a binding used to evaluate everything it might imply on the spot:
find_constant_bounds() on every let's value, is_pure() on it, and two fact
Exprs per loop. Passes that walk a whole tree but query at a handful of
nodes paid that on every node they descended through, which showed up as
2-4x slowdowns in bound_constant_extent_loops and
simplify_correlated_differences.
Push now only records the Expr. The constant bounds, the purity and the
loop facts are derived on the first query and memoized for the binding's
lifetime. Bindings realize outermost-first into a prefix of the scope, so
derivation order and the shadowing of repeated names are unchanged.
The slow path also stopped discriminating: it wrapped the query in every
pending pure let and handed the simplifier every fact. It now wraps only
the lets the expression transitively reaches -- the trick
bound_constant_extent_loops used before it moved onto BoundsTracker -- and
passes only the facts sharing a variable with it, swept to a fixed point.
known_facts() becomes relevant_facts(e) for that reason.
Lowering times for the passes on this machinery, mean of 3 runs of the
apps/ generators (merge-base -> before -> after, ms):
lens_blur local_laplacian
total 532 682 621 318 376 330
bounding constant extent loops 0.6 4.4 0.5 0.6 2.3 0.5
bounding small realizations 10 28 2.8 7.6 16 9.9
bounding small allocations 13 97 72 5.2 27 18
simplifying correlated diffs 25 81 26 20 46 22
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B8pMM1zmBL89nNwzwCEJM4
wrap_in_used_lets() inlines every pure let the query reaches before
anything asks for a bound, so deriving constant bounds for those values at
push time was work whose result nothing consulted. Bounding the inlined
expression is also sharper than bounding the value and then looking it up
through an opaque variable.
A pure let still takes a slot in the scope, holding Interval::everything():
one shadowing an outer variable that does have a bound has to hide it,
rather than letting lookups fall through to a bound that no longer applies.
An impure let is never inlined, so it keeps a real bound.
Lowering time, mean of 3 runs of the apps/ generators (ms):
lens_blur local_laplacian
total 544 -> 526 339 -> 313
bounding small allocations 65.8 -> 58.1 17.3 -> 15.6
bounding small realizations 2.87 -> 2.73 9.90 -> 5.67
The lowered IR is unchanged: both generators emit byte-identical stmt
output once temporary numbering is normalized.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B8pMM1zmBL89nNwzwCEJM4
Inlining every wrapped let up front repeats its value at each use. The
result is the same graph, but the simplifier walks it as a tree and does not
memoize on the shared nodes, so the walk grows with the number of paths
rather than the number of nodes. On the Mullapudi2016 schedule for
apps/lens_blur, a 456-node graph reached the simplifier as a 70549-node
walk, and bound_small_allocations spent 1.5s there across 41 allocations --
all of which it failed to bound, leaving the IR untouched.
simplify() already inlines a Let whose value is trivial or used once, so
handing it the lets rather than the inlined expression keeps the cancelling
power that matters here.
Lowering time, mean of 3 runs of every apps/ generator that reports one --
129 pipelines over 28 apps, merge base vs this branch (ms):
before after
total, 28 apps 15084 13455 (main: 13241)
lens_blur 2542 987 (main: 994)
resnet_50 964 969 (main: 846)
Both the full test suite (743 tests) and the stmt output of all 132 app
pipelines are unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B8pMM1zmBL89nNwzwCEJM4
relevant_facts() selects the conditions that could say something about an expression: those sharing a variable with it, swept to a fixed point. The check walked each candidate and built a std::set of its names every time it was asked, so the cost grew with the enclosing loop depth times the number of sweeps. Lowering apps/resnet_50 spent 124ms of the 152ms in simplify_correlated_differences there, across 92 queries that each considered 3080 candidates and ran 707584 of those checks to select at most eight conditions. A Variable's IRNode::hash is derived from its name, so two sets of variables can be compared as sorted lists of 32-bit hashes, with a Bloom filter making the common case of sharing nothing a single AND. Each loop entry caches the two conditions its range implies and the variables they mention; an entry's range never changes, so that survives any number of queries, and the candidate list is rebuilt only when a fact or an enclosing loop is pushed or popped. Hashes collide, which costs one extra condition handed to the simplifier and nothing else: this only has to avoid dropping a condition that matters. The selection is unchanged on resnet_50 -- the same 428 conditions over the same 92 queries. simplify_correlated_differences, apps/resnet_50 152ms -> 38ms gathering the facts 124ms -> 7ms whole-pipeline lowering 940ms -> 796ms Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8pMM1zmBL89nNwzwCEJM4
The list of conditions the enclosing loops imply was rebuilt from every binding in scope whenever one of those loops changed, so asking cost time proportional to the loop nest depth even when almost nothing had moved since the last question. Lowering apps/resnet_50 rebuilt a 3080-entry list at each of 92 queries. The list is a stack over the same bindings, so it can be extended as they are pushed and trimmed as they are popped, the way `realized` already tracks which bindings have reached the scope. Asking now costs only what the bindings pushed since the last question cost, and the version counters that drove the rebuild are gone. simplify_correlated_differences, apps/resnet_50, mean of 8 runs: 39.22ms -> 38.24ms (min 38.08ms -> 37.16ms) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8pMM1zmBL89nNwzwCEJM4
A pure let contributes no bounds of its own -- whatever it is worth is found by inlining it and bounding the result. It was still pushed into the scope as an unbounded interval, purely so that it would hide an enclosing binding of the same name. But a lookup that misses falls back to the bounds of the type, which is exactly what an unbounded interval narrows to, so such a binding reads the same as no binding at all unless there is something of the same name to hide. Most names are unique by the time these passes run, so nearly all of those slots were dead weight in the map that every lookup searches, and in the scope handed to simplify(). simplify_correlated_differences, apps/resnet_50, mean of 8 runs: 38.27ms -> 32.64ms (min 37.34ms -> 31.67ms) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8pMM1zmBL89nNwzwCEJM4
relevant_facts() sweeps the conditions in force until no more become relevant, since one that shares a variable with the expression can bring in variables that make a third relevant. Every sweep re-tested every condition, but after the first only the variables the previous sweep picked up can make one relevant that wasn't before. Tracking those and skipping conditions that share none of them leaves the later sweeps proportional to what they find rather than to the enclosing loop depth. Lowering apps/resnet_50 sweeps ~3080 conditions at each of 92 queries. The conditions selected are unchanged -- the same 428 over the same queries. simplify_correlated_differences, apps/resnet_50, mean of 8 runs: 33.56ms -> 31.52ms (min 31.83ms -> 29.49ms) of which picking the conditions: 6.4ms -> 4.1ms Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8pMM1zmBL89nNwzwCEJM4
mcourteaux
force-pushed
the
mcourteaux/bounds-tracker
branch
from
September 11, 2026 18:52
a3528fb to
4996ae4
Compare
mcourteaux
marked this pull request as ready for review
September 11, 2026 19:02
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
3 participants
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.
Problem statement
When a simple call to
simplify()is executed on the root of the IR tree, it gathers a lot of facts and bounds along the way, which can be exploited to simplify Exprs more given this context.Several passes make use of
simplify()when attempting to find an upper bound to some Expr:BoundConstantLoopExtent: used for when a loop has to be unrolled, but the loop extent is variable, but has an upper bound: the loop extent will be set to the found upper bound, and if-guards will be inserted in the loop body.BoundSmallAllocations: same principle: we like to put things on the stack, and not malloc, so this pass tries to find an upper bound to the required storage.LowerWarpShuffles: finds size of hoisted allocations.The problem with these is that the passes themselves drill down the IR to find the allocation or loop on which they have to work and find their upper bound. So they would walk past all the evidence that
simplify()could have picked up, and then executesimplify()in without all that evidence passed. This obviously doesn't work so each of those passes implements some form of the same evidence gathering assimplify()does. Each pass implements it's own subset of such operations and passes those asScope<ConstantBounds>orstd::vector<Expr> assumptionstosimplify(), which seems very ad-hoc.Solution
This PR introduces the
BoundsTracker: a shared utility which these bound-seeking passes can use to collect similar info as a regularsimplify()on the root would do. The BoundsTracker now has a few utility functions the passes can make use of:simplify_with_context()Runs the simplifier along with given facts and constant bounds intervals.find_constant_bound()Tries to find an upper bound by passing constant bounds.find_constant_bound_aggressive()Tries to find an upper bound by usingfind_constant_bound()first (as a fast path), if that fails, resorts to a sequence ofsimplify_with_context(),find_constant_bound(), andtighten_using_loop_monotonicity()as a hardest attempt to find an actual good lower/upper bound.PR open for feedback. There are two more lowering passes that can make use of this:
StorageFoldingtries to find an upper bound for the allocation too.HexagonOptimizehas one call size using find_constant_bound(). Perhaps not that useful. Used during lowering of div_round_to_zero and mod_round_to_zero.Performance
Breaking changes
None.
These do not necessarily disqualify a PR from being merged, but they should at
least be tagged with the
release_noteslabel.Checklist