Repository navigation
Conversation
The simplifier already collapses select(x, select(x, y, z), w) when the inner select is at the root of a branch, but in practice it is usually buried under some arithmetic, e.g. select(x, p, (select(x, q, r) + s)/c0) which shows up in the bounds expressions of multi-resolution pipelines (interpolate, local_laplacian). Add the wrapped forms, covering an addition or subtraction in either operand order, with and without a division, for both branches. Also add the corresponding rule for a min of two selects on the same condition where one is divided. Substituting the branch the condition selects is safe here even though both branches of a select are evaluated: it only ever removes an operand that was already being evaluated, so it cannot make the expression less defined. That is why these work as rewrite rules where a scoped fact would not. All rules were checked with apps/super_simplify/filter_rewrite_rules, which verifies them with z3 and confirms they obey the reduction order. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The rules added in the previous commit covered addition, subtraction and division around the inner select, but not multiplication, and those cases do not simplify on their own. Add them, in both operand orders of the multiply, with and without a surrounding addition, for both branches, plus the corresponding min rule. Also reword the justification, which as written was vacuous - every operand of a select is always evaluated, so saying the rewrite does not introduce one that wasn't says nothing. The actual reason these are safe is that the replacement is a subexpression of what it replaces, which is weaker than assuming the condition holds while simplifying the branch. The arithmetic here is quasi-affine rather than affine, since the division is integer division. Verified with apps/super_simplify/filter_rewrite_rules, and clang-format applied. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Going down this path we could add rules arbitrarily deep, so restrict the set to the shapes actually observed in pipeline bounds expressions: addition, subtraction, and integer division around the inner select. Also drops the corresponding min rule and the tests for the removed forms, and reworks the comment. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sliding window can warm up a window either by computing the whole region on the first loop iteration (a select in the bounds), or by rewinding the loop min so the steady-state bounds are all that's needed. The second form was broken whenever more than one Func slid over the same loop, which is issue 8140. Three things were wrong: - The warm-up guard was placed by asking "is this a consume node that doesn't contain the producer we just slid?". In a chain f0 -> f1 -> f2 where f0 and f1 both slide, `consume f0` doesn't contain `produce f0`, so the guard went around the whole thing including `produce f1`, suppressing f1's own warm-up iterations. f2 then read values f1 never wrote. Guards are now placed once, after all Funcs have been slid, around everything the warm-up iterations don't need. - Each rewind emitted a `<loop>.loop_min.orig` let whose value was the already-rewound min, so the name meant something different on each successive slide, and the rename substituted `<loop>.loop_min` throughout the body, silently retargeting the select-style warm-up of any Func that had already been slid the other way. Both are gone; the pass now uses the loop's own min directly, and AddLoopMinOrig with it. - Funcs were slid in order of their realizations, but the prev_loop_mins machinery needs consumers first, and a store_root'd producer is realized outside its consumers. They're now topologically sorted, and a Func warms up early enough for the earliest of its consumers rather than the first one found. Also adds the sliding window fuzzer from PR 8144, which generates random stencil pipelines laid out on the levels of a pyramid and schedules them with random sliding, storage hoisting, and register sliding. On main it produces wrong output for a few percent of the pipelines it generates; with this change that drops to roughly one in four thousand. The remaining case is understood: a Func consumed by a warming-up Func needs to start exactly when that consumer starts, rather than running from the rewound loop min, or it corrupts its own sliding over an enclosing loop. Issue 8141 appears to have been fixed in the meantime, so the fuzzer exercises hoist_storage too. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The guard injected after rewinding a loop used a single start iteration for everything the warm-up didn't need, and left everything it did need unguarded. That's too coarse in both directions. A Func consumed by a warming-up Func has to run during that Func's warm-up, but not before it: if it's warming up a window of its own over an enclosing loop, running it extra times corrupts that. The fuzzer finds this with a Func computed inside another Func's production, sliding over the outermost loop, whose consumer warms up over an inner loop that some third Func has already rewound further. Each statement now carries the iteration it starts at: nothing for a warming-up producer, whose bounds are empty until it has work to do; the start of the earliest warming-up consumer for the things such a producer consumes; and the original loop min for real work. Guards are placed at the outermost node where everything below agrees on a start. The old code got this right by accident. Each rewind redefined <loop>.loop_min.orig to the previous rewind point, so each slide's guards landed at the preceding Func's start. That let is gone, so the relationship is now explicit. Fuzzing 14000 random sliding schedules finds no failures, where the previous commit failed about one in four thousand. The lowered IR for apps/local_laplacian is unchanged. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The pass was hard to follow because it never wrote down what it had decided. Each phase communicated with the next through the IR, using naming conventions, and rederived what it needed by pattern-matching what the previous phase emitted. That's how a let called ".loop_min.orig" came to mean two different things on successive slides without anyone noticing. Deciding what to do with a Func now produces a SlideDecision: which dimension, which direction, the region required before and after, and the iteration to rewind the loop to. Applying it makes no decisions. The decisions for all the Funcs around a loop are collected into a plan, which HL_DEBUG_CODEGEN=2 prints as a few readable lines per Func instead of the scattered debug spew you'd otherwise have to reconstruct it from. Also: - The order in which Funcs are slid is asserted to be topological. Getting it wrong means a Func doesn't know how far back its consumers start, which was the original bug. - Dependencies come from the Function DAG rather than from walking the loop body. Both give the same answers - we only ask about Funcs whose producers are both present - but the DAG doesn't cost a traversal per query, and the produce/consume nesting caveat goes away with it. - ExprDependsOnVar is replaced by expr_uses_var, and FindProducedFuncs by a visit_with. - Two worked examples of the IR before and after, including one showing why a single warm-up guard isn't enough. No functional change: the lowered IR for apps/local_laplacian is identical, the correctness suite passes, and 6000 random sliding schedules produce the same results as compute_root. The fuzzer gains piecewise-affine indices, off by default. Turning them on finds wrong output on main as well as here, so it's a pre-existing bug rather than something to fix in this commit: a region required that is flat over part of its range slides incorrectly. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The check for whether the solver found a useful new loop min compared it to the old one syntactically. When the region required is monotonic but flat over part of its range - which is what a clamped coordinate in the consumer looks like, so it happens whenever there's a boundary condition - the solver returns something like min(max(loop_min, 4) - 2, loop_min), which isn't syntactically the loop min but is equal to it. The pass then believed it had warm-up iterations that don't exist, and the producer computed an empty region on every iteration, so the consumer read values that were never written. Ask can_prove whether the new loop min is genuinely earlier instead. Where it isn't, we fall back to warming up on the first iteration, which is correct here. Fixes the degenerate case. A flat region can still get the wrong number of warm-up iterations when the loop does rewind, because the solve assumes the window advances once per iteration; turning on generate_piecewise_affine in the fuzzer still finds that, on main as well as here. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Warming a window up by rewinding the loop works by tiling the warm-up with copies of the steady state, so it needs every iteration to advance the window by the same amount. That holds when the region required is affine in the loop var, and doesn't in general otherwise: a region that's flat over part of its range doesn't advance at all there, so no number of extra iterations covers it, and the solver's estimate of how many to add comes out short. Stencils over anything with a boundary condition hit this, because clamping a coordinate is exactly what makes the region required flat and then rising. Check for it directly, by asking whether the region required advances by an amount that doesn't depend on the loop var. Resampling by a constant factor advances uniformly over several iterations rather than one, so allow any small period; that keeps pyramids sliding. Where the check fails we fall back to warming up on the first iteration, which handles these cases correctly. The fuzzer now generates piecewise-affine indices, which finds the above on main within a hundred pipelines. Six thousand random schedules pass here. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
A Func stored in registers slides by shifting values along an unrolled array,
which can only carry them from one iteration of the innermost loop the array
lives in to the next. When such a Func slid over two nested loops - a dimension
each - it got two nested rolls, and the outer one read values that the sweep of
the inner loop had already overwritten. For
f(x, y) = ...;
g(x, y) = f(x, y) + f(x - 1, y) + f(x, y - 1);
g.split(y, yo, yi, 4);
f.store_at(g, yo).compute_at(g, x).store_in(MemoryType::Register);
every value of g came out wrong.
Refuse to roll over a loop that has another loop between it and the producer,
leaving the roll to that inner loop. Loops that only run once don't count;
they can't carry anything from one iteration to the next either way, and a
consumer computed at an outer loop level puts one of those in between without
changing where the roll belongs.
The fuzzer used to require the store and compute levels to be adjacent to
avoid generating this, which is no longer needed, so register sliding now gets
generated for about 8% of pipelines rather than 6%.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The grammar only generated indices with positive coefficients, so the region required always moved upwards as a loop advanced. Sliding downwards, which is a separate branch of the analysis, was never generated at all: zero downward slides in a hundred pipelines. Give each stage a direction as well as a pyramid level, and walk it backwards when the consumer's direction differs. Like the level, the direction has to be a property of the stage rather than of each access, or a Func read both forwards and backwards would have a footprint that grows with the loop var. Roughly half of the pipelines now slide something downwards. Also let the factor by which one pyramid level resamples the one below it be symbolic rather than a literal 2, in two flavours: a Param directly, whose sign isn't known at compile time, and one clamped positive. The first should stop sliding rather than slide the wrong way, and does. The second still slides. Neither extension finds anything here, but together they find fifteen wrong results in three hundred pipelines on main. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The grammar only ever indexed a Func as f(x + c, y + d), so the region required always depended on the loop vars one dimension at a time, in the obvious order. Give each Func an indexing mode that all of its consumers use: a dimension pinned to a constant, both dimensions taken from the same coordinate, the dimensions swapped, or one dimension depending on both coordinates. These reach parts of the analysis nothing else does. A broadcast dimension lands in the case where the footprint doesn't depend on the loop var at all, a transpose means sliding over one loop slides the other dimension, and a shear or a diagonal makes more than one dimension depend on a single loop var, which we have to decline to slide. That last bail fires eleven times per eighty pipelines now, and never fired before. Like the level and the direction, the mode is a property of the Func rather than of each access, or a Func indexed two different ways would have a footprint that grows with the loop var. Register sliding is restricted to the plain mode, which is the only one whose footprint is a constant size. Finds nothing here in eight hundred pipelines, and four wrong results in three hundred on main. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The grammar generated pure Funcs only, so nothing exercised the parts of sliding window that deal with update stages: the check that the dimension we want to slide is pure in every update, the copying of the sliding bounds across the stages, and the expansion of the last stage's bounds to cover what the earlier ones wrote. Give some stages an update definition: a couple that stay pure, and a couple that scatter along a dimension with an RDom, which sliding has to decline. The "scatters along the related axis" bail now fires twenty-four times per eighty pipelines, and never fired before. This turned up an unrelated pre-existing crash. Hoisting a Func into the loops of a Func with update definitions produces one hoisted storage node per stage of the consumer, but the producer is only realized inside the stages that actually use it, so storage flattening hit an assertion looking for the allocation belonging to one of the others. An empty hoisted storage node has nothing to hoist, so drop it. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Three parts of the pass had no coverage: the handling that stops a specialization's two copies of the loop nest confusing the record of which dimensions have been slid, the rewriting of the loop a prefetch directive refers to when we rewind that loop, and the interaction with splits of the producer's own loops. Specializing the output and prefetching the producers finds nothing, in four thousand pipelines. Splitting the producer does find something, so it's off by default. If the dimension a Func slides along is split with a tail strategy that computes outside the region it was asked for, the extra values land on top of parts of the window that earlier iterations computed and later ones still need, and the output is wrong. This is not new - it fails the same way on main. RoundUp, ShiftInwards and both of the blend variants all do it; GuardWithIf is clean. Sliding window already compensates for exactly this when a Func has update definitions, by growing the region computed to cover everything the producer writes, but that only runs for Funcs with updates. Extending it to all of them isn't right as it stands: it changes the region computed for Funcs that were sliding correctly, which the tracing test notices. Declining to slide a dimension whose splits overcompute isn't right either - the existing tests include schedules that vectorize the dimension they slide and expect it to keep sliding. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Splitting the dimension a Func slides along was off because it found wrong output. It only does so for tail strategies that compute outside the region they were asked for, so turn it back on using GuardWithIf, which doesn't. Three quarters of pipelines now vectorize or unroll a producer, and six thousand of them agree with compute_root. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
It was living in test/correctness with its own main, its own trial loop and a time-based seed. Make it a FUZZ_TEST like the others, so the harness decides how many runs to do and a failing input can be replayed. The one thing it needs that the others don't is to build the same pipeline twice, once compute_root and once scheduled to slide. It used to do that by reseeding its own rng between the two. Now the first build records the choices it makes and the second replays them, which is less fragile anyway: the old arrangement broke silently if a configuration flag changed how many times the rng was called, which happened twice while writing this. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Three related changes. Sliding window clamped the shrunk min against the region required at a fixed loop min. That constant clamp survived simplification and, where several Funcs slide over the same loop with different warm-up starts, compounded into a min of maxes that defeated storage folding's bounds analysis. Clamp against the region required at the current iteration instead, which is the tighter statement of the same intent and stays affine in the loop variable, so it folds away. Storage folding computed the footprint by CSEing the min and the max independently and then subtracting. The two get divergent let bindings for the same subexpression, so the terms they have in common never cancel. Take the difference first and CSE that, and strip likelies at the source. The extent then reduces to a constant in most cases, and to a select of constants when a warm-up guard is live, which the existing find_constant_bound call already handles. Relatedly, folding needed the min to advance monotonically. Either end advancing is sufficient: the footprint always fits in extent values, so everything below max - extent + 1 is outside every future footprint. The other end may then wander backwards within the window without aliasing anything live. The fuzzer no longer computes anything inside the inner loop of a PredicateStores split. Those iterations still run their producers after the store is predicated off, and the producers access outside the region their allocation was sized for. That is a pre-existing bug unrelated to these changes. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The clamp against the region required at the loop min is restored. Using the region required at the current iteration instead made the max a no-op, so a Func no longer had an empty region on the iterations that were rewound for some other Func's benefit, and it computed and read outside what anything asks for. Storage folding still gets the fold factors it needs, because the simplifier now collapses the min of maxes that per-Func warm-up starts produce. Folding a Func with an async producer needs the amount released each iteration to be non-negative. Requiring either end of the footprint to advance isn't enough for that, since either one alone can satisfy it, so check the required footprint's ends directly in the async case. The assert against visiting a producer twice now only fires if a slide was actually chosen, matching what the code it replaced checked. Producers can legitimately appear more than once, for instance under a specialization. Also drops an unused parameter and an unused return value, and gives the two-Funcs-sliding test a coordinate-dependent input, since with a constant input any coordinate mix-up produced the same answer. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The sliding_window_marker named the loop with op->name, but by that point the loop may have been renamed because it was rewound, so the argument was a reference to a variable that no longer exists. Nothing reads that argument today - storage folding only uses the func name - but it should name the loop it means. Also fixes a comment that still referred to new_loop_min, notes why checking max_provided alone is enough on the acquire side, and says explicitly that the last of the new sliding window tests passes without the fix and is there for coverage rather than as a regression test. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The sliding window fuzzer holds the tail strategy it picked in a local called strat, which codespell reads as a misspelling of start. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sliding window picks a loop to slide over. When the dimension the window should advance along has been split, no loop corresponds to it: it survives only as a let, whose value is an affine combination of several loop vars. Sliding over one of those loops instead computes a whole iteration's worth of the producer at a time, which is not what was wanted, and makes the folded storage bigger rather than smaller. f.store_root().compute_at(g, xi).slide(g, x); records the dimension in the schedule, and sliding window then slides at the let that carries it. A let can't have iterations prepended to it, so these warm up with a select rather than by rewinding. Also factors storage folding's loop visitor into a helper parameterised by a name and its bounds, so that it can fold over such a let too. That part isn't wired up yet - the let doesn't survive to storage folding, because the simplifier substitutes it - so folding still happens over the loops, which is why the modulus doesn't disappear yet. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Three fixes and a check. Sliding window bound each loop's range only while considering funcs for that loop, not while descending into its body. A let carrying a dimension therefore couldn't be bounded when we reached it, so the warm-up condition came out as x < x, which is never true, and the region never shrank. The answers were right, but nothing slid. Keep the range in scope while descending. Storage folding can't derive the fold factor for such a dimension. The let is gone by the time it runs, because the simplifier substitutes it, and measuring the footprint over any of the loops the dimension spans counts values that are never simultaneously live. Sliding window knows the window width, so it now records it on the sliding_window_marker, whose second argument previously went unread. The select-style warm-up marks the steady-state branch with likely_if_innermost, which only asks for the innermost loop to be partitioned. When sliding over a dimension the condition is on an outer loop, so nothing was peeled. Use likely there instead. Finally, sliding over a dimension is only sound if the storage lives across every loop that dimension varies over. Loop-based sliding gets that for free, because it only slides over loops between store_at and compute_at. Naming a dimension doesn't, so check it and give the user an error. Without it, a schedule like f.store_at(g, yo).compute_at(g, x).slide(g, y) reads values from the previous yo iteration out of an allocation that has since been freed and remade. It happened to give correct answers whenever the allocator returned the same block; poisoning fresh allocations shows the garbage. The fuzzer now generates the directive too, subject to the same rule. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The window advances once per value of the dimension being slid over. If the region required also depends on a loop between that dimension and where the producer is computed - a shear, for instance, where the y index depends on x - then the window would have to advance within a single value of the dimension too, and by the time a later iteration wants them, the values have been passed over. The fold factor derived from the region at one point of that dimension is then too small, and the values get clobbered. Found by the fuzzer, which now only asks to slide when the producer is computed directly inside the dimension. Whether computing further in is legal depends on the index expressions rather than the schedule, so it can't tell from where it decides. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Adds a correctness case that checks the call count as well as the values. Checking values alone isn't enough here: an earlier version of this that did no sliding at all still produced the right answers, and only the count showed it. Adds error tests for the two schedules that can't be honoured - storage that doesn't live across the loops the dimension spans, and a region that moves with a loop between the dimension and the producer. Serializes slide_level, appended to the end of the FuncSchedule table so older pipelines still read. Note this build has no flatc, so WITH_SERIALIZATION is off and that code is not compiled here; it needs checking somewhere that has it. Finally, writes down why storage folding takes sliding window's word for the fold factor, and what sliding window checks to earn it. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The test is written to be read: it walks through sliding over a loop, what goes wrong when that loop is a split of the dimension you meant, and what naming the dimension fixes. Each case checks how many times the producer ran, because the values alone don't distinguish sliding from not sliding - an earlier version of this feature computed the right answers while sliding not at all. The last example slides over a Var that is intermediate in a chain of splits, which is neither one of the consumer's original Vars nor a loop. That already worked; it needed a test and a mention in the docs. Note the producer has to be computed at the loop where that Var changes - computing it further in means the region required moves with a loop inside the dimension, which is rejected. Serialization of slide_level is now verified rather than written blind. It needs flatbuffers newer than the flatc 2.0.0 in the snap package; the Ubuntu flatbuffers-compiler package works. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The point of naming the dimension is that each unrolled body knows its index into the folded buffer, so indexing it doesn't cost a modulus. That isn't visible in the output values, so use a custom lowering pass to go and count the moduli in the loads from the producer: the version that slides over a loop has some, and the version that slides over the dimension has none. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sliding window analysis already slides a Func over several dimensions at once when it picks loops itself - a two-tap-square stencil slides over both x and y, and folds storage to two rows. The directive could only name one dimension, so asking for one gave up the other. slide_level becomes a list, and calls to Func::slide accumulate. Naming a dimension that a loop does correspond to now works too. It previously did nothing at all: a Func with the directive set was skipped by loop-based sliding on the assumption that the dimension would be caught at the let carrying it, but an unsplit Var has a loop and no such let, so nothing slid and nothing said so. The opt-out is now per loop. Naming two dimensions that move the same dimension of the Func, such as a Var and a split of it, is rejected. The window can only advance along a dimension once, so one of the two requests was being dropped silently, leaving a schedule that looked like it asked for one thing and did another. Error messages now use the names the schedule used rather than the mangled loop names in the IR. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sliding a stage over both x and y needs the region required of it in y to depend only on y, and that's a property of every stage between it and the output rather than of the stage itself. A downstream transpose or shear moves the region in y with the x loop, which is rejected. Registers need a fixed-size allocation. Rolling only shrinks the dimension that moves with the innermost loop, so every other dimension keeps whatever extent the store level gives it, and storing at the root leaves an extent that grows with the output. The fuzzer constrained the compute level for this and not the store level. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Storage folding can't measure the window itself when the schedule names the dimension, because the dimension is a let by then and the simplifier substitutes it away, so sliding window works the width out and leaves it in a marker. That only happened when the width was already a literal. A dimension spread over several loops has a width written in terms of their bounds - xi.loop_max + 5 rather than 8 - so no marker was left and the allocation kept its full extent. Expand the lets to reach the constant behind them, and take an upper bound rather than insisting on an exact one. Erring large folds to a larger power of two, which is the direction the marker's contract requires. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
A consumer's work can happen in an update stage, where the dimension to slide over is an RVar. LoopLevel already takes a VarOrRVar, so slide was narrower than the thing it constructs. VarOrRVar converts implicitly from both, so this doesn't disturb existing callers. Func::slide was missing from the Python bindings. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Two fixes to sliding over a named dimension. A window only ever moves forwards. Sliding over a loop never had to check that, because a loop counter only goes up, but a dimension reassembled from several loops has no such guarantee, and the schedule can break it two ways. Reordering the loops of a split puts the smaller term on the outer loop, so the dimension runs 0, 8, 16, ... then 1, 9, 17, ... ShiftInwards pulls the last iteration back so it fits, so with an extent of 10 and a factor of 4 it runs 0-3, 4-7, then 6-9. Both silently computed wrong answers - 24 of 32 values in the reordered case. The condition checked is the one that makes a mixed-radix number count upwards: each contributing loop has to advance the dimension by at least as much as the loops inside it take back when they wrap around. That still accepts RoundUp, GuardWithIf, PredicateStores, and ShiftInwards once the extent is aligned, because then the tail never has to shift. A consumer that clamps the index it reaches back to, as it must when the producer is backed by an input buffer, was not folding. The two ends of the window are correlated, and cancelling them needs what we know about the dimension, but the bounds went into scope as unsimplified arithmetic on loop mins and extents - a min of ((0*2) + 0) + 0 rather than 0 - so nothing could use them. Simplify the ends before binding them, and take the window width from the form written in terms of the dimension rather than the rebased loop and its base. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Simplifying the ends of a let's bounds before they go into scope is what lets a clamped stencil fold, but doing it at every let is quadratic: bounds expressions nest, and a cascade of sliding stages made correctness_sliding_window_cascade take over an hour instead of three seconds. Only the dimensions being slid over need it. Restricting it that way lost the fold for a dimension split across several loops, because the two forms of the window's width fail for different reasons. Written in terms of the dimension, the ends cancel against each other, which is what a consumer that clamps its index needs. Written in terms of the loops the dimension was split across, it reaches the constants in their bounds. Try both and keep the tighter. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
A window slid over a dimension warmed up by computing the whole region on the first iteration, chosen with a select. That leaves the producer's extent data-dependent, which matmul and tile operations can't use - they need constant shapes - and no amount of downstream simplification gets it back. Rewind instead. The dimension can't have iterations prepended to it directly, but the outermost loop it depends on can, so the let works out how far back it needs to reach and the loop prepends that many iterations on the way out, when the count is finally known. The count is a symbol inside the loop and gets its value in a let wrapped around it, which is how the loop's own min is already handled. Rewinding by whole steps of a loop overshoots, so the producer and the consumer are both guarded on the dimension: the consumer waits for the dimension to reach its real min, and the producer waits for the point its work begins. Guarding the producer is what lets its bounds stay the plain steady state. Sliding over a loop keeps clamping them instead, because there the rewind lands exactly where it was asked to and the bounds go empty on their own. The guards ask for the loop to be partitioned rather than only the innermost one, because a dimension has loops nested inside it, and likely_if_innermost would leave the warm-up folded into the steady state. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Naming a dimension made storage folding decline to fold at every loop, on the grounds that folding happens at the let carrying the dimension instead. That holds for a dimension split across several loops, but a dimension that hasn't been split is a loop, with no such let, so nothing folded at all and the allocation stayed at full extent. The values were right, so only an allocation size catches it. Decline only at loops that don't carry a named dimension. That needed the loop name stripped of the ".$n" that sliding window appends when it rewinds a loop, because a LoopLevel naming the original Var doesn't match the renamed loop. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Storage folding was told how wide the window is by the region required of the producer. Sliding doesn't write that region, though - it writes the sliver from where the previous iteration stopped up to what's needed now. The two coincide while the region required steps along, but a select in the consumer's index makes it jump, and then the producer covers the gap and writes a wider range than anyone asks for. Folded to the narrower width, those writes landed on top of each other, and the consumer read a value that had been overwritten. Wrong answers, no error, and only in the iteration where the jump happens. Take the span of the two instead. The sliver has to come from before the warm-up select was folded into it, because interval arithmetic can't see through that select, and the promises the two ends carry about staying in range have to be dropped, because the simplifier can't cancel across them - left in, a difference that is plainly one bounds to the whole extent, and nothing folds at all. Found by the sliding window fuzzer, which needed a reversed access, a split dimension and a select in the same schedule to reach it, and took 1393 seeds to do so. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Successive edits to the code below it each left their comment behind, so it had accumulated four stale blocks: a truncated line, a repeated paragraph, and a sentence saying to bound the two ends separately rather than taking a min and a max of them, which is the opposite of what the code does. Comment only; no change in behaviour. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Bounds inference emits a .min/.max let per enclosing loop level, and sliding rewrote only the one immediately inside the producer's loop. A consumer staged at an outer level reads an outer copy, which still asks for the region required before the window slid. For a Func defined inductively over the dimension it slides along that is the entire prefix of the walk, so a producer staged once per step gets allocated the whole walk. The answers stay right; the allocation doesn't, and on a GPU it is fatal, because a symbolic extent can't be unrolled. Rewriting every occurrence instead is not sound. The replacement describes the region required at the producer, so it can mention loops and lets between there and an outer declaration, and moving it outwards leaves references to names that aren't in scope. The sliding window fuzzer produces such a schedule within a hundred seeds. So rewrite each declaration with a bound valid where it sits: the loops in between are bounded away, taking the low end of a min and the high end of a max, which is the union over them. If a name bound in between survives that, the declaration is left alone, because a region required that is too large is wasteful whereas a reference to something out of scope is not valid IR. Only declarations that lexically enclose the producer are considered at all - the traversal also reaches lets in sibling subtrees, which is what the loop-level check this replaces was guarding against. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Rewriting a declaration outside the producer's own loop level means widening the replacement over the loops between the two, since their variables are not in scope out there. bound_over_inner_loops did that with a scope chained to the enclosing bounds, which holds an interval for every enclosing loop - including the loop being slid over, pushed by the caller just before the slider is invoked. So the widening ran over the walk as well, and the union of a per-step region over every step is the whole walk. An outer declaration collapsed to the region required before the window slid, which is what rewriting it was supposed to avoid, and a producer staged once per step and read at an outer level inherited it. That variable never needed widening: both call sites hand the slider the body of the loop being slid over, so it is in scope at every declaration the mutator visits. Widen only the loops in between and leave everything else exact. The test covers it with an ordinary sliding window rather than a recurrence. What the bug needs is a Func that slides over the walk, a producer feeding it computed at an outer level, and a loop in between; a recurrence only makes the symptom louder, because its unslid region is the entire prefix rather than one extra sliver. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Storage folding measures a producer's footprint over the loop it slides along and bounds the difference between the two ends. A consumer that clamps the index it reaches back to carries promises about staying in range, and the simplifier cannot cancel across those, so the difference bounds to the whole extent and nothing folds at all. Take the promise-stripped form of the extent when it bounds more tightly. Stripping changes what can be proved about the value, not the value itself. It is not always tighter, because a promise can be the only thing bounding an index, so the two forms are compared rather than one replacing the other. Sliding window already strips promises when it derives a window width for storage folding, which it does for a dimension spread across several loops. Doing it on only one of the two paths meant that splitting a consumer's loop by one - which changes none of the loops that get generated - changed how tightly the producer folded. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
test/correctness/scratch.cpp is a scratch pad, not a test. It has no entry in the CMake file list, which the file-list check rejects, and it is not built or run by either build system. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Every .c/.cpp beside a CMakeLists.txt has to be named in it, which is what keeps the file lists complete. The check reads the directory, so it also sees files git has been told to ignore - a scratch pad kept next to the tests fails the check even though it is nobody else's business. Skip the files git ignores, and ignore the scratch name that has been used for this so far. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sliding over a loop and sliding over a let that a split Var survives as now go through one path: a loop is just a target whose value is its own variable. A let target must step by exactly one as the loop nest runs, checked with derivative bounds over the enclosing loops, and storage folding trusts that rather than rechecking it. Warm-up rewinds the outermost loop the dimension varies over, and producers of a slid Func computed outside the dimension are bounded to its warm-up region. Sliding over a dimension that varies over a non-serial loop is an error. Also simplify for review: consumers_first is a sort by dependency count, dead fields are gone, and the standalone slide error and correctness tests are folded into slide_directive.cpp and sliding_window.cpp. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve the FuncSchedule schema conflict by appending slide_levels after main's storage_splits. Produce/consume trace events now live inside the node body (#9505), so warm-up guards treat a LetStmt as a unit when everything in it starts together, and never guard an end trace event on its own, keeping it paired with its begin event. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Whether a dimension a Func slides over steps through its values one at a time is a property of the consumer's splits and loop order, so check it in validate_schedule rather than reconstructing it from the lowered loop nest. The loops a dimension is split into must be serial or unrolled, adjacent, outer outside inner, and only the outermost split may have a tail, which can't be ShiftInwards. Fusing the dimension with another is rejected. Exactness of a split reuses apply_split's extent alignment tracking, seeded by a known_extent_alignments helper shared with build_loop_nest. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Require constant split factors along a slid dimension. A non-constant factor used to hit an internal error, and the window's step is now always a known constant, so sliding can always rewind. - Check in validate_schedule that a slid dimension's outermost loop is inside the Func's storage and at or outside its compute level. Both mistakes used to silently not slide when slid over a loop. - Record what each storage dimension was slid over in slid_dimensions, reporting dimensions that move the same storage dimension directly. - Explain why lets use likely rather than likely_if_innermost. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove an unused operator<< from the sliding window fuzzer, which broke every CMake build under -Werror. - Reserve before filling a vector in consumers_first, for clang-tidy. - Guard the profiler's box declarations with the start of the Func they describe, rather than as work starting at the original loop min. The warm-up iterations a rewound producer runs weren't being counted in points_required_at_production (profiler_instances). - Fold constant let values (e.g. a split's inner loop max of 2 - 1) before putting them in the bounds scope, and use that scope when checking a solved warm-up start. camera_pipe's curved couldn't prove its rewind was valid, fell back to a select warm-up, and everything it consumes then failed to slide, tripping its explicit fold factor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Strips the explicit slide directive and the machinery for sliding and folding over split Vars (lets), to evaluate the sliding window overhaul as a PR of its own. Sliding and folding are back to working over loops only, and the abstractions that existed to cover both cases are collapsed: SlideTarget, FindSlideLets, loops_used, the let branch of SlidingWindowOnFunctionAndLoop, the likely/likely_if_innermost choice, and fold_over/fold_at_loop in storage folding. The fixes that apply to loops are kept, including the CI fixes for the profiler box declarations and camera_pipe. Lowered IR for the sliding and folding tests is unchanged from the branch it was cut from. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
alexreinking
reviewed
Nov 2, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9648 +/- ##
==========================================
+ Coverage 72.78% 72.84% +0.05%
==========================================
Files 264 266 +2
Lines 82985 83414 +429
Branches 20372 20496 +124
==========================================
+ Hits 60404 60765 +361
- Misses 16139 16175 +36
- Partials 6442 6474 +32 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The sliding window fuzzer built each pipeline twice, recording the choices made the first time and replaying them the second so that both builds described the same algorithm. Instead, build it once, deep copy it, schedule the copy compute_root for the reference, and schedule the original to slide. This removes the replay mechanism. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
3 tasks done
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is #9633 stripped down to just fixing sliding window and adding a fuzzer for it, without adding any new features.