From 2c01cc2e45a51202163c3349ef28e02488eac051 Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Thu, 16 Jul 2026 12:59:44 -0700 Subject: [PATCH 1/4] Assert callable arg count matches buffer dimensionality in for_each_element Buffer::for_each_element previously accepted a callable taking fewer int arguments than the buffer's dimensionality, silently iterating only over a reduced-dimensionality slice. A nullary callable (e.g. buffer.fill(rng)) therefore wrote only the origin element. This is an easy mistake to make and produces no error. Require the callable to take either a const int *, or exactly as many int arguments as the buffer has dimensions. This uncovered three real bugs where buffers were only ever having their first element randomized: - test/fuzz/lossless_cast.cpp: buf.fill(fuzz) - test/correctness/interleave_rgb.cpp: fill([]() { return rand(); }) - test/correctness/multiple_scatter.cpp: fill(std::mt19937{0}) All three are fixed to use for_each_value, which passes an element reference and does not depend on coordinates. The reduced-slice usage in test/correctness/for_each_element.cpp is rewritten to slice off a channel first so the callable's arg count matches the iteration domain. Co-Authored-By: Claude Opus 4.8 --- src/runtime/HalideBuffer.h | 20 +++++++++----------- test/correctness/for_each_element.cpp | 6 ++++-- test/correctness/interleave_rgb.cpp | 2 +- test/correctness/multiple_scatter.cpp | 3 ++- test/fuzz/lossless_cast.cpp | 4 ++-- 5 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/runtime/HalideBuffer.h b/src/runtime/HalideBuffer.h index b49272d0f962..219fad464cb2 100644 --- a/src/runtime/HalideBuffer.h +++ b/src/runtime/HalideBuffer.h @@ -2463,7 +2463,10 @@ class Buffer { template HALIDE_ALWAYS_INLINE static void for_each_element(double, int dims, const for_each_element_task_dim *t, Fn &&f) { int args = num_args(0, std::forward(f)); - assert(dims >= args); + assert(args == dims && + "Callable passed to for_each_element (or fill) must accept either a " + "const int *, or a number of int arguments equal to the dimensionality " + "of the buffer."); for_each_element_variadic(0, args - 1, t, std::forward(f)); } @@ -2481,19 +2484,14 @@ class Buffer { public: /** Call a function at each site in a buffer. This is likely to be * much slower than using Halide code to populate a buffer, but is - * convenient for tests. If the function has more arguments than the - * buffer has dimensions, the remaining arguments will be zero. If it - * has fewer arguments than the buffer has dimensions then the last - * few dimensions of the buffer are not iterated over. For example, - * the following code exploits this to set a floating point RGB image - * to red: + * convenient for tests. The function must take a number of int + * arguments equal to the dimensionality of the buffer. For example, + * the following code sets a floating point RGB image to red: \code Buffer im(100, 100, 3); - im.for_each_element([&](int x, int y) { - im(x, y, 0) = 1.0f; - im(x, y, 1) = 0.0f; - im(x, y, 2) = 0.0f: + im.for_each_element([&](int x, int y, int c) { + im(x, y, c) = (c == 0) ? 1.0f : 0.0f; }); \endcode diff --git a/test/correctness/for_each_element.cpp b/test/correctness/for_each_element.cpp index 85fcfc2380fd..487d1e0e16b8 100644 --- a/test/correctness/for_each_element.cpp +++ b/test/correctness/for_each_element.cpp @@ -27,8 +27,10 @@ int main(int argc, char **argv) { im(pos) *= 3; }); - im.for_each_element([&](int x, int y) { - for (int c = 0; c < 3; c++) { + // Slice off a channel to get a two-dimensional domain to iterate + // over, then handle all channels inside the callable. + im.sliced(2, im.dim(2).min()).for_each_element([&](int x, int y) { + for (int c : im.dim(2)) { int correct = (10 * x + 5 * y + c) * 3; if (im(x, y, c) != correct) { printf("im(%d, %d, %d) = %d instead of %d\n", diff --git a/test/correctness/interleave_rgb.cpp b/test/correctness/interleave_rgb.cpp index 092b54eb569a..dc50843eb9b7 100644 --- a/test/correctness/interleave_rgb.cpp +++ b/test/correctness/interleave_rgb.cpp @@ -81,7 +81,7 @@ bool test_deinterleave(int x_stride) { deinterleaved.vectorize(x, target.natural_vector_size(), TailStrategy::GuardWithIf).unroll(c); } Buffer input_buf = Buffer::make_interleaved(255, 128, x_stride); - input_buf.fill([]() { return rand(); }); + input_buf.for_each_value([](T &v) { v = rand(); }); input_buf.crop(2, 0, 3); input.set(input_buf); diff --git a/test/correctness/multiple_scatter.cpp b/test/correctness/multiple_scatter.cpp index 9c5c0387fbab..39e03d86992a 100644 --- a/test/correctness/multiple_scatter.cpp +++ b/test/correctness/multiple_scatter.cpp @@ -33,7 +33,8 @@ int main(int argc, char **argv) { Buffer input(128, 8); - input.fill(std::mt19937{0}); + std::mt19937 rng{0}; + input.for_each_value([&](int &v) { v = rng(); }); Func sorted1; Var x, y; diff --git a/test/fuzz/lossless_cast.cpp b/test/fuzz/lossless_cast.cpp index 01ef1c38e8d7..b820f97f2071 100644 --- a/test/fuzz/lossless_cast.cpp +++ b/test/fuzz/lossless_cast.cpp @@ -264,8 +264,8 @@ bool might_have_ub(Expr e) { } // namespace FUZZ_TEST(lossless_cast, FuzzingContext &fuzz) { - buf_u8.fill(fuzz); - buf_i8.fill(fuzz); + buf_u8.for_each_value([&](uint8_t &v) { v = (uint8_t)fuzz(); }); + buf_i8.for_each_value([&](int8_t &v) { v = (int8_t)fuzz(); }); Expr e1 = random_expr(fuzz); Expr simplified = simplify(e1); From e008abc66db656098c604c55387147532b91dcd0 Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Thu, 16 Jul 2026 13:15:41 -0700 Subject: [PATCH 2/4] Make Buffer::fill dispatch zero-arg callables to for_each_value A zero-argument callable passed to fill (for example a random number generator) has no coordinate to consume, so there is no distinction between filling via for_each_element and for_each_value. Dispatch such callables to for_each_value, which evaluates the callable once per element. This makes the natural buffer.fill(rng) idiom fill the whole buffer instead of tripping the for_each_element arg-count assertion. With this, the previously-broken buffer initializations in interleave_rgb, multiple_scatter, and lossless_cast (which only ever randomized the origin element) are fixed by reverting them to their original fill(rng) form. Co-Authored-By: Claude Opus 4.8 --- src/runtime/HalideBuffer.h | 17 +++++++++++++---- test/correctness/interleave_rgb.cpp | 2 +- test/correctness/multiple_scatter.cpp | 3 +-- test/fuzz/lossless_cast.cpp | 4 ++-- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/runtime/HalideBuffer.h b/src/runtime/HalideBuffer.h index 219fad464cb2..de8c63fabf9a 100644 --- a/src/runtime/HalideBuffer.h +++ b/src/runtime/HalideBuffer.h @@ -2570,13 +2570,22 @@ class Buffer { /** Fill a buffer by evaluating a callable at every site. The * callable should look much like a callable passed to * for_each_element, but it should return the value that should be - * stored to the coordinate corresponding to the arguments. */ + * stored to the coordinate corresponding to the arguments. A callable + * that takes no arguments (for example a random number generator) is + * evaluated once per element, ignoring the coordinate. */ template>>> Buffer &fill(Fn &&f) { - // We'll go via for_each_element. We need a variadic wrapper lambda. - FillHelper wrapper(std::forward(f), this); - return for_each_element(wrapper); + if constexpr (std::is_invocable_v) { + // A callable that takes no coordinates has nothing to + // distinguish sites, so fill every value with its result. + for_each_value([&](T &v) { v = f(); }); + } else { + // We'll go via for_each_element. We need a variadic wrapper lambda. + FillHelper wrapper(std::forward(f), this); + for_each_element(wrapper); + } + return *this; } /** Check if an input buffer passed extern stage is a querying diff --git a/test/correctness/interleave_rgb.cpp b/test/correctness/interleave_rgb.cpp index dc50843eb9b7..092b54eb569a 100644 --- a/test/correctness/interleave_rgb.cpp +++ b/test/correctness/interleave_rgb.cpp @@ -81,7 +81,7 @@ bool test_deinterleave(int x_stride) { deinterleaved.vectorize(x, target.natural_vector_size(), TailStrategy::GuardWithIf).unroll(c); } Buffer input_buf = Buffer::make_interleaved(255, 128, x_stride); - input_buf.for_each_value([](T &v) { v = rand(); }); + input_buf.fill([]() { return rand(); }); input_buf.crop(2, 0, 3); input.set(input_buf); diff --git a/test/correctness/multiple_scatter.cpp b/test/correctness/multiple_scatter.cpp index 39e03d86992a..9c5c0387fbab 100644 --- a/test/correctness/multiple_scatter.cpp +++ b/test/correctness/multiple_scatter.cpp @@ -33,8 +33,7 @@ int main(int argc, char **argv) { Buffer input(128, 8); - std::mt19937 rng{0}; - input.for_each_value([&](int &v) { v = rng(); }); + input.fill(std::mt19937{0}); Func sorted1; Var x, y; diff --git a/test/fuzz/lossless_cast.cpp b/test/fuzz/lossless_cast.cpp index b820f97f2071..01ef1c38e8d7 100644 --- a/test/fuzz/lossless_cast.cpp +++ b/test/fuzz/lossless_cast.cpp @@ -264,8 +264,8 @@ bool might_have_ub(Expr e) { } // namespace FUZZ_TEST(lossless_cast, FuzzingContext &fuzz) { - buf_u8.for_each_value([&](uint8_t &v) { v = (uint8_t)fuzz(); }); - buf_i8.for_each_value([&](int8_t &v) { v = (int8_t)fuzz(); }); + buf_u8.fill(fuzz); + buf_i8.fill(fuzz); Expr e1 = random_expr(fuzz); Expr simplified = simplify(e1); From 556ed7052bd5dc093a2d0b8701d270c79db6bbee Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Thu, 16 Jul 2026 15:44:31 -0700 Subject: [PATCH 3/4] Fix rgb_interleaved performance test for stricter for_each_element The for_each_element arg-count assertion rejects a callable taking fewer int arguments than the buffer's dimensionality. These calls iterated a 2D domain over a 3D buffer and handled all channels inside the callable, so slice off the channel dimension first to get a matching 2D domain. Co-Authored-By: Claude Opus 4.8 --- test/performance/rgb_interleaved.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/performance/rgb_interleaved.cpp b/test/performance/rgb_interleaved.cpp index 3adfbeda4f52..68b53fc81d86 100644 --- a/test/performance/rgb_interleaved.cpp +++ b/test/performance/rgb_interleaved.cpp @@ -33,7 +33,7 @@ void test_deinterleave() { // Setup dst to be planar, with no extra padding between channels or rows. Buffer dst_image(1 << 12, 1 << 12, 3); - src_image.for_each_element([&](int x, int y) { + src_image.sliced(2, 0).for_each_element([&](int x, int y) { src_image(x, y, 0) = 0; src_image(x, y, 1) = 128; src_image(x, y, 2) = 255; @@ -54,7 +54,7 @@ void test_deinterleave() { printf("Interleaved to planar bandwidth %.3e byte/s.\n", dst_image.number_of_elements() / t1); - dst_image.for_each_element([&](int x, int y) { + dst_image.sliced(2, 0).for_each_element([&](int x, int y) { assert(dst_image(x, y, 0) == 0); assert(dst_image(x, y, 1) == 128); assert(dst_image(x, y, 2) == 255); @@ -69,7 +69,7 @@ void test_deinterleave() { dst.realize(dst_image); }); - dst_image.for_each_element([&](int x, int y) { + dst_image.sliced(2, 0).for_each_element([&](int x, int y) { assert(dst_image(x, y, 0) == 0); assert(dst_image(x, y, 1) == 128); assert(dst_image(x, y, 2) == 255); @@ -111,7 +111,7 @@ void test_interleave(bool fast) { // Setup dst to be interleaved Buffer dst_image = Buffer::make_interleaved(1 << 12, 1 << 12, 3); - src_image.for_each_element([&](int x, int y) { + src_image.sliced(2, 0).for_each_element([&](int x, int y) { src_image(x, y, 0) = 0; src_image(x, y, 1) = 128; src_image(x, y, 2) = 255; @@ -136,7 +136,7 @@ void test_interleave(bool fast) { printf("Planar to interleaved bandwidth %.3e byte/s.\n", dst_image.number_of_elements() / t); - dst_image.for_each_element([&](int x, int y) { + dst_image.sliced(2, 0).for_each_element([&](int x, int y) { assert(dst_image(x, y, 0) == 0); assert(dst_image(x, y, 1) == 128); assert(dst_image(x, y, 2) == 255); From 9d2f05330cd2cdfd05ee175d97121255624960a8 Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Thu, 16 Jul 2026 20:51:41 -0700 Subject: [PATCH 4/4] Mark host buffer dirty when filling with a zero-arg callable The zero-arg fill path fills via for_each_value, which writes through raw element references and does not set the host_dirty flag the way operator() does. Without it, a subsequent GPU run does not copy the freshly filled data to the device and reads stale contents. Set host_dirty explicitly, as the scalar fill overload already does. Co-Authored-By: Claude Opus 4.8 --- src/runtime/HalideBuffer.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/runtime/HalideBuffer.h b/src/runtime/HalideBuffer.h index de8c63fabf9a..f95541221c2b 100644 --- a/src/runtime/HalideBuffer.h +++ b/src/runtime/HalideBuffer.h @@ -2579,6 +2579,9 @@ class Buffer { if constexpr (std::is_invocable_v) { // A callable that takes no coordinates has nothing to // distinguish sites, so fill every value with its result. + // for_each_value writes through raw references, so it does not + // mark the host buffer dirty the way operator() does. + set_host_dirty(); for_each_value([&](T &v) { v = f(); }); } else { // We'll go via for_each_element. We need a variadic wrapper lambda.