Skip to content

Commit ac130ed

Browse files
committed
mapreduce: add exact-arithmetic regression test for strided mixed reductions
Follow-up to the #7 resolution, addressing the adversarial-review caveats: the prior comment overstated coalescing for a small leading dim, and the empirical sweep left nothing committed and used Float32 (which can pass or fail for the wrong reason on large sums). Tighten the comment to the precise mechanism (the miscompile needs adjacent lanes a *full* stride apart on every lane; reducing dim 1 keeps a contiguous innermost axis, so lanes read stride-1 within each length-n1 run) and add an Int32 regression test covering dims=(1,3)/(1,4)/(1,2,3) plus the coalesced dims=2/3 cases. Int32 is exact and associative, so it is immune to the Float32 accumulation-order rounding that made a stress sweep of >2^24-length sums look corrupted when it was not; verified all cases match the CPU exactly, including small leading dims and large strided sizes. Refines PR_REVIEW.md finding #7.
1 parent df6c1a4 commit ac130ed

2 files changed

Lines changed: 33 additions & 7 deletions

File tree

src/mapreduce.jl

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -199,13 +199,18 @@ function GPUArrays.mapreducedim!(f::F, op::OP, R::oneWrappedArray{T},
199199
# reductions get less parallelism but stay correct; the common many-slice case is also fast.
200200
#
201201
# `size(Rreduce, 1) == 1` (i.e. dim 1 kept) is the correct predicate, not just a special
202-
# case: in `partial_mapreduce_device` consecutive work-item lanes step through consecutive
203-
# `Rreduce` entries, whose *first* axis is dim 1. When dim 1 is among the reduced dims the
204-
# innermost reduced axis is contiguous, so those lane reads coalesce and the result is
205-
# correct even if a strided dim (e.g. dim 3) is *also* reduced — the miscompile only bites
206-
# when the innermost reduced axis is itself strided (dim 1 kept). Empirically verified: a
207-
# sweep of `dims=(1,3)`-style mixed reductions with small leading dims (n1 ∈ 2..7) matched
208-
# the CPU exactly, so no extra materialization is needed for those (see PR_REVIEW.md #7).
202+
# case. In `partial_mapreduce_device` adjacent work-item lanes step through adjacent
203+
# `Rreduce` entries, whose *fastest* axis is array dim 1. The miscompile bites only when
204+
# adjacent lanes land a *full stride* apart on every lane — i.e. when the innermost reduced
205+
# axis is itself strided, which is exactly the dim-1-kept case routed to the coalesced
206+
# kernel above. When dim 1 is among the reduced dims, adjacent lanes instead read
207+
# consecutive memory (stride 1) within each length-n1 run, with only an occasional jump at
208+
# a run boundary — not the every-lane full-stride pattern — so the reads stay well-formed
209+
# even when a strided dim (e.g. dim 3) is *also* reduced. Verified with exact Int32
210+
# reductions (immune to the accumulation-order rounding that makes >2^24-length Float32
211+
# sums differ between GPU and CPU): `dims=(1,3)`, `(1,4)` and full `(1,2,3)` reductions at
212+
# large strided sizes match the CPU exactly, so these need no extra materialization (see
213+
# PR_REVIEW.md #7 and the "strided mixed reductions" regression test).
209214
if oneL0.LTS[] && size(Rreduce, 1) == 1
210215
# cap the group size at what this kernel actually supports on this device (queried
211216
# from the driver), rather than a hardcoded 256 that can exceed the kernel's max

test/array.jl

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,3 +104,24 @@ end
104104
resize!(b, 1)
105105
@test length(b) == 1
106106
end
107+
108+
@testset "strided mixed reductions" begin
109+
# The Aurora LTS IGC miscompiles a reduction kernel's global reads when the *innermost*
110+
# reduced axis is strided (dim 1 kept, e.g. `dims=2`); mapreducedim! routes those to a
111+
# coalesced kernel. Reductions that also reduce dim 1 (e.g. `dims=(1,3)`) keep a contiguous
112+
# innermost axis and stay correct on the workgroup-per-slice kernel — including with a small
113+
# leading dim, where the contiguous run is short. Use Int32 (exact, associative) so the
114+
# check is immune to Float32 accumulation-order rounding. Regression for PR_REVIEW.md #7.
115+
for (sz, dts) in (
116+
((2, 512, 64), ((1, 3), (2,), (3,), (2, 3), (1, 2, 3), 1)),
117+
((3, 256, 48), ((1, 3), (2,), (1, 2, 3))),
118+
((7, 300, 20), ((1, 3), (2,), (3,))),
119+
((2, 128, 8, 32), ((1, 3), (1, 4), (2, 4), (1, 2, 4))),
120+
)
121+
A = rand(Int32(1):Int32(4), sz...)
122+
dA = oneArray(A)
123+
for dt in dts
124+
@test Array(sum(dA; dims = dt)) == sum(A; dims = dt)
125+
end
126+
end
127+
end

0 commit comments

Comments
 (0)