From f2a160fb93a37eba8c2e119b4bce1ebc706dad79 Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Tue, 25 Aug 2026 18:14:54 +0200 Subject: [PATCH 01/17] WIP: shared BoundsTracker for BoundConstantExtentLoops/BoundSmallAllocations/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 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 Claude-Session: https://claude.ai/code/session_019EGMmdqNC6mTcMSCDBFbwV --- src/AllocationBoundsInference.cpp | 24 +++++- src/BoundConstantExtentLoops.cpp | 76 +++++++++--------- src/BoundSmallAllocations.cpp | 24 +++--- src/BoundsTracker.cpp | 110 ++++++++++++++++++++++++++ src/BoundsTracker.h | 126 ++++++++++++++++++++++++++++++ src/CMakeLists.txt | 2 + 6 files changed, 303 insertions(+), 59 deletions(-) create mode 100644 src/BoundsTracker.cpp create mode 100644 src/BoundsTracker.h diff --git a/src/AllocationBoundsInference.cpp b/src/AllocationBoundsInference.cpp index 9aaaf1c7e661..187aad4f1f6f 100644 --- a/src/AllocationBoundsInference.cpp +++ b/src/AllocationBoundsInference.cpp @@ -1,5 +1,6 @@ #include "AllocationBoundsInference.h" #include "Bounds.h" +#include "BoundsTracker.h" #include "CSE.h" #include "ExternFuncArgument.h" #include "Function.h" @@ -19,10 +20,6 @@ using std::vector; namespace { -Expr cse_and_simplify(const Expr &x) { - return simplify(common_subexpression_elimination(x)); -} - // Figure out the region touched of each buffer, and deposit them as // let statements outside of each realize node, or at the top level if // they're not internal allocations. @@ -34,6 +31,25 @@ class AllocationInference : public IRMutator { const FuncValueBounds &func_bounds; set touched_by_extern; + // Tracks the enclosing pure lets and for loops, so that box_touched's + // result -- computed with no knowledge of anything outside op->body -- + // can be simplified as thoroughly as if it had been. + BoundsTracker tracker; + + Expr cse_and_simplify(const Expr &x) { + return simplify(common_subexpression_elimination(tracker.simplify_with_context(x))); + } + + Stmt visit(const LetStmt *op) override { + auto binding = tracker.push_let(op->name, op->value); + return IRMutator::visit(op); + } + + Stmt visit(const For *op) override { + auto binding = tracker.push_for(op->name, op->min, op->max); + return IRMutator::visit(op); + } + Stmt visit(const Realize *op) override { map::const_iterator iter = env.find(op->name); internal_assert(iter != env.end()); diff --git a/src/BoundConstantExtentLoops.cpp b/src/BoundConstantExtentLoops.cpp index ebc41007e6bf..5691ee575df6 100644 --- a/src/BoundConstantExtentLoops.cpp +++ b/src/BoundConstantExtentLoops.cpp @@ -1,11 +1,10 @@ #include "BoundConstantExtentLoops.h" #include "Bounds.h" -#include "CSE.h" +#include "BoundsTracker.h" #include "IRMutator.h" #include "IROperator.h" #include "Simplify.h" -#include "SimplifyCorrelatedDifferences.h" -#include "Substitute.h" +#include "Util.h" namespace Halide { namespace Internal { @@ -15,29 +14,23 @@ class BoundLoops : public IRMutator { protected: using IRMutator::visit; - std::vector> lets; + BoundsTracker tracker; Stmt visit(const LetStmt *op) override { - if (is_pure(op->value)) { - lets.emplace_back(op->name, op->value); - Stmt s = IRMutator::visit(op); - lets.pop_back(); - return s; - } else { - return IRMutator::visit(op); - } + auto binding = tracker.push_let(op->name, op->value); + return IRMutator::visit(op); } - std::vector facts; Stmt visit(const IfThenElse *op) override { - facts.push_back(op->condition); - Stmt then_case = mutate(op->then_case); - Stmt else_case; + Stmt then_case, else_case; + { + auto fact = tracker.push_fact(op->condition); + then_case = mutate(op->then_case); + } if (op->else_case.defined()) { - facts.back() = simplify(!op->condition); + auto fact = tracker.push_fact(simplify(!op->condition)); else_case = mutate(op->else_case); } - facts.pop_back(); if (then_case.same_as(op->then_case) && else_case.same_as(op->else_case)) { return op; @@ -59,33 +52,36 @@ class BoundLoops : public IRMutator { Stmt body = op->body; const IntImm *e = extent.as(); - if (e == nullptr) { - // We're about to hard fail. Get really aggressive - // with the simplifier. - extent = rewrap_used_lets(extent, lets); - extent = remove_likelies(extent); - extent = substitute_in_all_lets(extent); - extent = simplify(extent, - Scope::empty_scope(), - Scope::empty_scope(), - facts); - e = extent.as(); - } - Expr extent_upper; if (e == nullptr) { - // Still no luck. Try taking an upper bound and - // injecting an if statement around the body. - extent_upper = find_constant_bound(extent, Direction::Upper, Scope()); - if (extent_upper.defined()) { - e = extent_upper.as(); - body = - IfThenElse::make(likely_if_innermost(Variable::make(Int(32), op->name) <= - op->max), - body); + // We're about to hard fail. Get really aggressive with the + // simplifier: inline every enclosing let and simplify under + // every dominating condition. + debug(4) << "Trying to find a constant bound for loop " << op->name << "\n" + << "Extent: " << extent << "\n"; + Interval bounds = tracker.find_constant_bounds_aggressive(extent); + debug(4) << "Bounds found: [" << bounds.min << ", " << bounds.max << "]\n"; + auto lo = bounds.has_lower_bound() ? as_const_int(bounds.min) : std::nullopt; + auto hi = bounds.has_upper_bound() ? as_const_int(bounds.max) : std::nullopt; + if (lo && hi && *lo == *hi) { + // The bound is exact. + e = bounds.max.as(); + } else if (hi) { + extent_upper = bounds.max; } } + if (e == nullptr && extent_upper.defined()) { + // Still no luck getting an exact extent. Take the upper + // bound instead and guard the body with an if statement. + debug(4) << "Found an upper bound instead: " << extent_upper << "\n"; + e = extent_upper.as(); + body = + IfThenElse::make(likely_if_innermost(Variable::make(Int(32), op->name) <= + op->max), + body); + } + if (e == nullptr && permit_failed_unroll && op->for_type == ForType::Unrolled) { // Still no luck, but we're allowed to fail. Rewrite // to a serial loop. diff --git a/src/BoundSmallAllocations.cpp b/src/BoundSmallAllocations.cpp index c8683b08a4ce..9c987c04dd6e 100644 --- a/src/BoundSmallAllocations.cpp +++ b/src/BoundSmallAllocations.cpp @@ -1,9 +1,8 @@ #include "BoundSmallAllocations.h" -#include "Bounds.h" +#include "BoundsTracker.h" #include "CodeGen_Internal.h" #include "IRMutator.h" #include "IROperator.h" -#include "Simplify.h" namespace Halide { namespace Internal { @@ -15,17 +14,17 @@ class BoundSmallAllocations : public IRMutator { using IRMutator::visit; // Track constant bounds - Scope scope; + BoundsTracker tracker; template auto visit_let(const LetOrLetStmt *op) -> decltype(op->body) { // Visit an entire chain of lets in a single method to conserve stack space. struct Frame { const LetOrLetStmt *op; - ScopedBinding binding; - Frame(const LetOrLetStmt *op, Scope &scope) + BoundsTracker::Binding binding; + Frame(const LetOrLetStmt *op, BoundsTracker &tracker) : op(op), - binding(scope, op->name, find_constant_bounds(op->value, scope)) { + binding(tracker.push_let(op->name, op->value)) { } }; std::vector frames; @@ -33,7 +32,7 @@ class BoundSmallAllocations : public IRMutator { do { result = op->body; - frames.emplace_back(op, scope); + frames.emplace_back(op, tracker); } while ((op = result.template as())); result = mutate(result); @@ -58,12 +57,7 @@ class BoundSmallAllocations : public IRMutator { DeviceAPI device_api = DeviceAPI::None; Stmt visit(const For *op) override { - Interval min_bounds = find_constant_bounds(op->min, scope); - Interval max_bounds = find_constant_bounds(op->max, scope); - Interval b = Interval::make_union(min_bounds, max_bounds); - b.min = simplify(b.min); - b.max = simplify(b.max); - ScopedBinding bind(scope, op->name, b); + auto binding = tracker.push_for(op->name, op->min, op->max); bool new_in_thread_loop = in_thread_loop || op->for_type == ForType::GPUThread; ScopedValue old_in_thread_loop(in_thread_loop, new_in_thread_loop); @@ -86,7 +80,7 @@ class BoundSmallAllocations : public IRMutator { bool changed = false; bool found_non_constant_extent = false; for (Range &r : region) { - Expr bound = find_constant_bound(r.extent, Direction::Upper, scope); + Expr bound = tracker.find_constant_bound_aggressive(r.extent, Direction::Upper); // We can allow non-constant extents for now, as long as all // remaining dimensions are 1 (so the stride is unused, which // will be non-constant). @@ -116,7 +110,7 @@ class BoundSmallAllocations : public IRMutator { for (const Expr &e : op->extents) { total_extent *= e; } - Expr bound = find_constant_bound(total_extent, Direction::Upper, scope); + Expr bound = tracker.find_constant_bound_aggressive(total_extent, Direction::Upper); if (!bound.defined() && must_be_constant(op->memory_type)) { user_assert(op->memory_type != MemoryType::Register) diff --git a/src/BoundsTracker.cpp b/src/BoundsTracker.cpp new file mode 100644 index 000000000000..d4d155072f36 --- /dev/null +++ b/src/BoundsTracker.cpp @@ -0,0 +1,110 @@ +#include "BoundsTracker.h" + +#include "IR.h" +#include "IROperator.h" +#include "Simplify.h" +#include "Substitute.h" + +namespace Halide { +namespace Internal { + +BoundsTracker::Binding::Binding(BoundsTracker *tracker, ScopedBinding scope_binding, bool recorded_let) + : tracker(tracker), scope_binding(std::move(scope_binding)), recorded_let(recorded_let) { +} + +BoundsTracker::Binding::Binding(Binding &&other) noexcept + : tracker(other.tracker), + scope_binding(std::move(other.scope_binding)), + recorded_let(other.recorded_let) { + other.recorded_let = false; +} + +BoundsTracker::Binding::~Binding() { + if (recorded_let) { + tracker->lets.pop_back(); + } +} + +BoundsTracker::Binding BoundsTracker::push_for(const std::string &name, const Expr &min, const Expr &max) { + Interval min_bounds = find_constant_bounds(min); + Interval max_bounds = find_constant_bounds(max); + Interval b = Interval::make_union(min_bounds, max_bounds); + b.min = simplify(b.min); + b.max = simplify(b.max); + return Binding(this, ScopedBinding(scope, name, b), false); +} + +BoundsTracker::Binding BoundsTracker::push_let(const std::string &name, const Expr &value) { + bool pure = is_pure(value); + if (pure) { + lets.emplace_back(name, value); + } + return Binding(this, ScopedBinding(scope, name, find_constant_bounds(value)), pure); +} + +BoundsTracker::FactGuard::FactGuard(BoundsTracker *tracker) + : tracker(tracker) { +} + +BoundsTracker::FactGuard::FactGuard(FactGuard &&other) noexcept + : tracker(other.tracker) { + other.tracker = nullptr; +} + +BoundsTracker::FactGuard::~FactGuard() { + if (tracker) { + tracker->facts.pop_back(); + } +} + +BoundsTracker::FactGuard BoundsTracker::push_fact(const Expr &condition) { + facts.push_back(condition); + return FactGuard(this); +} + +Expr BoundsTracker::find_constant_bound(const Expr &e, Direction d) const { + return Halide::Internal::find_constant_bound(e, d, scope); +} + +Interval BoundsTracker::find_constant_bounds(const Expr &e) const { + return Halide::Internal::find_constant_bounds(e, scope); +} + +Expr BoundsTracker::simplify_with_context(const Expr &e) const { + Expr wrapped = e; + for (const auto &[name, value] : reverse_view(lets)) { + wrapped = Let::make(name, value, wrapped); + } + wrapped = remove_likelies(wrapped); + wrapped = substitute_in_all_lets(wrapped); + // Deliberately pass an empty bounds scope here, not `scope`: mixing a + // bounds scope with equality facts can make the simplifier represent a + // variable by its (wide) interval instead of substituting the exact + // value an equality fact implies, which weakens the very reasoning this + // call exists to do. Any tightening from `scope` happens afterward, once + // this expression is no longer being asked to prove an equality. + wrapped = Halide::Internal::simplify(wrapped, Scope::empty_scope(), + Scope::empty_scope(), facts); + return wrapped; +} + +Expr BoundsTracker::find_constant_bound_aggressive(const Expr &e, Direction d) const { + Expr bound = find_constant_bound(e, d); + if (bound.defined()) { + return bound; + } + Expr wrapped = simplify_with_context(e); + return Halide::Internal::find_constant_bound(wrapped, d, scope); +} + +Interval BoundsTracker::find_constant_bounds_aggressive(const Expr &e) const { + Interval interval = find_constant_bounds(e); + if (interval.has_lower_bound() && interval.has_upper_bound()) { + return interval; + } + Expr wrapped = simplify_with_context(e); + return Halide::Internal::find_constant_bounds(wrapped, scope); +} + +} // namespace Internal +} // namespace Halide diff --git a/src/BoundsTracker.h b/src/BoundsTracker.h new file mode 100644 index 000000000000..f5af577b8055 --- /dev/null +++ b/src/BoundsTracker.h @@ -0,0 +1,126 @@ +#ifndef HALIDE_BOUNDS_TRACKER_H +#define HALIDE_BOUNDS_TRACKER_H + +/** \file + * A utility for finding constant bounds of expressions at some point inside + * a Stmt tree. + */ + +#include +#include +#include + +#include "Bounds.h" +#include "Expr.h" +#include "Scope.h" + +namespace Halide { +namespace Internal { + +/** Accumulates the bounds-relevant context available at some point inside a + * Stmt tree -- enclosing pure LetStmt/Let bindings and For loop ranges -- as + * a mutator or visitor descends, and uses it to find constant bounds for + * expressions at that point far more reliably than a bare + * find_constant_bound() call. + * + * In addition to the usual scope-based lookup (cheap, but only sees a bound + * if every intermediate variable it passes through was itself pushed with an + * already-constant bound), find_constant_bound_aggressive() falls back to + * literally wrapping an expression in all enclosing pure lets, inlining + * them, and re-simplifying. This is the trick bound_constant_extent_loops + * has always used to find constant loop extents, generalized so other + * passes that infer constant bounds (e.g. BoundSmallAllocations, + * AllocationBoundsInference) can use it too. + */ +class BoundsTracker { +public: + /** An RAII binding produced by push_for/push_let. Pops everything it + * pushed when destroyed. */ + class Binding { + public: + Binding() = default; + Binding(const Binding &) = delete; + Binding &operator=(const Binding &) = delete; + Binding(Binding &&other) noexcept; + ~Binding(); + + private: + friend class BoundsTracker; + Binding(BoundsTracker *tracker, ScopedBinding scope_binding, bool recorded_let); + + BoundsTracker *tracker = nullptr; + ScopedBinding scope_binding; + bool recorded_let = false; + }; + + /** Push the bounds of a for loop variable: the envelope [lower bound of + * min, upper bound of max], each resolved against everything pushed so + * far. */ + Binding push_for(const std::string &name, const Expr &min, const Expr &max); + + /** Push a let binding. Always updates the fast-path scope with a + * constant-bounds estimate of the value. Additionally records the + * syntactic binding for find_constant_bound_aggressive()'s slow path, + * but only if the value is pure -- substituting an impure expression + * into multiple places would change its meaning. */ + Binding push_let(const std::string &name, const Expr &value); + + /** An RAII guard produced by push_fact. Pops the fact when destroyed. */ + class FactGuard { + public: + FactGuard() = default; + FactGuard(const FactGuard &) = delete; + FactGuard &operator=(const FactGuard &) = delete; + FactGuard(FactGuard &&other) noexcept; + ~FactGuard(); + + private: + friend class BoundsTracker; + explicit FactGuard(BoundsTracker *tracker); + + BoundsTracker *tracker = nullptr; + }; + + /** Push a condition known to be true at this point (e.g. because we're + * in the then-case of an IfThenElse that tests it, or it's the + * condition of a dominating assert). Used as a simplifier assumption by + * the slow path in find_constant_bound_aggressive(). Doesn't affect the + * fast-path scope. */ + FactGuard push_fact(const Expr &condition); + + /** Fast path only: find a constant bound using the current scope. See + * find_constant_bound() in Bounds.h. */ + Expr find_constant_bound(const Expr &e, Direction d) const; + Interval find_constant_bounds(const Expr &e) const; + + /** Fast path first; on failure, wrap e in all pending pure lets + * (producing a self-contained copy with no free references to enclosing + * lets), inline them with substitute_in_all_lets, and simplify before + * retrying against the resulting expression and the scope. More + * expensive than find_constant_bound(), but succeeds far more often, + * because the simplifier can cancel terms across let boundaries that + * interval arithmetic through opaque variable lookups cannot. */ + Expr find_constant_bound_aggressive(const Expr &e, Direction d) const; + Interval find_constant_bounds_aggressive(const Expr &e) const; + + /** Wrap e in all pending pure lets, inline them with + * substitute_in_all_lets, and simplify under the current scope and + * dominating facts. Unlike find_constant_bound_aggressive(), the result + * need not be a constant -- this is just a plain simplify() call that + * can see context (enclosing let values, dominating conditions) that a + * caller holding only a bare Expr has no way to pass in. Useful for + * expressions built by context-free helpers (e.g. box_touched) whose + * result ends up referencing variables bound by lets enclosing the + * point the helper was called from. */ + Expr simplify_with_context(const Expr &e) const; + +private: + Scope scope; + std::vector> lets; + std::vector facts; +}; + +} // namespace Internal +} // namespace Halide + +#endif diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 02215600ef8e..407eafccca8b 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -71,6 +71,7 @@ target_sources( Bounds.h BoundsInference.h BoundSmallAllocations.h + BoundsTracker.h Buffer.h Callable.h CanonicalizeGPUVars.h @@ -255,6 +256,7 @@ target_sources( Bounds.cpp BoundsInference.cpp BoundSmallAllocations.cpp + BoundsTracker.cpp Buffer.cpp Callable.cpp CanonicalizeGPUVars.cpp From ab27fcd50d40a528d69a184e13cc27d523510a53 Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Tue, 25 Aug 2026 20:46:32 +0200 Subject: [PATCH 02/17] Fix use-after-free in BoundConstantExtentLoops's aggressive extent lookup BoundLoops::visit(For*) took bounds.max.as() 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 Claude-Session: https://claude.ai/code/session_019EGMmdqNC6mTcMSCDBFbwV --- src/BoundConstantExtentLoops.cpp | 15 ++++++++++----- src/runtime/CMakeLists.txt | 13 ++++++++++++- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/BoundConstantExtentLoops.cpp b/src/BoundConstantExtentLoops.cpp index 5691ee575df6..17d896bf3739 100644 --- a/src/BoundConstantExtentLoops.cpp +++ b/src/BoundConstantExtentLoops.cpp @@ -1,5 +1,4 @@ #include "BoundConstantExtentLoops.h" -#include "Bounds.h" #include "BoundsTracker.h" #include "IRMutator.h" #include "IROperator.h" @@ -63,11 +62,17 @@ class BoundLoops : public IRMutator { debug(4) << "Bounds found: [" << bounds.min << ", " << bounds.max << "]\n"; auto lo = bounds.has_lower_bound() ? as_const_int(bounds.min) : std::nullopt; auto hi = bounds.has_upper_bound() ? as_const_int(bounds.max) : std::nullopt; - if (lo && hi && *lo == *hi) { - // The bound is exact. - e = bounds.max.as(); - } else if (hi) { + if (hi) { + // Copy the Expr out of `bounds` before it goes out of + // scope below -- otherwise e, taken as a raw pointer via + // as(), would be left dangling into a node whose + // only reference was owned by this soon-to-be-destroyed + // Interval. extent_upper = bounds.max; + if (lo && *lo == *hi) { + // The bound is exact: no guard needed. + e = extent_upper.as(); + } } } diff --git a/src/runtime/CMakeLists.txt b/src/runtime/CMakeLists.txt index cac63a328b52..74beb59978c6 100644 --- a/src/runtime/CMakeLists.txt +++ b/src/runtime/CMakeLists.txt @@ -155,7 +155,18 @@ add_library(Halide_initmod OBJECT) add_library(Halide::initmod ALIAS Halide_initmod) # All these are binary2cpp-generated files, so no need to export compile commands for them. -set_target_properties(Halide_initmod PROPERTIES EXPORT_COMPILE_COMMANDS NO) +# POSITION_INDEPENDENT_CODE must be set explicitly: this object library is +# consumed by the Halide target (see POSITION_INDEPENDENT_CODE there), but as +# an OBJECT library its own sources don't automatically inherit that -- without +# it, non-optimized builds (e.g. Debug, ASan) can fail to link into a shared +# Halide with relocation errors on the large embedded byte arrays generated +# here, even though optimized builds usually happen to produce PIC-compatible +# code anyway on x86-64 and don't show the problem. +set_target_properties(Halide_initmod + PROPERTIES + EXPORT_COMPILE_COMMANDS NO + POSITION_INDEPENDENT_CODE ON +) # Note: ensure that these flags match the flags in the Makefile. # Note: this always uses Clang-from-LLVM for compilation, so none of these flags should need conditionalization. From b0afb2f8ee6f89b34fe663dfb38832e7a22338c8 Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Tue, 25 Aug 2026 23:38:43 +0200 Subject: [PATCH 03/17] Migrate LowerWarpShuffles onto BoundsTracker Both DetermineAllocStride and LowerWarpShuffles maintained their own Scope 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 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 Claude-Session: https://claude.ai/code/session_019EGMmdqNC6mTcMSCDBFbwV --- src/BoundsTracker.cpp | 4 +++ src/BoundsTracker.h | 20 +++++++++++++++ src/LowerWarpShuffles.cpp | 53 +++++++++++++++++++++++---------------- 3 files changed, 55 insertions(+), 22 deletions(-) diff --git a/src/BoundsTracker.cpp b/src/BoundsTracker.cpp index d4d155072f36..63aca5219588 100644 --- a/src/BoundsTracker.cpp +++ b/src/BoundsTracker.cpp @@ -34,6 +34,10 @@ BoundsTracker::Binding BoundsTracker::push_for(const std::string &name, const Ex return Binding(this, ScopedBinding(scope, name, b), false); } +BoundsTracker::Binding BoundsTracker::push_interval(const std::string &name, const Interval &interval) { + return Binding(this, ScopedBinding(scope, name, interval), false); +} + BoundsTracker::Binding BoundsTracker::push_let(const std::string &name, const Expr &value) { bool pure = is_pure(value); if (pure) { diff --git a/src/BoundsTracker.h b/src/BoundsTracker.h index f5af577b8055..44c578530c02 100644 --- a/src/BoundsTracker.h +++ b/src/BoundsTracker.h @@ -58,6 +58,13 @@ class BoundsTracker { * far. */ Binding push_for(const std::string &name, const Expr &min, const Expr &max); + /** Push an already-computed Interval directly, bypassing derivation. + * Useful when a caller has proven a tighter bound for a variable already + * in scope (e.g. because a dominating conditional narrows it) and wants + * to temporarily refine it. Does not participate in + * find_constant_bound_aggressive()'s let-substitution. */ + Binding push_interval(const std::string &name, const Interval &interval); + /** Push a let binding. Always updates the fast-path scope with a * constant-bounds estimate of the value. Additionally records the * syntactic binding for find_constant_bound_aggressive()'s slow path, @@ -114,6 +121,19 @@ class BoundsTracker { * point the helper was called from. */ Expr simplify_with_context(const Expr &e) const; + /** The current fast-path scope of constant bounds, for passes that need + * to feed it directly into simplify() or a similar helper that accepts a + * Scope of assumptions, rather than going through + * find_constant_bound(). Note that Simplify's own internal bounds + * representation is constant-only anyway (it converts via as_const_int + * at ingestion), so this scope -- itself always constant-or-unbounded -- + * loses nothing for that use case. It is not, however, suitable for + * general symbolic interval arithmetic (e.g. bounds_of_expr_in_scope) + * where a non-constant symbolic bound would otherwise be useful. */ + const Scope &interval_scope() const { + return scope; + } + private: Scope scope; std::vector> lets; diff --git a/src/LowerWarpShuffles.cpp b/src/LowerWarpShuffles.cpp index fb800c3f37e4..de0f0e7d9715 100644 --- a/src/LowerWarpShuffles.cpp +++ b/src/LowerWarpShuffles.cpp @@ -1,5 +1,6 @@ #include "LowerWarpShuffles.h" +#include "BoundsTracker.h" #include "ExprUsesVar.h" #include "IREquality.h" #include "IRMatch.h" @@ -139,7 +140,7 @@ class DetermineAllocStride : public IRVisitor { // be assumed to be zero. Scope dependent_vars; - Scope bounds; + BoundsTracker tracker; // Get the derivative of an integer expression w.r.t the warp // lane. Returns an undefined Expr if the result is non-trivial. @@ -189,11 +190,13 @@ class DetermineAllocStride : public IRVisitor { void visit(const Let *op) override { ScopedBinding bind(dependent_vars, op->name, warp_stride(op->value)); + auto bounds_bind = tracker.push_let(op->name, op->value); IRVisitor::visit(op); } void visit(const LetStmt *op) override { ScopedBinding bind(dependent_vars, op->name, warp_stride(op->value)); + auto bounds_bind = tracker.push_let(op->name, op->value); IRVisitor::visit(op); } @@ -233,9 +236,7 @@ class DetermineAllocStride : public IRVisitor { } void visit(const For *op) override { - ScopedBinding - bind_bounds_if(is_const(op->min) && is_const(op->max), - bounds, op->name, Interval(op->min, op->max)); + auto bounds_bind = tracker.push_for(op->name, op->min, op->max); ScopedBinding bound_dependent_if((expr_uses_vars(op->min, dependent_vars) || expr_uses_vars(op->max, dependent_vars)), @@ -285,7 +286,7 @@ class DetermineAllocStride : public IRVisitor { // A version of can_prove which exploits the constant bounds we've been tracking bool can_prove(const Expr &e) { - return is_const_one(simplify(e, bounds)); + return is_const_one(simplify(e, tracker.interval_scope())); } Expr get_stride() { @@ -307,7 +308,7 @@ class DetermineAllocStride : public IRVisitor { // any already discovered on previous stores. bool this_ok = (s.defined() && (can_prove(stride == s) && - can_prove(reduce_expr(e / stride - var, warp_size, bounds) == 0))); + can_prove(reduce_expr(e / stride - var, warp_size, tracker.interval_scope()) == 0))); internal_assert(stride.defined()); @@ -331,7 +332,7 @@ class DetermineAllocStride : public IRVisitor { for (const Expr &e : single_stores) { // If only thread zero was active for the store, that makes the proof simpler. Expr simpler = substitute(lane_var, 0, e); - bool this_ok = can_prove(reduce_expr(simpler / stride, warp_size, bounds) == 0); + bool this_ok = can_prove(reduce_expr(simpler / stride, warp_size, tracker.interval_scope()) == 0); if (!this_ok) { bad.push_back(e); } @@ -367,13 +368,21 @@ class LowerWarpShuffles : public IRMutator { Expr stride; }; Scope allocation_info; - Scope bounds; + BoundsTracker tracker; int cuda_cap; + Expr visit(const Let *op) override { + auto binding = tracker.push_let(op->name, op->value); + return IRMutator::visit(op); + } + + Stmt visit(const LetStmt *op) override { + auto binding = tracker.push_let(op->name, op->value); + return IRMutator::visit(op); + } + Stmt visit(const For *op) override { - ScopedBinding - bind_if(is_const(op->min) && is_const(op->max), - bounds, op->name, Interval(op->min, op->max)); + auto bounds_bind = tracker.push_for(op->name, op->min, op->max); if (!this_lane.defined() && op->for_type == ForType::GPULane) { bool should_mask = false; @@ -411,8 +420,8 @@ class LowerWarpShuffles : public IRMutator { // the number of lanes (rounded up). Expr extent = op->extent(); Expr new_size = (alloc->extents[0] + extent - 1) / extent; - new_size = simplify(new_size, bounds); - new_size = find_constant_bound(new_size, Direction::Upper, bounds); + new_size = simplify(new_size, tracker.interval_scope()); + new_size = tracker.find_constant_bound_aggressive(new_size, Direction::Upper); auto sz = as_const_int(new_size); user_assert(sz) << "Warp-level allocation with non-constant size: " << alloc->extents[0] << ". Use Func::bound_extent."; @@ -472,11 +481,11 @@ class LowerWarpShuffles : public IRMutator { if ((lt && equal(lt->a, this_lane) && is_const(lt->b)) || (le && equal(le->a, this_lane) && is_const(le->b))) { Expr condition = mutate(op->condition); - const Interval *in = bounds.find(this_lane_name); + const Interval *in = tracker.interval_scope().find(this_lane_name); internal_assert(in); Interval interval = *in; interval.max = lt ? simplify(lt->b - 1) : le->b; - ScopedBinding bind(bounds, this_lane_name, interval); + auto bind = tracker.push_interval(this_lane_name, interval); Stmt then_case = mutate(op->then_case); Stmt else_case = mutate(op->else_case); return IfThenElse::make(condition, then_case, else_case); @@ -506,7 +515,7 @@ class LowerWarpShuffles : public IRMutator { // of the index and shifting the high bits down to cover // them. Reassembling the result into a flat address gives // the expression below. - Expr in_warp_idx = simplify((idx / (warp_size * stride)) * stride + reduce_expr(idx, stride, bounds), bounds); + Expr in_warp_idx = simplify((idx / (warp_size * stride)) * stride + reduce_expr(idx, stride, tracker.interval_scope()), tracker.interval_scope()); return op->with(value, in_warp_idx, op->predicate, ModulusRemainder()); } else { return IRMutator::visit(op); @@ -531,7 +540,7 @@ class LowerWarpShuffles : public IRMutator { // Load the right lanes from stripe number i equiv = select(idx >= i, make_warp_load(type, name, make_const(idx.type(), i), lane), equiv); } - return simplify(equiv, bounds); + return simplify(equiv, tracker.interval_scope()); } // Load the value to be shuffled @@ -600,7 +609,7 @@ class LowerWarpShuffles : public IRMutator { } else if (expr_match((this_lane + wild) % wild, lane, result) && (bits = is_const_power_of_two_integer(result[1])) && *bits <= 5) { - result[0] = simplify(result[0] % result[1], bounds); + result[0] = simplify(result[0] % result[1], tracker.interval_scope()); // Rotate. Mux a shuffle up and a shuffle down. Uses fewer // intermediate registers than using a general gather for // this. @@ -611,7 +620,7 @@ class LowerWarpShuffles : public IRMutator { shfl_args({membermask, base_val, (1 << *bits) - result[0], 0}), Call::PureExtern); Expr cond = (this_lane >= (1 << *bits) - result[0]); Expr equiv = select(cond, up, down); - shuffled = simplify(equiv, bounds); + shuffled = simplify(equiv, tracker.interval_scope()); } else { // The format of the mask is a pain. The high bits tell // you how large the a warp is for this instruction @@ -641,10 +650,10 @@ class LowerWarpShuffles : public IRMutator { Expr stride = alloc->stride; // Break the index into lane and stripe components - Expr lane = simplify(reduce_expr(idx / stride, warp_size, bounds), bounds); - idx = simplify((idx / (warp_size * stride)) * stride + reduce_expr(idx, stride, bounds), bounds); + Expr lane = simplify(reduce_expr(idx / stride, warp_size, tracker.interval_scope()), tracker.interval_scope()); + idx = simplify((idx / (warp_size * stride)) * stride + reduce_expr(idx, stride, tracker.interval_scope()), tracker.interval_scope()); // We don't want the idx to depend on the lane var, so try to eliminate it - idx = simplify(solve_expression(idx, this_lane_name).result, bounds); + idx = simplify(solve_expression(idx, this_lane_name).result, tracker.interval_scope()); return make_warp_load(op->type, op->name, idx, lane); } else { return IRMutator::visit(op); From 32404729974a7d15d93db7eda6470250d0ef235c Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Wed, 26 Aug 2026 00:24:21 +0200 Subject: [PATCH 04/17] Gather Let/Assert context in SimplifyCorrelatedDifferences via BoundsTracker 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 Claude-Session: https://claude.ai/code/session_019EGMmdqNC6mTcMSCDBFbwV --- src/BoundsTracker.h | 9 +++++ src/SimplifyCorrelatedDifferences.cpp | 58 +++++++++++++++++++++++---- 2 files changed, 59 insertions(+), 8 deletions(-) diff --git a/src/BoundsTracker.h b/src/BoundsTracker.h index 44c578530c02..a24b4fedc8b4 100644 --- a/src/BoundsTracker.h +++ b/src/BoundsTracker.h @@ -134,6 +134,15 @@ class BoundsTracker { return scope; } + /** The dominating conditions currently known to hold (see push_fact()), + * for passes that want to feed them directly into simplify() as + * assumptions alongside interval_scope(), without paying for the more + * expensive wrap-in-every-pending-let-and-resimplify path that + * find_constant_bound_aggressive()/simplify_with_context() use. */ + const std::vector &known_facts() const { + return facts; + } + private: Scope scope; std::vector> lets; diff --git a/src/SimplifyCorrelatedDifferences.cpp b/src/SimplifyCorrelatedDifferences.cpp index f7d1049ea4f0..31300c2f114c 100644 --- a/src/SimplifyCorrelatedDifferences.cpp +++ b/src/SimplifyCorrelatedDifferences.cpp @@ -1,5 +1,6 @@ #include "SimplifyCorrelatedDifferences.h" +#include "BoundsTracker.h" #include "CSE.h" #include "ExprUsesVar.h" #include "IRMatch.h" @@ -79,19 +80,34 @@ class SimplifyCorrelatedDifferences : public IRMutator { }; vector lets; + // Tracks constant bounds implied by enclosing lets and dominating + // asserts. This is a whole-tree lowering pass (see + // simplify_correlated_differences() below), not just a helper called + // from find_constant_bounds() on one Expr at a time, so it's worth + // gathering this context as we descend: it lets the final simplify() + // in cancel_correlated_subexpression() resolve free variables that + // `lets` above doesn't bother tracking (lets that are pure and + // constant w.r.t. loop_var are deliberately excluded from `lets`, + // since the monotonicity analysis doesn't need them) and lets it use + // dominating assert conditions it otherwise never sees. + BoundsTracker tracker; + template auto visit_let(const LetStmtOrLet *op) -> decltype(op->body) { // Visit an entire chain of lets in a single method to conserve stack space. struct Frame { const LetStmtOrLet *op; ScopedBinding binding; + BoundsTracker::Binding tracker_binding; Expr new_value; - Frame(const LetStmtOrLet *op, const string &loop_var, Scope &scope) + Frame(const LetStmtOrLet *op, const string &loop_var, Scope &scope, BoundsTracker &tracker) : op(op), - binding(scope, op->name, derivative_bounds(op->value, loop_var, scope)) { + binding(scope, op->name, derivative_bounds(op->value, loop_var, scope)), + tracker_binding(tracker.push_let(op->name, op->value)) { } - Frame(const LetStmtOrLet *op) - : op(op) { + Frame(const LetStmtOrLet *op, BoundsTracker &tracker) + : op(op), + tracker_binding(tracker.push_let(op->name, op->value)) { } }; std::vector frames; @@ -111,13 +127,13 @@ class SimplifyCorrelatedDifferences : public IRMutator { do { result = op->body; if (loop_var.empty()) { - frames.emplace_back(op); + frames.emplace_back(op, tracker); continue; } bool pure = is_pure(op->value); if (!pure || expr_uses_vars(op->value, monotonic) || monotonic.contains(op->name)) { - frames.emplace_back(op, loop_var, monotonic); + frames.emplace_back(op, loop_var, monotonic, tracker); Expr new_value = mutate(op->value); bool may_substitute_in = new_value.type() == Int(32) && pure; lets.emplace_back(OuterLet{op->name, new_value, may_substitute_in}); @@ -126,7 +142,7 @@ class SimplifyCorrelatedDifferences : public IRMutator { // Pure and constant w.r.t the loop var. Doesn't // shadow any outer thing already in the monotonic // scope. - frames.emplace_back(op); + frames.emplace_back(op, tracker); } } while ((op = result.template as())); @@ -155,6 +171,7 @@ class SimplifyCorrelatedDifferences : public IRMutator { } Stmt visit(const For *op) override { + auto bounds_bind = tracker.push_for(op->name, op->min, op->max); Stmt s = op; // This is unfortunately quadratic in maximum loop nesting depth if (loop_var.empty()) { @@ -175,6 +192,25 @@ class SimplifyCorrelatedDifferences : public IRMutator { return s; } + // A leading assert in a Block holds for everything after it (Halide + // blocks are left-leaning, so a chain of asserts shows up as nested + // Blocks, each with an AssertStmt as `first`; recursing into `rest` + // naturally walks the whole chain). + Stmt visit(const Block *op) override { + if (const AssertStmt *a = op->first.as()) { + Stmt first = mutate(op->first); + auto fact = tracker.push_fact(a->condition); + Stmt rest = mutate(op->rest); + if (first.same_as(op->first) && rest.same_as(op->rest)) { + return op; + } else { + return Block::make(first, rest); + } + } else { + return IRMutator::visit(op); + } + } + // Add the names of any free variables in an expr to the provided set void track_free_vars(const Expr &e, std::set *vars) { class TrackFreeVars : public IRVisitor { @@ -227,7 +263,13 @@ class SimplifyCorrelatedDifferences : public IRMutator { e = common_subexpression_elimination(e); e = solve_expression(e, loop_var).result; e = PartiallyCancelDifferences()(e); - e = simplify(e); + // Cheaper than find_constant_bound_aggressive()'s + // wrap-every-pending-let-and-resimplify path (this pass is + // already quadratic in loop nesting depth and runs across the + // whole tree several times, so that would be too expensive + // here): just hand the already-computed constant scope and + // dominating facts to the simplifier directly. + e = simplify(e, tracker.interval_scope(), Scope::empty_scope(), tracker.known_facts()); debug(1, "non-monotonic") << [&]() -> std::string { if (is_monotonic(e, loop_var) != Monotonic::Unknown) { From e294bd9b91df3288801f59f78e5736f760048759 Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Wed, 26 Aug 2026 12:05:46 +0200 Subject: [PATCH 05/17] Bound a min-clamped ceil-div minus its unclamped multiple 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. --- src/BoundConstantExtentLoops.cpp | 2 + src/SimplifyCorrelatedDifferences.cpp | 12 +++ test/correctness/CMakeLists.txt | 1 + .../split_predicate_stores_compute_at.cpp | 102 ++++++++++++++++++ 4 files changed, 117 insertions(+) create mode 100644 test/correctness/split_predicate_stores_compute_at.cpp diff --git a/src/BoundConstantExtentLoops.cpp b/src/BoundConstantExtentLoops.cpp index 17d896bf3739..f893e63f9eaa 100644 --- a/src/BoundConstantExtentLoops.cpp +++ b/src/BoundConstantExtentLoops.cpp @@ -39,6 +39,7 @@ class BoundLoops : public IRMutator { } Stmt visit(const For *op) override { + auto bind = tracker.push_for(op->name, op->min, op->max); Expr extent = simplify(op->extent()); if (is_const(extent)) { // Nothing needs to be done @@ -48,6 +49,7 @@ class BoundLoops : public IRMutator { if (op->for_type == ForType::Unrolled || op->for_type == ForType::Vectorized) { // Give it one last chance to simplify to an int + extent = tracker.simplify_with_context(extent); Stmt body = op->body; const IntImm *e = extent.as(); diff --git a/src/SimplifyCorrelatedDifferences.cpp b/src/SimplifyCorrelatedDifferences.cpp index 31300c2f114c..8d545943a6d8 100644 --- a/src/SimplifyCorrelatedDifferences.cpp +++ b/src/SimplifyCorrelatedDifferences.cpp @@ -29,6 +29,7 @@ class PartiallyCancelDifferences : public IRMutator { IRMatcher::Wild<2> z; IRMatcher::WildConst<0> c0; IRMatcher::WildConst<1> c1; + IRMatcher::WildConst<2> c2; Expr visit(const Sub *op) override { @@ -57,6 +58,17 @@ class PartiallyCancelDifferences : public IRMutator { rewrite(min(x + c0, y) - select(z, min(x, y) + c1, x), select(z, (max(min(y - x, c0), 0) - c1), min(y - x, c0)), c0 > 0) || rewrite(min(y, x + c0) - select(z, min(y, x) + c1, x), select(z, (max(min(y - x, c0), 0) - c1), min(y - x, c0)), c0 > 0) || + // A ceiling-divide of a quantity clamped above by an exact + // multiple of the divisor (x*c0 + c1), minus the unclamped + // multiple (x) that the clamp is anchored to. Comes up when + // rounding a min-clamped region's extent up to a multiple of + // c0: (min(x*c0 + c1, y) + c2)/c0 - x/1 == min((y+c2)/c0 - x, + // (c1+c2)/c0), exactly, for any c1, c2 -- not just when they + // 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) || + false) { return rewrite.result; } diff --git a/test/correctness/CMakeLists.txt b/test/correctness/CMakeLists.txt index fa463f242e7e..8dac8e66fc9f 100644 --- a/test/correctness/CMakeLists.txt +++ b/test/correctness/CMakeLists.txt @@ -344,6 +344,7 @@ tests( split_factor_type.cpp split_fuse_rvar.cpp split_predicate.cpp + split_predicate_stores_compute_at.cpp split_reuse_inner_name_bug.cpp split_store_compute.cpp stable_realization_order.cpp diff --git a/test/correctness/split_predicate_stores_compute_at.cpp b/test/correctness/split_predicate_stores_compute_at.cpp new file mode 100644 index 000000000000..883d9e07dfd4 --- /dev/null +++ b/test/correctness/split_predicate_stores_compute_at.cpp @@ -0,0 +1,102 @@ +#include "Halide.h" +#include + +using namespace Halide; +using namespace Halide::Internal; + +namespace { + +// Note: this test is built with NDEBUG, so assert() compiles to nothing. +bool check(bool ok, const char *msg) { + if (!ok) { + printf("Failed: %s\n", msg); + } + return ok; +} + +// Counts IfThenElse nodes reached while inside the named Func's produce node. +class CountIfsInProduce : public IRVisitor { + using IRVisitor::visit; + + std::string name; + int depth = 0; + + void visit(const ProducerConsumer *op) override { + if (op->is_producer && op->name == name) { + depth++; + IRVisitor::visit(op); + depth--; + } else { + IRVisitor::visit(op); + } + } + + void visit(const IfThenElse *op) override { + if (depth > 0) { + count++; + } + IRVisitor::visit(op); + } + +public: + explicit CountIfsInProduce(std::string n) + : name(std::move(n)) { + } + int count = 0; +}; + +} // namespace + +int main(int argc, char **argv) { + // A producer compute_at a plain (non-aligned) split tile of its + // consumer, with the consumer's tail handled by PredicateStores rather + // than GuardWithIf. PredicateStores only predicates the consumer's + // store, not the loads that feed it, so the producer's required region + // for a boundary tile still comes out tied to the consumer's declared + // extent rather than as an unconditional full tile -- bounds inference + // needs to find a compile-time-constant *upper* bound (the split + // factor) for that region's extent to unroll it at all, and fold all + // the per-position validity checks into a single guard around the + // whole tile rather than one nested check per unrolled position. + Var x{"x"}, xo{"xo"}, xi{"xi"}; + Func g{"g"}, f{"f"}; + + g(x) = x * 2; + f(x) = g(x) + 1; + f.output_buffer().dim(0).set_min(0); + + f.split(x, xo, xi, 5, TailStrategy::PredicateStores).never_partition_all(); + g.compute_at(f, xo).align_bounds(x, 5).unroll(x); + + Module m = f.compile_to_module({}); + + CountIfsInProduce checker("g"); + for (const LoweredFunc &lf : m.functions()) { + lf.body.accept(&checker); + } + + // Before the fix, bounds inference couldn't find any bound for the + // tile's extent at all, and unrolling g failed outright. After the + // fix it finds at least an upper bound (the split factor), which + // BoundConstantExtentLoops uses as the unrolled extent, guarded by at + // most one check around the whole tile -- not one check per position. + if (!check(checker.count <= 1, + "expected at most one guard around the whole unrolled tile")) + return 1; + + // Values must still come out right at and past the boundary, for + // several sizes that aren't a multiple of the split factor. + for (int w : {1, 4, 5, 6, 9, 11, 23}) { + Buffer out = f.realize({w}); + for (int i = 0; i < w; i++) { + int expected = i * 2 + 1; + if (out(i) != expected) { + printf("out(%d) = %d instead of %d (w = %d)\n", i, out(i), expected, w); + return 1; + } + } + } + + printf("Success!\n"); + return 0; +} From c1142121baa0ddb94f5adb85420437631d1d8930 Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Wed, 26 Aug 2026 14:27:08 +0200 Subject: [PATCH 06/17] Use loop monotonicity to bound an unrollable tile exactly 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 Claude-Session: https://claude.ai/code/session_018SvYp54SUAXaJ49MizJg63 --- src/BoundsTracker.cpp | 85 +++++++++++++++++-- src/BoundsTracker.h | 25 +++++- .../split_predicate_stores_compute_at.cpp | 15 ++-- 3 files changed, 111 insertions(+), 14 deletions(-) diff --git a/src/BoundsTracker.cpp b/src/BoundsTracker.cpp index 63aca5219588..15a97e0129c3 100644 --- a/src/BoundsTracker.cpp +++ b/src/BoundsTracker.cpp @@ -1,28 +1,38 @@ #include "BoundsTracker.h" +#include "ExprUsesVar.h" #include "IR.h" #include "IROperator.h" +#include "Monotonic.h" #include "Simplify.h" +#include "SimplifyCorrelatedDifferences.h" #include "Substitute.h" namespace Halide { namespace Internal { -BoundsTracker::Binding::Binding(BoundsTracker *tracker, ScopedBinding scope_binding, bool recorded_let) - : tracker(tracker), scope_binding(std::move(scope_binding)), recorded_let(recorded_let) { +BoundsTracker::Binding::Binding(BoundsTracker *tracker, ScopedBinding scope_binding, bool recorded_let, + bool recorded_loop) + : tracker(tracker), scope_binding(std::move(scope_binding)), recorded_let(recorded_let), + recorded_loop(recorded_loop) { } BoundsTracker::Binding::Binding(Binding &&other) noexcept : tracker(other.tracker), scope_binding(std::move(other.scope_binding)), - recorded_let(other.recorded_let) { + recorded_let(other.recorded_let), + recorded_loop(other.recorded_loop) { other.recorded_let = false; + other.recorded_loop = false; } BoundsTracker::Binding::~Binding() { if (recorded_let) { tracker->lets.pop_back(); } + if (recorded_loop) { + tracker->loops.pop_back(); + } } BoundsTracker::Binding BoundsTracker::push_for(const std::string &name, const Expr &min, const Expr &max) { @@ -31,7 +41,17 @@ BoundsTracker::Binding BoundsTracker::push_for(const std::string &name, const Ex Interval b = Interval::make_union(min_bounds, max_bounds); b.min = simplify(b.min); b.max = simplify(b.max); - return Binding(this, ScopedBinding(scope, name, b), false); + + // Also record the range symbolically. The scope above can only hold + // constants, so it drops any relationship between the loop variable and a + // symbol appearing in its min or max (e.g. the tile index of a split being + // bounded by a ceiling-divide of the extent being split). + bool recorded_loop = false; + if (min.type() == Int(32) && max.type() == Int(32) && is_pure(min) && is_pure(max)) { + loops.push_back(LoopRange{name, min, max}); + recorded_loop = true; + } + return Binding(this, ScopedBinding(scope, name, b), false, recorded_loop); } BoundsTracker::Binding BoundsTracker::push_interval(const std::string &name, const Interval &interval) { @@ -89,6 +109,21 @@ Expr BoundsTracker::simplify_with_context(const Expr &e) const { // this expression is no longer being asked to prove an equality. wrapped = Halide::Internal::simplify(wrapped, Scope::empty_scope(), Scope::empty_scope(), facts); + + // Now that the lets are inlined, a dependence on an enclosing loop + // variable may appear on both sides of a subtraction. Cancel those out + // and simplify again -- the facts recorded by push_for can only relate + // the loop variable to the rest of the expression once the expression + // mentions it directly. This can grow the expression, so only keep the + // result if it actually bought us something. + Expr cancelled = bound_correlated_differences(wrapped); + if (!cancelled.same_as(wrapped)) { + cancelled = Halide::Internal::simplify(cancelled, Scope::empty_scope(), + Scope::empty_scope(), facts); + if (is_const(cancelled) || cancelled.node_type() < wrapped.node_type()) { + wrapped = cancelled; + } + } return wrapped; } @@ -101,13 +136,53 @@ Expr BoundsTracker::find_constant_bound_aggressive(const Expr &e, Direction d) c return Halide::Internal::find_constant_bound(wrapped, d, scope); } +Interval BoundsTracker::tighten_using_loop_monotonicity(const Expr &e, Interval interval) const { + if (e.type() != Int(32)) { + return interval; + } + // Innermost first: the tightest correlation is usually with the nearest + // enclosing loop. + for (const LoopRange &loop : reverse_view(loops)) { + if (interval.has_lower_bound() && interval.has_upper_bound()) { + break; + } + if (!expr_uses_var(e, loop.name)) { + continue; + } + Monotonic m = is_monotonic(e, loop.name); + Expr at_lower, at_upper; + if (m == Monotonic::Increasing || m == Monotonic::Constant) { + at_lower = loop.min; + at_upper = loop.max; + } else if (m == Monotonic::Decreasing) { + at_lower = loop.max; + at_upper = loop.min; + } else { + continue; + } + if (!interval.has_lower_bound()) { + Expr lo = simplify(substitute(loop.name, at_lower, e)); + interval.min = Halide::Internal::find_constant_bounds(lo, scope).min; + } + if (!interval.has_upper_bound()) { + Expr hi = simplify(substitute(loop.name, at_upper, e)); + interval.max = Halide::Internal::find_constant_bounds(hi, scope).max; + } + } + return interval; +} + Interval BoundsTracker::find_constant_bounds_aggressive(const Expr &e) const { Interval interval = find_constant_bounds(e); if (interval.has_lower_bound() && interval.has_upper_bound()) { return interval; } Expr wrapped = simplify_with_context(e); - return Halide::Internal::find_constant_bounds(wrapped, scope); + interval = Halide::Internal::find_constant_bounds(wrapped, scope); + if (interval.has_lower_bound() && interval.has_upper_bound()) { + return interval; + } + return tighten_using_loop_monotonicity(wrapped, interval); } } // namespace Internal diff --git a/src/BoundsTracker.h b/src/BoundsTracker.h index a24b4fedc8b4..ac222989ad09 100644 --- a/src/BoundsTracker.h +++ b/src/BoundsTracker.h @@ -46,16 +46,23 @@ class BoundsTracker { private: friend class BoundsTracker; - Binding(BoundsTracker *tracker, ScopedBinding scope_binding, bool recorded_let); + Binding(BoundsTracker *tracker, ScopedBinding scope_binding, bool recorded_let, + bool recorded_loop = false); BoundsTracker *tracker = nullptr; ScopedBinding scope_binding; bool recorded_let = false; + bool recorded_loop = false; }; /** Push the bounds of a for loop variable: the envelope [lower bound of * min, upper bound of max], each resolved against everything pushed so - * far. */ + * far. Additionally records the range symbolically, so that + * find_constant_bounds_aggressive() can substitute the endpoints into an + * expression that turns out to be monotonic in the loop variable. That + * recovers bounds the constants-only scope can't represent, because a + * loop's min or max may itself mention a symbol the expression also + * mentions. */ Binding push_for(const std::string &name, const Expr &min, const Expr &max); /** Push an already-computed Interval directly, bypassing derivation. @@ -144,9 +151,23 @@ class BoundsTracker { } private: + /** Tighten an interval by exploiting monotonicity in an enclosing loop + * variable: if e is monotonic in it, e's extremes over the loop are + * reached at the ends of the loop's range, so substituting the symbolic + * endpoints and constant-bounding the results can succeed where + * per-node interval arithmetic can't, because substitution keeps a + * symbol shared by e and the loop's range correlated. */ + Interval tighten_using_loop_monotonicity(const Expr &e, Interval interval) const; + + struct LoopRange { + std::string name; + Expr min, max; + }; + Scope scope; std::vector> lets; std::vector facts; + std::vector loops; }; } // namespace Internal diff --git a/test/correctness/split_predicate_stores_compute_at.cpp b/test/correctness/split_predicate_stores_compute_at.cpp index 883d9e07dfd4..2cee901c960f 100644 --- a/test/correctness/split_predicate_stores_compute_at.cpp +++ b/test/correctness/split_predicate_stores_compute_at.cpp @@ -75,13 +75,14 @@ int main(int argc, char **argv) { lf.body.accept(&checker); } - // Before the fix, bounds inference couldn't find any bound for the - // tile's extent at all, and unrolling g failed outright. After the - // fix it finds at least an upper bound (the split factor), which - // BoundConstantExtentLoops uses as the unrolled extent, guarded by at - // most one check around the whole tile -- not one check per position. - if (!check(checker.count <= 1, - "expected at most one guard around the whole unrolled tile")) + // The tile's extent is exactly the split factor: the enclosing tile + // loop's own max bounds the consumer's extent from below, so the + // ceiling-divide that rounds the region up to a multiple of the factor + // is exact. BoundConstantExtentLoops must find that as an exact + // constant, not just an upper bound, so the unrolled body needs no + // guard at all. + if (!check(checker.count == 0, + "expected no guard inside the unrolled tile")) return 1; // Values must still come out right at and past the boundary, for From eaad3d92a402adde84924571c66ac68195ac15e4 Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Thu, 27 Aug 2026 12:07:53 +0200 Subject: [PATCH 07/17] Learn loop bounds as facts. --- src/BoundsTracker.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/BoundsTracker.cpp b/src/BoundsTracker.cpp index 15a97e0129c3..f8226d4b9cae 100644 --- a/src/BoundsTracker.cpp +++ b/src/BoundsTracker.cpp @@ -32,6 +32,8 @@ BoundsTracker::Binding::~Binding() { } if (recorded_loop) { tracker->loops.pop_back(); + tracker->facts.pop_back(); + tracker->facts.pop_back(); } } @@ -49,6 +51,9 @@ BoundsTracker::Binding BoundsTracker::push_for(const std::string &name, const Ex bool recorded_loop = false; if (min.type() == Int(32) && max.type() == Int(32) && is_pure(min) && is_pure(max)) { loops.push_back(LoopRange{name, min, max}); + Expr loop_var = Variable::make(Int(32), name); + facts.push_back(loop_var >= min); + facts.push_back(loop_var <= max); recorded_loop = true; } return Binding(this, ScopedBinding(scope, name, b), false, recorded_loop); @@ -107,6 +112,10 @@ Expr BoundsTracker::simplify_with_context(const Expr &e) const { // value an equality fact implies, which weakens the very reasoning this // call exists to do. Any tightening from `scope` happens afterward, once // this expression is no longer being asked to prove an equality. + debug(4) << "Simplify with context: " << wrapped << "\n"; + for (const auto &fact : facts) { + debug(4) << " [fact] " << fact << "\n"; + } wrapped = Halide::Internal::simplify(wrapped, Scope::empty_scope(), Scope::empty_scope(), facts); From b15c3531cc92cfbe1a97e183fe689674a48fec36 Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Thu, 27 Aug 2026 13:25:31 +0200 Subject: [PATCH 08/17] Make find_constant_bound_aggressive a wrapper of find_constant_bounds_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 Claude-Session: https://claude.ai/code/session_01H8YeexA67LffkteXd9ekdy --- src/BoundsTracker.cpp | 18 +++++++++--------- src/BoundsTracker.h | 15 ++++++++++----- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/BoundsTracker.cpp b/src/BoundsTracker.cpp index f8226d4b9cae..dfeda2fd0b38 100644 --- a/src/BoundsTracker.cpp +++ b/src/BoundsTracker.cpp @@ -136,15 +136,6 @@ Expr BoundsTracker::simplify_with_context(const Expr &e) const { return wrapped; } -Expr BoundsTracker::find_constant_bound_aggressive(const Expr &e, Direction d) const { - Expr bound = find_constant_bound(e, d); - if (bound.defined()) { - return bound; - } - Expr wrapped = simplify_with_context(e); - return Halide::Internal::find_constant_bound(wrapped, d, scope); -} - Interval BoundsTracker::tighten_using_loop_monotonicity(const Expr &e, Interval interval) const { if (e.type() != Int(32)) { return interval; @@ -194,5 +185,14 @@ Interval BoundsTracker::find_constant_bounds_aggressive(const Expr &e) const { return tighten_using_loop_monotonicity(wrapped, interval); } +Expr BoundsTracker::find_constant_bound_aggressive(const Expr &e, Direction d) const { + Interval interval = find_constant_bounds_aggressive(e); + if (d == Direction::Lower) { + return interval.has_lower_bound() ? interval.min : Expr(); + } else { + return interval.has_upper_bound() ? interval.max : Expr(); + } +} + } // namespace Internal } // namespace Halide diff --git a/src/BoundsTracker.h b/src/BoundsTracker.h index ac222989ad09..9a5149c6437a 100644 --- a/src/BoundsTracker.h +++ b/src/BoundsTracker.h @@ -110,12 +110,17 @@ class BoundsTracker { /** Fast path first; on failure, wrap e in all pending pure lets * (producing a self-contained copy with no free references to enclosing * lets), inline them with substitute_in_all_lets, and simplify before - * retrying against the resulting expression and the scope. More - * expensive than find_constant_bound(), but succeeds far more often, - * because the simplifier can cancel terms across let boundaries that - * interval arithmetic through opaque variable lookups cannot. */ - Expr find_constant_bound_aggressive(const Expr &e, Direction d) const; + * retrying against the resulting expression and the scope. Any endpoint + * still missing after that is attempted once more by substituting the + * range of an enclosing loop e is monotonic in. More expensive than + * find_constant_bound(), but succeeds far more often, because the + * simplifier can cancel terms across let boundaries that interval + * arithmetic through opaque variable lookups cannot. + * + * The single-Direction form is a wrapper around the Interval form; it + * returns an undefined Expr if no bound in that direction was found. */ Interval find_constant_bounds_aggressive(const Expr &e) const; + Expr find_constant_bound_aggressive(const Expr &e, Direction d) const; /** Wrap e in all pending pure lets, inline them with * substitute_in_all_lets, and simplify under the current scope and From 03129e6cbee4f9af9a2473215e1586ad4c39412c Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Thu, 27 Aug 2026 14:21:47 +0200 Subject: [PATCH 09/17] BoundsTracker in Makefile. --- Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile b/Makefile index c66ac327781f..b69a101ab510 100644 --- a/Makefile +++ b/Makefile @@ -460,6 +460,7 @@ SOURCE_FILES = \ Bounds.cpp \ BoundsInference.cpp \ BoundSmallAllocations.cpp \ + BoundsTracker.cpp \ Buffer.cpp \ Callable.cpp \ CanonicalizeGPUVars.cpp \ @@ -665,6 +666,7 @@ HEADER_FILES = \ Bounds.h \ BoundsInference.h \ BoundSmallAllocations.h \ + BoundsTracker.h \ Buffer.h \ Callable.h \ CanonicalizeGPUVars.h \ From c02a39a1c74acee20883511a4dca2864ed77b413 Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Thu, 27 Aug 2026 14:33:33 +0200 Subject: [PATCH 10/17] Revert using BoundsTracker in AllocationBoundsInference. That's not useful as we're not trying to find any upper bound. --- src/AllocationBoundsInference.cpp | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/src/AllocationBoundsInference.cpp b/src/AllocationBoundsInference.cpp index 187aad4f1f6f..9aaaf1c7e661 100644 --- a/src/AllocationBoundsInference.cpp +++ b/src/AllocationBoundsInference.cpp @@ -1,6 +1,5 @@ #include "AllocationBoundsInference.h" #include "Bounds.h" -#include "BoundsTracker.h" #include "CSE.h" #include "ExternFuncArgument.h" #include "Function.h" @@ -20,6 +19,10 @@ using std::vector; namespace { +Expr cse_and_simplify(const Expr &x) { + return simplify(common_subexpression_elimination(x)); +} + // Figure out the region touched of each buffer, and deposit them as // let statements outside of each realize node, or at the top level if // they're not internal allocations. @@ -31,25 +34,6 @@ class AllocationInference : public IRMutator { const FuncValueBounds &func_bounds; set touched_by_extern; - // Tracks the enclosing pure lets and for loops, so that box_touched's - // result -- computed with no knowledge of anything outside op->body -- - // can be simplified as thoroughly as if it had been. - BoundsTracker tracker; - - Expr cse_and_simplify(const Expr &x) { - return simplify(common_subexpression_elimination(tracker.simplify_with_context(x))); - } - - Stmt visit(const LetStmt *op) override { - auto binding = tracker.push_let(op->name, op->value); - return IRMutator::visit(op); - } - - Stmt visit(const For *op) override { - auto binding = tracker.push_for(op->name, op->min, op->max); - return IRMutator::visit(op); - } - Stmt visit(const Realize *op) override { map::const_iterator iter = env.find(op->name); internal_assert(iter != env.end()); From 60c0cd66a0fa626626277414a88d95f9caa0a788 Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Wed, 9 Sep 2026 15:01:17 +0200 Subject: [PATCH 11/17] Only derive bounds context in BoundsTracker when something asks for it 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 Claude-Session: https://claude.ai/code/session_01B8pMM1zmBL89nNwzwCEJM4 --- src/BoundsTracker.cpp | 265 +++++++++++++++++++------- src/BoundsTracker.h | 106 ++++++++--- src/SimplifyCorrelatedDifferences.cpp | 2 +- 3 files changed, 279 insertions(+), 94 deletions(-) diff --git a/src/BoundsTracker.cpp b/src/BoundsTracker.cpp index dfeda2fd0b38..86d0b534b55e 100644 --- a/src/BoundsTracker.cpp +++ b/src/BoundsTracker.cpp @@ -3,72 +3,71 @@ #include "ExprUsesVar.h" #include "IR.h" #include "IROperator.h" +#include "IRVisitor.h" #include "Monotonic.h" #include "Simplify.h" #include "SimplifyCorrelatedDifferences.h" #include "Substitute.h" +#include "Util.h" namespace Halide { namespace Internal { -BoundsTracker::Binding::Binding(BoundsTracker *tracker, ScopedBinding scope_binding, bool recorded_let, - bool recorded_loop) - : tracker(tracker), scope_binding(std::move(scope_binding)), recorded_let(recorded_let), - recorded_loop(recorded_loop) { -} +namespace { -BoundsTracker::Binding::Binding(Binding &&other) noexcept - : tracker(other.tracker), - scope_binding(std::move(other.scope_binding)), - recorded_let(other.recorded_let), - recorded_loop(other.recorded_loop) { - other.recorded_let = false; - other.recorded_loop = false; -} +/** Every name an Expr refers to, whether as a variable or as a buffer. */ +class CollectUsedNames : public IRGraphVisitor { + using IRGraphVisitor::visit; -BoundsTracker::Binding::~Binding() { - if (recorded_let) { - tracker->lets.pop_back(); + void visit(const Variable *op) override { + names->insert(op->name); + } + + void visit(const Load *op) override { + names->insert(op->name); + IRGraphVisitor::visit(op); + } + + void visit(const Store *op) override { + names->insert(op->name); + IRGraphVisitor::visit(op); } - if (recorded_loop) { - tracker->loops.pop_back(); - tracker->facts.pop_back(); - tracker->facts.pop_back(); + +public: + std::set *names; + + explicit CollectUsedNames(std::set *names) + : names(names) { + } +}; + +bool mentions_any(const Expr &e, const std::set &names) { + std::set used; + CollectUsedNames collect(&used); + e.accept(&collect); + for (const std::string &name : used) { + if (names.count(name)) { + return true; + } } + return false; } -BoundsTracker::Binding BoundsTracker::push_for(const std::string &name, const Expr &min, const Expr &max) { - Interval min_bounds = find_constant_bounds(min); - Interval max_bounds = find_constant_bounds(max); - Interval b = Interval::make_union(min_bounds, max_bounds); - b.min = simplify(b.min); - b.max = simplify(b.max); - - // Also record the range symbolically. The scope above can only hold - // constants, so it drops any relationship between the loop variable and a - // symbol appearing in its min or max (e.g. the tile index of a split being - // bounded by a ceiling-divide of the extent being split). - bool recorded_loop = false; - if (min.type() == Int(32) && max.type() == Int(32) && is_pure(min) && is_pure(max)) { - loops.push_back(LoopRange{name, min, max}); - Expr loop_var = Variable::make(Int(32), name); - facts.push_back(loop_var >= min); - facts.push_back(loop_var <= max); - recorded_loop = true; - } - return Binding(this, ScopedBinding(scope, name, b), false, recorded_loop); +} // namespace + +BoundsTracker::Binding::Binding(BoundsTracker *tracker) + : tracker(tracker) { } -BoundsTracker::Binding BoundsTracker::push_interval(const std::string &name, const Interval &interval) { - return Binding(this, ScopedBinding(scope, name, interval), false); +BoundsTracker::Binding::Binding(Binding &&other) noexcept + : tracker(other.tracker) { + other.tracker = nullptr; } -BoundsTracker::Binding BoundsTracker::push_let(const std::string &name, const Expr &value) { - bool pure = is_pure(value); - if (pure) { - lets.emplace_back(name, value); +BoundsTracker::Binding::~Binding() { + if (tracker) { + tracker->pop_entry(); } - return Binding(this, ScopedBinding(scope, name, find_constant_bounds(value)), pure); } BoundsTracker::FactGuard::FactGuard(BoundsTracker *tracker) @@ -82,30 +81,161 @@ BoundsTracker::FactGuard::FactGuard(FactGuard &&other) noexcept BoundsTracker::FactGuard::~FactGuard() { if (tracker) { - tracker->facts.pop_back(); + tracker->pop_fact(); } } +BoundsTracker::Binding BoundsTracker::push_for(const std::string &name, const Expr &min, const Expr &max) { + entries.push_back(Entry{Kind::Loop, name, min, max}); + return Binding(this); +} + +BoundsTracker::Binding BoundsTracker::push_interval(const std::string &name, const Interval &interval) { + entries.push_back(Entry{Kind::Explicit, name, Expr(), Expr(), interval}); + return Binding(this); +} + +BoundsTracker::Binding BoundsTracker::push_let(const std::string &name, const Expr &value) { + entries.push_back(Entry{Kind::Let, name, value}); + return Binding(this); +} + BoundsTracker::FactGuard BoundsTracker::push_fact(const Expr &condition) { facts.push_back(condition); return FactGuard(this); } +void BoundsTracker::pop_entry() { + internal_assert(!entries.empty()); + if (realized == entries.size()) { + scope.pop(entries.back().name); + realized--; + } + entries.pop_back(); +} + +void BoundsTracker::pop_fact() { + internal_assert(!facts.empty()); + facts.pop_back(); +} + +bool BoundsTracker::entry_is_pure(const Entry &entry) const { + if (entry.purity == Entry::Purity::Unknown) { + bool pure = (!entry.lo.defined() || is_pure(entry.lo)) && + (!entry.hi.defined() || is_pure(entry.hi)); + entry.purity = pure ? Entry::Purity::Pure : Entry::Purity::Impure; + } + return entry.purity == Entry::Purity::Pure; +} + +Interval BoundsTracker::bounds_of(const Entry &entry) const { + switch (entry.kind) { + case Kind::Let: + return Halide::Internal::find_constant_bounds(entry.lo, scope); + case Kind::Loop: { + Interval b = Interval::make_union(Halide::Internal::find_constant_bounds(entry.lo, scope), + Halide::Internal::find_constant_bounds(entry.hi, scope)); + b.min = simplify(b.min); + b.max = simplify(b.max); + return b; + } + case Kind::Explicit: + return entry.bounds; + } + return Interval::everything(); +} + +void BoundsTracker::realize_all() const { + // Outermost first: each entry's bounds may depend on the ones enclosing + // it, and pushing in this order also keeps the scope's shadowing of + // repeated names matching the tree. + for (; realized < entries.size(); realized++) { + Entry &entry = entries[realized]; + entry.bounds = bounds_of(entry); + scope.push(entry.name, entry.bounds); + } +} + +const Scope &BoundsTracker::interval_scope() const { + realize_all(); + return scope; +} + +Expr BoundsTracker::wrap_in_used_lets(const Expr &e, std::set *used) const { + CollectUsedNames collect(used); + e.accept(&collect); + Expr result = e; + // Innermost first, so that a let picked up along the way can still pull in + // the outer lets its value refers to. Conservatively treats every name the + // body mentions as a possible reference to an enclosing let, even where an + // inner let shadows it. + for (const Entry &entry : reverse_view(entries)) { + if (entry.kind == Kind::Let && used->count(entry.name) && entry_is_pure(entry)) { + entry.lo.accept(&collect); + result = Let::make(entry.name, entry.lo, result); + } + } + return result; +} + +std::vector BoundsTracker::facts_mentioning(std::set *used) const { + std::vector result; + std::vector candidates = facts; + for (const Entry &entry : entries) { + // The scope can only hold constants, so it drops any relationship + // between a loop variable and a symbol appearing in its min or max + // (e.g. the tile index of a split being bounded by a ceiling-divide of + // the extent being split). Record the range symbolically as well. + if (entry.kind == Kind::Loop && + entry.lo.type() == Int(32) && entry.hi.type() == Int(32) && + entry_is_pure(entry)) { + Expr loop_var = Variable::make(Int(32), entry.name); + candidates.push_back(loop_var >= entry.lo); + candidates.push_back(loop_var <= entry.hi); + } + } + + // A fact that shares no variable with what we're asking about can't say + // anything about it, but a fact it does share one with can bring in + // variables that make a third fact relevant, so keep sweeping until + // nothing new is picked up. + bool changed = true; + while (changed) { + changed = false; + for (Expr &candidate : candidates) { + if (candidate.defined() && mentions_any(candidate, *used)) { + CollectUsedNames collect(used); + candidate.accept(&collect); + result.push_back(candidate); + candidate = Expr(); + changed = true; + } + } + } + return result; +} + +std::vector BoundsTracker::relevant_facts(const Expr &e) const { + std::set used; + CollectUsedNames collect(&used); + e.accept(&collect); + return facts_mentioning(&used); +} + Expr BoundsTracker::find_constant_bound(const Expr &e, Direction d) const { - return Halide::Internal::find_constant_bound(e, d, scope); + return Halide::Internal::find_constant_bound(e, d, interval_scope()); } Interval BoundsTracker::find_constant_bounds(const Expr &e) const { - return Halide::Internal::find_constant_bounds(e, scope); + return Halide::Internal::find_constant_bounds(e, interval_scope()); } Expr BoundsTracker::simplify_with_context(const Expr &e) const { - Expr wrapped = e; - for (const auto &[name, value] : reverse_view(lets)) { - wrapped = Let::make(name, value, wrapped); - } + std::set used; + Expr wrapped = wrap_in_used_lets(e, &used); wrapped = remove_likelies(wrapped); wrapped = substitute_in_all_lets(wrapped); + std::vector relevant = facts_mentioning(&used); // Deliberately pass an empty bounds scope here, not `scope`: mixing a // bounds scope with equality facts can make the simplifier represent a // variable by its (wide) interval instead of substituting the exact @@ -113,11 +243,11 @@ Expr BoundsTracker::simplify_with_context(const Expr &e) const { // call exists to do. Any tightening from `scope` happens afterward, once // this expression is no longer being asked to prove an equality. debug(4) << "Simplify with context: " << wrapped << "\n"; - for (const auto &fact : facts) { + for (const Expr &fact : relevant) { debug(4) << " [fact] " << fact << "\n"; } wrapped = Halide::Internal::simplify(wrapped, Scope::empty_scope(), - Scope::empty_scope(), facts); + Scope::empty_scope(), relevant); // Now that the lets are inlined, a dependence on an enclosing loop // variable may appear on both sides of a subtraction. Cancel those out @@ -128,7 +258,7 @@ Expr BoundsTracker::simplify_with_context(const Expr &e) const { Expr cancelled = bound_correlated_differences(wrapped); if (!cancelled.same_as(wrapped)) { cancelled = Halide::Internal::simplify(cancelled, Scope::empty_scope(), - Scope::empty_scope(), facts); + Scope::empty_scope(), relevant); if (is_const(cancelled) || cancelled.node_type() < wrapped.node_type()) { wrapped = cancelled; } @@ -142,30 +272,33 @@ Interval BoundsTracker::tighten_using_loop_monotonicity(const Expr &e, Interval } // Innermost first: the tightest correlation is usually with the nearest // enclosing loop. - for (const LoopRange &loop : reverse_view(loops)) { + for (const Entry &entry : reverse_view(entries)) { if (interval.has_lower_bound() && interval.has_upper_bound()) { break; } - if (!expr_uses_var(e, loop.name)) { + if (entry.kind != Kind::Loop || + entry.lo.type() != Int(32) || entry.hi.type() != Int(32) || + !entry_is_pure(entry) || + !expr_uses_var(e, entry.name)) { continue; } - Monotonic m = is_monotonic(e, loop.name); + Monotonic m = is_monotonic(e, entry.name); Expr at_lower, at_upper; if (m == Monotonic::Increasing || m == Monotonic::Constant) { - at_lower = loop.min; - at_upper = loop.max; + at_lower = entry.lo; + at_upper = entry.hi; } else if (m == Monotonic::Decreasing) { - at_lower = loop.max; - at_upper = loop.min; + at_lower = entry.hi; + at_upper = entry.lo; } else { continue; } if (!interval.has_lower_bound()) { - Expr lo = simplify(substitute(loop.name, at_lower, e)); + Expr lo = simplify(substitute(entry.name, at_lower, e)); interval.min = Halide::Internal::find_constant_bounds(lo, scope).min; } if (!interval.has_upper_bound()) { - Expr hi = simplify(substitute(loop.name, at_upper, e)); + Expr hi = simplify(substitute(entry.name, at_upper, e)); interval.max = Halide::Internal::find_constant_bounds(hi, scope).max; } } diff --git a/src/BoundsTracker.h b/src/BoundsTracker.h index 9a5149c6437a..a93894b98cb1 100644 --- a/src/BoundsTracker.h +++ b/src/BoundsTracker.h @@ -6,8 +6,8 @@ * a Stmt tree. */ +#include #include -#include #include #include "Bounds.h" @@ -31,11 +31,20 @@ namespace Internal { * has always used to find constant loop extents, generalized so other * passes that infer constant bounds (e.g. BoundSmallAllocations, * AllocationBoundsInference) can use it too. + * + * Pushing a binding is deliberately cheap: it only records the Expr. None of + * the derived information -- the constant bounds of a let's value, whether + * it's pure, the symbolic facts implied by a loop's range -- is computed + * until something actually asks a question. Passes that walk a whole tree + * but only query at a handful of nodes therefore pay almost nothing for the + * bindings they descend through. Once computed, the derived information is + * memoized for as long as the binding is alive, so repeated queries under + * the same bindings don't redo the work. */ class BoundsTracker { public: - /** An RAII binding produced by push_for/push_let. Pops everything it - * pushed when destroyed. */ + /** An RAII binding produced by push_for/push_let/push_interval. Pops + * what it pushed when destroyed. */ class Binding { public: Binding() = default; @@ -46,13 +55,9 @@ class BoundsTracker { private: friend class BoundsTracker; - Binding(BoundsTracker *tracker, ScopedBinding scope_binding, bool recorded_let, - bool recorded_loop = false); + explicit Binding(BoundsTracker *tracker); BoundsTracker *tracker = nullptr; - ScopedBinding scope_binding; - bool recorded_let = false; - bool recorded_loop = false; }; /** Push the bounds of a for loop variable: the envelope [lower bound of @@ -72,11 +77,11 @@ class BoundsTracker { * find_constant_bound_aggressive()'s let-substitution. */ Binding push_interval(const std::string &name, const Interval &interval); - /** Push a let binding. Always updates the fast-path scope with a - * constant-bounds estimate of the value. Additionally records the - * syntactic binding for find_constant_bound_aggressive()'s slow path, - * but only if the value is pure -- substituting an impure expression - * into multiple places would change its meaning. */ + /** Push a let binding. Feeds both the fast-path scope (via a constant + * bounds estimate of the value) and find_constant_bound_aggressive()'s + * slow path (via the syntactic binding, but only if the value turns out + * to be pure -- substituting an impure expression into multiple places + * would change its meaning). Both of those are derived lazily. */ Binding push_let(const std::string &name, const Expr &value); /** An RAII guard produced by push_fact. Pops the fact when destroyed. */ @@ -141,19 +146,21 @@ class BoundsTracker { * at ingestion), so this scope -- itself always constant-or-unbounded -- * loses nothing for that use case. It is not, however, suitable for * general symbolic interval arithmetic (e.g. bounds_of_expr_in_scope) - * where a non-constant symbolic bound would otherwise be useful. */ - const Scope &interval_scope() const { - return scope; - } + * where a non-constant symbolic bound would otherwise be useful. + * + * Asking for the scope forces every pending binding to be evaluated, so + * prefer find_constant_bound() where it suffices. */ + const Scope &interval_scope() const; - /** The dominating conditions currently known to hold (see push_fact()), + /** The dominating conditions that could possibly say something about e -- + * those passed to push_fact(), plus the range of every enclosing loop -- * for passes that want to feed them directly into simplify() as * assumptions alongside interval_scope(), without paying for the more * expensive wrap-in-every-pending-let-and-resimplify path that - * find_constant_bound_aggressive()/simplify_with_context() use. */ - const std::vector &known_facts() const { - return facts; - } + * find_constant_bound_aggressive()/simplify_with_context() use. Facts + * sharing no variable (even transitively, via another fact) with e are + * dropped: they can't help, and the simplifier pays to ingest each one. */ + std::vector relevant_facts(const Expr &e) const; private: /** Tighten an interval by exploiting monotonicity in an enclosing loop @@ -164,15 +171,60 @@ class BoundsTracker { * symbol shared by e and the loop's range correlated. */ Interval tighten_using_loop_monotonicity(const Expr &e, Interval interval) const; - struct LoopRange { + /** Wrap e in the enclosing pure lets it refers to, innermost first, + * skipping those it can't reach. Returns the names it ended up + * mentioning in *used, so a caller can go on to pick out the facts that + * could say something about it. */ + Expr wrap_in_used_lets(const Expr &e, std::set *used) const; + + /** The facts mentioning any name in *used, growing *used with the names + * each selected fact brings in, until it reaches a fixed point. */ + std::vector facts_mentioning(std::set *used) const; + + enum class Kind { + Let, //< lo is the value + Loop, //< lo and hi are the loop min and max + Explicit //< bounds is given up front + }; + + struct Entry { + Kind kind; std::string name; - Expr min, max; + Expr lo, hi; + /** Only meaningful once this entry has been realized into `scope`, + * i.e. once its index is below `realized`. */ + Interval bounds; + /** Whether the bound Exprs are pure, and so safe to substitute into + * multiple places. Computed on first use. */ + enum class Purity { + Unknown, + Pure, + Impure + }; + mutable Purity purity = Purity::Unknown; }; - Scope scope; - std::vector> lets; + /** Realize every entry not yet reflected in `scope`, outermost first, so + * that each one's bounds are derived in the scope of the entries that + * enclose it. */ + void realize_all() const; + Interval bounds_of(const Entry &entry) const; + bool entry_is_pure(const Entry &entry) const; + void pop_entry(); + void pop_fact(); + + /** The bindings in scope, outermost first. Mutable because realizing an + * entry -- a pure memoization of what the entry already implies -- has + * to be possible from the const query methods. */ + mutable std::vector entries; + + /** entries[0, realized) have been pushed into `scope`, in that order. */ + mutable size_t realized = 0; + mutable Scope scope; + + /** The conditions passed to push_fact(). Those implied by enclosing + * loops are materialized on demand by relevant_facts(). */ std::vector facts; - std::vector loops; }; } // namespace Internal diff --git a/src/SimplifyCorrelatedDifferences.cpp b/src/SimplifyCorrelatedDifferences.cpp index 8d545943a6d8..564428513c45 100644 --- a/src/SimplifyCorrelatedDifferences.cpp +++ b/src/SimplifyCorrelatedDifferences.cpp @@ -281,7 +281,7 @@ class SimplifyCorrelatedDifferences : public IRMutator { // whole tree several times, so that would be too expensive // here): just hand the already-computed constant scope and // dominating facts to the simplifier directly. - e = simplify(e, tracker.interval_scope(), Scope::empty_scope(), tracker.known_facts()); + e = simplify(e, tracker.interval_scope(), Scope::empty_scope(), tracker.relevant_facts(e)); debug(1, "non-monotonic") << [&]() -> std::string { if (is_monotonic(e, loop_var) != Monotonic::Unknown) { From 0358fa36847cfc080f2bfe8147856d4be4b66636 Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Wed, 9 Sep 2026 16:14:04 +0200 Subject: [PATCH 12/17] Don't bound a pure let's value; bound the expression it's inlined into 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 Claude-Session: https://claude.ai/code/session_01B8pMM1zmBL89nNwzwCEJM4 --- src/BoundsTracker.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/BoundsTracker.cpp b/src/BoundsTracker.cpp index 86d0b534b55e..35e7fede7cf9 100644 --- a/src/BoundsTracker.cpp +++ b/src/BoundsTracker.cpp @@ -131,6 +131,16 @@ bool BoundsTracker::entry_is_pure(const Entry &entry) const { Interval BoundsTracker::bounds_of(const Entry &entry) const { switch (entry.kind) { case Kind::Let: + // A pure let gets inlined by wrap_in_used_lets before anything asks + // for a bound, and bounding the expression that results is both + // cheaper and sharper than bounding the value here and looking it up + // through an opaque variable. It still has to occupy a slot in the + // scope, though: a let shadowing an outer variable that does have a + // bound must hide it, not let lookups fall through to it. An impure + // let is never inlined, so it needs a real bound. + if (entry_is_pure(entry)) { + return Interval::everything(); + } return Halide::Internal::find_constant_bounds(entry.lo, scope); case Kind::Loop: { Interval b = Interval::make_union(Halide::Internal::find_constant_bounds(entry.lo, scope), From 351be9bfa6cc96606f18f24478d2a98fadaec7b9 Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Fri, 11 Sep 2026 17:46:31 +0200 Subject: [PATCH 13/17] Let simplify() choose which lets to inline in simplify_with_context 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 Claude-Session: https://claude.ai/code/session_01B8pMM1zmBL89nNwzwCEJM4 --- src/BoundsTracker.cpp | 19 ++++++++++++------- src/BoundsTracker.h | 33 ++++++++++++++++----------------- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/BoundsTracker.cpp b/src/BoundsTracker.cpp index 35e7fede7cf9..28e9d1fea97a 100644 --- a/src/BoundsTracker.cpp +++ b/src/BoundsTracker.cpp @@ -244,7 +244,12 @@ Expr BoundsTracker::simplify_with_context(const Expr &e) const { std::set used; Expr wrapped = wrap_in_used_lets(e, &used); wrapped = remove_likelies(wrapped); - wrapped = substitute_in_all_lets(wrapped); + // Leave the lets standing and let simplify() pick the ones worth + // inlining. Inlining them all up front repeats each value at every use, + // which costs nothing in the resulting graph but a great deal in the tree + // the simplifier actually walks, since it does not memoize on the shared + // nodes -- a few hundred nodes of graph can become tens of thousands of + // nodes of walk. std::vector relevant = facts_mentioning(&used); // Deliberately pass an empty bounds scope here, not `scope`: mixing a // bounds scope with equality facts can make the simplifier represent a @@ -259,12 +264,12 @@ Expr BoundsTracker::simplify_with_context(const Expr &e) const { wrapped = Halide::Internal::simplify(wrapped, Scope::empty_scope(), Scope::empty_scope(), relevant); - // Now that the lets are inlined, a dependence on an enclosing loop - // variable may appear on both sides of a subtraction. Cancel those out - // and simplify again -- the facts recorded by push_for can only relate - // the loop variable to the rest of the expression once the expression - // mentions it directly. This can grow the expression, so only keep the - // result if it actually bought us something. + // A dependence on an enclosing loop variable may now appear on both sides + // of a subtraction. Cancel those out and simplify again -- the facts + // recorded by push_for can only relate the loop variable to the rest of + // the expression once the expression mentions it directly. This can grow + // the expression, so only keep the result if it actually bought us + // something. Expr cancelled = bound_correlated_differences(wrapped); if (!cancelled.same_as(wrapped)) { cancelled = Halide::Internal::simplify(cancelled, Scope::empty_scope(), diff --git a/src/BoundsTracker.h b/src/BoundsTracker.h index a93894b98cb1..e7720436b937 100644 --- a/src/BoundsTracker.h +++ b/src/BoundsTracker.h @@ -26,11 +26,10 @@ namespace Internal { * In addition to the usual scope-based lookup (cheap, but only sees a bound * if every intermediate variable it passes through was itself pushed with an * already-constant bound), find_constant_bound_aggressive() falls back to - * literally wrapping an expression in all enclosing pure lets, inlining - * them, and re-simplifying. This is the trick bound_constant_extent_loops - * has always used to find constant loop extents, generalized so other - * passes that infer constant bounds (e.g. BoundSmallAllocations, - * AllocationBoundsInference) can use it too. + * literally wrapping an expression in the enclosing pure lets it refers to + * and re-simplifying. This is the trick bound_constant_extent_loops uses to + * find constant loop extents, generalized so other passes that infer + * constant bounds (e.g. BoundSmallAllocations) can use it too. * * Pushing a binding is deliberately cheap: it only records the Expr. None of * the derived information -- the constant bounds of a let's value, whether @@ -112,24 +111,24 @@ class BoundsTracker { Expr find_constant_bound(const Expr &e, Direction d) const; Interval find_constant_bounds(const Expr &e) const; - /** Fast path first; on failure, wrap e in all pending pure lets - * (producing a self-contained copy with no free references to enclosing - * lets), inline them with substitute_in_all_lets, and simplify before - * retrying against the resulting expression and the scope. Any endpoint - * still missing after that is attempted once more by substituting the - * range of an enclosing loop e is monotonic in. More expensive than - * find_constant_bound(), but succeeds far more often, because the - * simplifier can cancel terms across let boundaries that interval - * arithmetic through opaque variable lookups cannot. + /** Fast path first; on failure, wrap e in the pending pure lets it refers + * to (producing a self-contained copy with no free references to + * enclosing lets) and simplify before retrying against the resulting + * expression and the scope. Any endpoint still missing after that is + * attempted once more by substituting the range of an enclosing loop e is + * monotonic in. More expensive than find_constant_bound(), but succeeds + * far more often, because the simplifier can cancel terms across let + * boundaries that interval arithmetic through opaque variable lookups + * cannot. * * The single-Direction form is a wrapper around the Interval form; it * returns an undefined Expr if no bound in that direction was found. */ Interval find_constant_bounds_aggressive(const Expr &e) const; Expr find_constant_bound_aggressive(const Expr &e, Direction d) const; - /** Wrap e in all pending pure lets, inline them with - * substitute_in_all_lets, and simplify under the current scope and - * dominating facts. Unlike find_constant_bound_aggressive(), the result + /** Wrap e in the pending pure lets it refers to and simplify under the + * current scope and dominating facts. Unlike + * find_constant_bound_aggressive(), the result * need not be a constant -- this is just a plain simplify() call that * can see context (enclosing let values, dominating conditions) that a * caller holding only a bare Expr has no way to pass in. Useful for From d63cc4674c18334c05078c92e712b3b2cb8a5441 Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Fri, 11 Sep 2026 19:32:21 +0200 Subject: [PATCH 14/17] Pick relevant facts by variable hash rather than by name 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 Claude-Session: https://claude.ai/code/session_01B8pMM1zmBL89nNwzwCEJM4 --- src/BoundsTracker.cpp | 164 +++++++++++++++++++++++++++++------------- src/BoundsTracker.h | 101 +++++++++++++++++++++++--- 2 files changed, 207 insertions(+), 58 deletions(-) diff --git a/src/BoundsTracker.cpp b/src/BoundsTracker.cpp index 28e9d1fea97a..fb06e4069af2 100644 --- a/src/BoundsTracker.cpp +++ b/src/BoundsTracker.cpp @@ -41,17 +41,21 @@ class CollectUsedNames : public IRGraphVisitor { } }; -bool mentions_any(const Expr &e, const std::set &names) { - std::set used; - CollectUsedNames collect(&used); - e.accept(&collect); - for (const std::string &name : used) { - if (names.count(name)) { - return true; - } +/** Every variable an Expr mentions, as the hash a Variable node carries. */ +class CollectVarHashes : public IRGraphVisitor { + using IRGraphVisitor::visit; + + void visit(const Variable *op) override { + out->push_back(op->hash); } - return false; -} + +public: + std::vector *out; + + explicit CollectVarHashes(std::vector *out) + : out(out) { + } +}; } // namespace @@ -87,6 +91,7 @@ BoundsTracker::FactGuard::~FactGuard() { BoundsTracker::Binding BoundsTracker::push_for(const std::string &name, const Expr &min, const Expr &max) { entries.push_back(Entry{Kind::Loop, name, min, max}); + facts_version++; return Binding(this); } @@ -101,7 +106,8 @@ BoundsTracker::Binding BoundsTracker::push_let(const std::string &name, const Ex } BoundsTracker::FactGuard BoundsTracker::push_fact(const Expr &condition) { - facts.push_back(condition); + facts.push_back(Fact{condition}); + facts_version++; return FactGuard(this); } @@ -111,12 +117,16 @@ void BoundsTracker::pop_entry() { scope.pop(entries.back().name); realized--; } + if (entries.back().kind == Kind::Loop) { + facts_version++; + } entries.pop_back(); } void BoundsTracker::pop_fact() { internal_assert(!facts.empty()); facts.pop_back(); + facts_version++; } bool BoundsTracker::entry_is_pure(const Entry &entry) const { @@ -171,8 +181,9 @@ const Scope &BoundsTracker::interval_scope() const { return scope; } -Expr BoundsTracker::wrap_in_used_lets(const Expr &e, std::set *used) const { - CollectUsedNames collect(used); +Expr BoundsTracker::wrap_in_used_lets(const Expr &e) const { + std::set used; + CollectUsedNames collect(&used); e.accept(&collect); Expr result = e; // Innermost first, so that a let picked up along the way can still pull in @@ -180,7 +191,7 @@ Expr BoundsTracker::wrap_in_used_lets(const Expr &e, std::set *used // body mentions as a possible reference to an enclosing let, even where an // inner let shadows it. for (const Entry &entry : reverse_view(entries)) { - if (entry.kind == Kind::Let && used->count(entry.name) && entry_is_pure(entry)) { + if (entry.kind == Kind::Let && used.count(entry.name) && entry_is_pure(entry)) { entry.lo.accept(&collect); result = Let::make(entry.name, entry.lo, result); } @@ -188,50 +199,108 @@ Expr BoundsTracker::wrap_in_used_lets(const Expr &e, std::set *used return result; } -std::vector BoundsTracker::facts_mentioning(std::set *used) const { - std::vector result; - std::vector candidates = facts; - for (const Entry &entry : entries) { - // The scope can only hold constants, so it drops any relationship - // between a loop variable and a symbol appearing in its min or max - // (e.g. the tile index of a split being bounded by a ceiling-divide of - // the extent being split). Record the range symbolically as well. - if (entry.kind == Kind::Loop && - entry.lo.type() == Int(32) && entry.hi.type() == Int(32) && - entry_is_pure(entry)) { +BoundsTracker::VarSet BoundsTracker::vars_of(const Expr &e) { + VarSet result; + CollectVarHashes collect(&result.hashes); + e.accept(&collect); + for (uint32_t h : result.hashes) { + result.bloom |= (uint64_t)1 << (h & 63); + } + result.sort_unique(); + return result; +} + +const Expr &BoundsTracker::candidate_expr(const Candidate &c) const { + const Entry &entry = entries[c.entry]; + return c.use_hi ? entry.fact_hi : entry.fact_lo; +} + +const BoundsTracker::VarSet &BoundsTracker::candidate_vars(const Candidate &c) const { + return entries[c.entry].fact_vars; +} + +const std::vector &BoundsTracker::candidates() const { + if (candidates_version == facts_version) { + return all_candidates; + } + all_candidates.clear(); + for (size_t i = 0; i < entries.size(); i++) { + const Entry &entry = entries[i]; + if (entry.kind != Kind::Loop || + entry.lo.type() != Int(32) || entry.hi.type() != Int(32) || + !entry_is_pure(entry)) { + continue; + } + if (!entry.facts_built) { + // The scope can only hold constants, so it drops any + // relationship between a loop variable and a symbol appearing in + // its min or max (e.g. the tile index of a split being bounded + // by a ceiling-divide of the extent being split). Record the + // range symbolically too. One variable summary covers both ends: + // over-estimating what a condition mentions only costs an extra + // condition handed to the simplifier. Expr loop_var = Variable::make(Int(32), entry.name); - candidates.push_back(loop_var >= entry.lo); - candidates.push_back(loop_var <= entry.hi); + entry.fact_lo = loop_var >= entry.lo; + entry.fact_hi = loop_var <= entry.hi; + entry.fact_vars = vars_of(entry.fact_lo); + entry.fact_vars.merge(vars_of(entry.fact_hi)); + entry.facts_built = true; } + all_candidates.push_back(Candidate{i, false}); + all_candidates.push_back(Candidate{i, true}); + } + candidates_version = facts_version; + return all_candidates; +} + +std::vector BoundsTracker::relevant_facts(const Expr &e) const { + std::vector result; + const std::vector &cands = candidates(); + if (cands.empty() && facts.empty()) { + return result; } - // A fact that shares no variable with what we're asking about can't say - // anything about it, but a fact it does share one with can bring in - // variables that make a third fact relevant, so keep sweeping until - // nothing new is picked up. + VarSet used = vars_of(e); + + // A condition sharing no variable with what we're asking about can't say + // anything about it, but one that does can bring in variables that make + // a third condition relevant, so keep sweeping until nothing new is + // picked up. + std::vector taken_fact(facts.size(), false); + std::vector taken(cands.size(), false); bool changed = true; while (changed) { changed = false; - for (Expr &candidate : candidates) { - if (candidate.defined() && mentions_any(candidate, *used)) { - CollectUsedNames collect(used); - candidate.accept(&collect); - result.push_back(candidate); - candidate = Expr(); - changed = true; + for (size_t i = 0; i < facts.size(); i++) { + const Fact &fact = facts[i]; + if (taken_fact[i]) { + continue; + } + if (!fact.vars_built) { + fact.vars = vars_of(fact.condition); + fact.vars_built = true; + } + if (!fact.vars.intersects(used)) { + continue; } + taken_fact[i] = true; + result.push_back(fact.condition); + used.merge(fact.vars); + changed = true; + } + for (size_t i = 0; i < cands.size(); i++) { + if (taken[i] || !candidate_vars(cands[i]).intersects(used)) { + continue; + } + taken[i] = true; + result.push_back(candidate_expr(cands[i])); + used.merge(candidate_vars(cands[i])); + changed = true; } } return result; } -std::vector BoundsTracker::relevant_facts(const Expr &e) const { - std::set used; - CollectUsedNames collect(&used); - e.accept(&collect); - return facts_mentioning(&used); -} - Expr BoundsTracker::find_constant_bound(const Expr &e, Direction d) const { return Halide::Internal::find_constant_bound(e, d, interval_scope()); } @@ -241,8 +310,7 @@ Interval BoundsTracker::find_constant_bounds(const Expr &e) const { } Expr BoundsTracker::simplify_with_context(const Expr &e) const { - std::set used; - Expr wrapped = wrap_in_used_lets(e, &used); + Expr wrapped = wrap_in_used_lets(e); wrapped = remove_likelies(wrapped); // Leave the lets standing and let simplify() pick the ones worth // inlining. Inlining them all up front repeats each value at every use, @@ -250,7 +318,7 @@ Expr BoundsTracker::simplify_with_context(const Expr &e) const { // the simplifier actually walks, since it does not memoize on the shared // nodes -- a few hundred nodes of graph can become tens of thousands of // nodes of walk. - std::vector relevant = facts_mentioning(&used); + std::vector relevant = relevant_facts(e); // Deliberately pass an empty bounds scope here, not `scope`: mixing a // bounds scope with equality facts can make the simplifier represent a // variable by its (wide) interval instead of substituting the exact diff --git a/src/BoundsTracker.h b/src/BoundsTracker.h index e7720436b937..3c8ec8fdbe31 100644 --- a/src/BoundsTracker.h +++ b/src/BoundsTracker.h @@ -6,6 +6,8 @@ * a Stmt tree. */ +#include +#include #include #include #include @@ -171,14 +173,60 @@ class BoundsTracker { Interval tighten_using_loop_monotonicity(const Expr &e, Interval interval) const; /** Wrap e in the enclosing pure lets it refers to, innermost first, - * skipping those it can't reach. Returns the names it ended up - * mentioning in *used, so a caller can go on to pick out the facts that - * could say something about it. */ - Expr wrap_in_used_lets(const Expr &e, std::set *used) const; + * skipping those it can't reach. */ + Expr wrap_in_used_lets(const Expr &e) const; - /** The facts mentioning any name in *used, growing *used with the names - * each selected fact brings in, until it reaches a fixed point. */ - std::vector facts_mentioning(std::set *used) const; + /** The set of variables an Expr mentions, identified by the hash a + * Variable node carries. That hash is derived from the name, so equal + * names hash equally, and two sets can be tested for a shared variable + * without touching a string. Held sorted, alongside a Bloom filter over + * the same hashes, so the common case of sharing nothing is one AND. + * + * Hashes collide, which here costs one extra condition handed to the + * simplifier and nothing else: picking out relevant conditions only has + * to avoid dropping one that matters. */ + struct VarSet { + std::vector hashes; + uint64_t bloom = 0; + + void add(uint32_t h) { + hashes.push_back(h); + bloom |= (uint64_t)1 << (h & 63); + } + + void sort_unique() { + std::sort(hashes.begin(), hashes.end()); + hashes.erase(std::unique(hashes.begin(), hashes.end()), hashes.end()); + } + + bool intersects(const VarSet &other) const { + if (!(bloom & other.bloom)) { + return false; + } + auto a = hashes.begin(); + auto b = other.hashes.begin(); + while (a != hashes.end() && b != other.hashes.end()) { + if (*a == *b) { + return true; + } + if (*a < *b) { + ++a; + } else { + ++b; + } + } + return false; + } + + void merge(const VarSet &other) { + hashes.insert(hashes.end(), other.hashes.begin(), other.hashes.end()); + bloom |= other.bloom; + sort_unique(); + } + }; + + /** The variables e mentions. */ + static VarSet vars_of(const Expr &e); enum class Kind { Let, //< lo is the value @@ -201,8 +249,30 @@ class BoundsTracker { Impure }; mutable Purity purity = Purity::Unknown; + /** For a Loop entry, the two conditions its range implies and the + * variables they mention between them. Built on first use; an + * entry's range never changes, so this outlives any number of + * queries. */ + mutable Expr fact_lo, fact_hi; + mutable VarSet fact_vars; + mutable bool facts_built = false; }; + /** One of the two conditions an enclosing loop implies, as an index into + * `entries` plus which end it is. An index rather than a pointer, + * because pushing a binding can reallocate `entries`. */ + struct Candidate { + size_t entry; + bool use_hi; + }; + + const Expr &candidate_expr(const Candidate &c) const; + const VarSet &candidate_vars(const Candidate &c) const; + + /** Every condition currently in force. Rebuilt only when a fact or an + * enclosing loop is pushed or popped. */ + const std::vector &candidates() const; + /** Realize every entry not yet reflected in `scope`, outermost first, so * that each one's bounds are derived in the scope of the entries that * enclose it. */ @@ -221,9 +291,20 @@ class BoundsTracker { mutable size_t realized = 0; mutable Scope scope; - /** The conditions passed to push_fact(). Those implied by enclosing - * loops are materialized on demand by relevant_facts(). */ - std::vector facts; + /** A condition passed to push_fact(), alongside the variables it + * mentions, collected on first use since most are never consulted. */ + struct Fact { + Expr condition; + mutable VarSet vars; + mutable bool vars_built = false; + }; + std::vector facts; + + mutable std::vector all_candidates; + /** Bumped whenever a fact or a loop entry is pushed or popped, to + * invalidate `all_candidates`. */ + size_t facts_version = 0; + mutable size_t candidates_version = (size_t)-1; }; } // namespace Internal From 4ea59448f3fc9d9a42c862ea5e2535335e918756 Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Fri, 11 Sep 2026 19:48:05 +0200 Subject: [PATCH 15/17] Extend the tracker's candidate conditions instead of rebuilding them 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 Claude-Session: https://claude.ai/code/session_01B8pMM1zmBL89nNwzwCEJM4 --- src/BoundsTracker.cpp | 19 ++++++++----------- src/BoundsTracker.h | 12 ++++++------ 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/BoundsTracker.cpp b/src/BoundsTracker.cpp index fb06e4069af2..dbd92f4f6f8b 100644 --- a/src/BoundsTracker.cpp +++ b/src/BoundsTracker.cpp @@ -91,7 +91,6 @@ BoundsTracker::FactGuard::~FactGuard() { BoundsTracker::Binding BoundsTracker::push_for(const std::string &name, const Expr &min, const Expr &max) { entries.push_back(Entry{Kind::Loop, name, min, max}); - facts_version++; return Binding(this); } @@ -107,7 +106,6 @@ BoundsTracker::Binding BoundsTracker::push_let(const std::string &name, const Ex BoundsTracker::FactGuard BoundsTracker::push_fact(const Expr &condition) { facts.push_back(Fact{condition}); - facts_version++; return FactGuard(this); } @@ -117,8 +115,12 @@ void BoundsTracker::pop_entry() { scope.pop(entries.back().name); realized--; } - if (entries.back().kind == Kind::Loop) { - facts_version++; + if (candidates_built == entries.size()) { + size_t idx = entries.size() - 1; + while (!all_candidates.empty() && all_candidates.back().entry == idx) { + all_candidates.pop_back(); + } + candidates_built--; } entries.pop_back(); } @@ -126,7 +128,6 @@ void BoundsTracker::pop_entry() { void BoundsTracker::pop_fact() { internal_assert(!facts.empty()); facts.pop_back(); - facts_version++; } bool BoundsTracker::entry_is_pure(const Entry &entry) const { @@ -220,11 +221,8 @@ const BoundsTracker::VarSet &BoundsTracker::candidate_vars(const Candidate &c) c } const std::vector &BoundsTracker::candidates() const { - if (candidates_version == facts_version) { - return all_candidates; - } - all_candidates.clear(); - for (size_t i = 0; i < entries.size(); i++) { + for (; candidates_built < entries.size(); candidates_built++) { + size_t i = candidates_built; const Entry &entry = entries[i]; if (entry.kind != Kind::Loop || entry.lo.type() != Int(32) || entry.hi.type() != Int(32) || @@ -249,7 +247,6 @@ const std::vector &BoundsTracker::candidates() const { all_candidates.push_back(Candidate{i, false}); all_candidates.push_back(Candidate{i, true}); } - candidates_version = facts_version; return all_candidates; } diff --git a/src/BoundsTracker.h b/src/BoundsTracker.h index 3c8ec8fdbe31..01b88d71be63 100644 --- a/src/BoundsTracker.h +++ b/src/BoundsTracker.h @@ -269,8 +269,9 @@ class BoundsTracker { const Expr &candidate_expr(const Candidate &c) const; const VarSet &candidate_vars(const Candidate &c) const; - /** Every condition currently in force. Rebuilt only when a fact or an - * enclosing loop is pushed or popped. */ + /** The conditions every enclosing loop implies. Extended as the bindings + * below it are pushed and trimmed as they are popped, so that asking + * costs only what the bindings pushed since the last question cost. */ const std::vector &candidates() const; /** Realize every entry not yet reflected in `scope`, outermost first, so @@ -301,10 +302,9 @@ class BoundsTracker { std::vector facts; mutable std::vector all_candidates; - /** Bumped whenever a fact or a loop entry is pushed or popped, to - * invalidate `all_candidates`. */ - size_t facts_version = 0; - mutable size_t candidates_version = (size_t)-1; + /** entries[0, candidates_built) have contributed whatever conditions + * they imply to `all_candidates`, in that order. */ + mutable size_t candidates_built = 0; }; } // namespace Internal From 33032734b043afcc9c0b0914bc47e6d9ca62f5b0 Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Fri, 11 Sep 2026 19:55:31 +0200 Subject: [PATCH 16/17] Only give a binding a slot in the scope when it hides something 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 Claude-Session: https://claude.ai/code/session_01B8pMM1zmBL89nNwzwCEJM4 --- src/BoundsTracker.cpp | 15 +++++++++++++-- src/BoundsTracker.h | 4 ++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/BoundsTracker.cpp b/src/BoundsTracker.cpp index dbd92f4f6f8b..b851737c781c 100644 --- a/src/BoundsTracker.cpp +++ b/src/BoundsTracker.cpp @@ -112,7 +112,9 @@ BoundsTracker::FactGuard BoundsTracker::push_fact(const Expr &condition) { void BoundsTracker::pop_entry() { internal_assert(!entries.empty()); if (realized == entries.size()) { - scope.pop(entries.back().name); + if (entries.back().in_scope) { + scope.pop(entries.back().name); + } realized--; } if (candidates_built == entries.size()) { @@ -173,7 +175,16 @@ void BoundsTracker::realize_all() const { for (; realized < entries.size(); realized++) { Entry &entry = entries[realized]; entry.bounds = bounds_of(entry); - scope.push(entry.name, entry.bounds); + // A binding that bounds nothing reads the same as no binding at all: + // a lookup that misses falls back to the bounds of the type, which + // is what an unbounded interval would have narrowed it to anyway. So + // it is only worth a slot when there is an enclosing binding of the + // same name for it to hide. Most names are unique by this point, so + // this keeps the scope down to the bindings that say something. + entry.in_scope = !entry.bounds.is_everything() || scope.contains(entry.name); + if (entry.in_scope) { + scope.push(entry.name, entry.bounds); + } } } diff --git a/src/BoundsTracker.h b/src/BoundsTracker.h index 01b88d71be63..ed651ee10899 100644 --- a/src/BoundsTracker.h +++ b/src/BoundsTracker.h @@ -249,6 +249,10 @@ class BoundsTracker { Impure }; mutable Purity purity = Purity::Unknown; + /** Whether this entry has a binding of its own in `scope`. An entry + * that bounds nothing only needs one to hide an enclosing binding of + * the same name. */ + mutable bool in_scope = false; /** For a Loop entry, the two conditions its range implies and the * variables they mention between them. Built on first use; an * entry's range never changes, so this outlives any number of From 4996ae40fa19ebeaf59aa5a4cb5d72f4fc30c775 Mon Sep 17 00:00:00 2001 From: Martijn Courteaux Date: Fri, 11 Sep 2026 20:15:05 +0200 Subject: [PATCH 17/17] Only re-test a condition against the variables the last sweep brought in 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 Claude-Session: https://claude.ai/code/session_01B8pMM1zmBL89nNwzwCEJM4 --- src/BoundsTracker.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/BoundsTracker.cpp b/src/BoundsTracker.cpp index b851737c781c..a97c7e9d6597 100644 --- a/src/BoundsTracker.cpp +++ b/src/BoundsTracker.cpp @@ -273,12 +273,16 @@ std::vector BoundsTracker::relevant_facts(const Expr &e) const { // A condition sharing no variable with what we're asking about can't say // anything about it, but one that does can bring in variables that make // a third condition relevant, so keep sweeping until nothing new is - // picked up. + // picked up. After the first sweep only the variables picked up by the + // one before can make a condition relevant, so later sweeps skip + // anything that shares none of those. std::vector taken_fact(facts.size(), false); std::vector taken(cands.size(), false); + uint64_t fresh = (uint64_t)-1; bool changed = true; while (changed) { changed = false; + uint64_t next_fresh = 0; for (size_t i = 0; i < facts.size(); i++) { const Fact &fact = facts[i]; if (taken_fact[i]) { @@ -288,23 +292,30 @@ std::vector BoundsTracker::relevant_facts(const Expr &e) const { fact.vars = vars_of(fact.condition); fact.vars_built = true; } - if (!fact.vars.intersects(used)) { + if (!(fact.vars.bloom & fresh) || !fact.vars.intersects(used)) { continue; } taken_fact[i] = true; result.push_back(fact.condition); + next_fresh |= fact.vars.bloom & ~used.bloom; used.merge(fact.vars); changed = true; } for (size_t i = 0; i < cands.size(); i++) { - if (taken[i] || !candidate_vars(cands[i]).intersects(used)) { + if (taken[i]) { + continue; + } + const VarSet &vars = candidate_vars(cands[i]); + if (!(vars.bloom & fresh) || !vars.intersects(used)) { continue; } taken[i] = true; result.push_back(candidate_expr(cands[i])); - used.merge(candidate_vars(cands[i])); + next_fresh |= vars.bloom & ~used.bloom; + used.merge(vars); changed = true; } + fresh = next_fresh; } return result; }