Skip to content

Commit 32ba946

Browse files
committed
[Fixup] Bump ndarray_data_gen on host-side ndarray writes + fill so cached adstack-sizer metadata is evicted
1 parent 17a5873 commit 32ba946

3 files changed

Lines changed: 74 additions & 0 deletions

File tree

quadrants/program/ndarray.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include <numeric>
22

3+
#include "quadrants/program/adstack_size_expr_eval.h"
34
#include "quadrants/program/ndarray.h"
45
#include "quadrants/program/program.h"
56
#include "fp16.h"
@@ -185,6 +186,10 @@ void Ndarray::write(const std::vector<int> &I, TypedConstant val) const {
185186
staging_buf_->device->memcpy_internal(this->ndarray_alloc_.get_ptr(index * size_), staging_buf_->get_ptr(), size_);
186187

187188
prog_->synchronize_and_assert();
189+
// Host-side mutation of the ndarray contents: bump the per-DeviceAllocation generation so any cached
190+
// adstack-sizer metadata that depended on `ExternalTensorRead` of this ndarray is evicted on next launch.
191+
// Keyed by the same `&ndarray_alloc_` the kernel launchers use in `bump_writes_for_kernel_*`.
192+
prog_->adstack_cache().bump_ndarray_data_gen(const_cast<DeviceAllocation *>(&ndarray_alloc_));
188193
}
189194

190195
int64 Ndarray::read_int(const std::vector<int> &i) {

quadrants/program/program.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,10 @@ void Program::fill_ndarray_fast_u32(Ndarray *ndarray, uint32_t val) {
442442
// This is a temporary solution to bypass device api. Should be moved to CommandList once available in CUDA.
443443
program_impl_->fill_ndarray(ndarray->ndarray_alloc_,
444444
ndarray->get_nelement() * ndarray->get_element_size() / sizeof(uint32_t), val);
445+
// Host-issued device fill mutates the ndarray contents without going through a quadrants kernel, so
446+
// `bump_writes_for_kernel_*` will not cover it. Bump explicitly using the same `&ndarray_alloc_` key the
447+
// kernel launchers use, so any cached adstack-sizer metadata depending on this ndarray is evicted.
448+
adstack_cache_->bump_ndarray_data_gen(&ndarray->ndarray_alloc_);
445449
}
446450

447451
std::pair<const StructType *, size_t> Program::get_struct_type_with_data_layout(const StructType *old_ty,

tests/python/test_adstack.py

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2089,6 +2089,71 @@ def compute(a: qd.types.ndarray(dtype=qd.i32, ndim=1)):
20892089
assert x.grad[i] == pytest.approx(expected, rel=1e-5)
20902090

20912091

2092+
@test_utils.test(require=qd.extension.adstack)
2093+
def test_adstack_metadata_cache_invalidates_on_qd_ndarray_host_mutation():
2094+
# Pins per-task adstack metadata cache invalidation against host-side ndarray writes. A reverse-mode kernel
2095+
# whose inner trip count is `range(n[i])` over a `qd.ndarray` populates the cache on the first launch with
2096+
# `max_size = n[i]` evaluated against the current ndarray contents. A subsequent host-side mutation via
2097+
# `Ndarray::write` (no kernel involvement) must bump `ndarray_data_gen` so the next launch evicts the cache
2098+
# entry and re-runs the sizer against the new values.
2099+
#
2100+
# Internal details: the cache key on every backend is `(AdStackSizingInfo *, snode_write_gen, ndarray_data_gen)`.
2101+
# Without the host-mutation bump, the second launch sees the same key, returns the stale `max_size` for the
2102+
# previous ndarray contents, and the reverse pass walks the wrong number of inner iters per outer iteration.
2103+
# On Metal the symptom is heap reads from out-of-bounds slots that produce garbage gradients (e.g. `x.grad[k]`
2104+
# in the dozens instead of the analytical `0.8`); on CPU the host-eval path replays observed reads against the
2105+
# mutated ndarray and recovers without the explicit gen bump, so the test asserts gradient values rather than
2106+
# an overflow trap to catch the bug on every backend uniformly.
2107+
N = 4
2108+
N_X = 16
2109+
2110+
x = qd.field(qd.f32, shape=(N_X,), needs_grad=True)
2111+
loss = qd.field(qd.f32, shape=(), needs_grad=True)
2112+
2113+
@qd.kernel
2114+
def compute(n: qd.types.ndarray(dtype=qd.i32, ndim=1)):
2115+
for i_e in range(n.shape[0]):
2116+
accum = 0.0
2117+
for j in range(n[i_e]):
2118+
accum = accum + x[j] * x[j]
2119+
loss[None] += accum
2120+
2121+
n_arr = qd.ndarray(qd.i32, shape=(N,))
2122+
for i in range(N):
2123+
n_arr[i] = 8
2124+
2125+
for i in range(N_X):
2126+
x[i] = 0.1
2127+
loss[None] = 0.0
2128+
compute(n_arr)
2129+
loss.grad[None] = 1.0
2130+
for i in range(N_X):
2131+
x.grad[i] = 0.0
2132+
compute.grad(n_arr)
2133+
qd.sync()
2134+
assert loss[None] == pytest.approx(N * 8 * 0.01, rel=1e-5)
2135+
for k in range(8):
2136+
assert x.grad[k] == pytest.approx(N * 2 * 0.1, rel=1e-5)
2137+
for k in range(8, N_X):
2138+
assert x.grad[k] == pytest.approx(0.0, abs=1e-7)
2139+
2140+
for i in range(N):
2141+
n_arr[i] = 16
2142+
2143+
for i in range(N_X):
2144+
x[i] = 0.1
2145+
loss[None] = 0.0
2146+
compute(n_arr)
2147+
loss.grad[None] = 1.0
2148+
for i in range(N_X):
2149+
x.grad[i] = 0.0
2150+
compute.grad(n_arr)
2151+
qd.sync()
2152+
assert loss[None] == pytest.approx(N * 16 * 0.01, rel=1e-5)
2153+
for k in range(N_X):
2154+
assert x.grad[k] == pytest.approx(N * 2 * 0.1, rel=1e-5)
2155+
2156+
20922157
@test_utils.test(require=qd.extension.adstack)
20932158
def test_adstack_inner_range_bounded_by_multidim_ndarray_read():
20942159
# Pins multi-axis stride handling in the `ExternalTensorRead` evaluator. The sizer routes a reverse-mode inner trip

0 commit comments

Comments
 (0)