Skip to content

Commit bf6dce3

Browse files
AlbertDachiChenmeta-codesync[bot]
authored andcommitted
Fix int32 stride overflow in jagged_to_padded_dense at B*L*D > INT_MAX (#5755)
Summary: Pull Request resolved: #5755 X-link: https://github.com/facebookresearch/FBGEMM/pull/2686 Fixes a class of int32 stride-overflow bugs in fbgemm::jagged_to_padded_dense.forward (and the dense_to_jagged backward path that calls it). The kernel jagged_dense_elementwise_dense_output_kernel_ in common.cuh declares its dense `y` and `output` parameters as PackedTensorAccessor32. PTA32 stores strides as int32, so `output[oidx][jidx][iidx]` lowers to `oidx * strides_[0]` in int32. When the dense tensor's numel exceeds INT_MAX (specifically when (B - 1) * max_L * D > INT_MAX), the multiply wraps negative, writes go outside the destination buffer, and the high-oidx rows of the at::empty_symint allocation stay unwritten. Because the outer-loop bound B * max_L need not itself overflow, FBGEMM_LAUNCH_KERNEL's grid sanity check does not fire - the kernel runs to completion silently, and the unwritten memory surfaces as NaN / garbage downstream. Fix (narrow, contained to a single kernel + its launcher): - Template walk_down_tensor_storage_tree_ on a new PosT type. The existing int callers continue to work via auto template deduction; the int64 path can now pass int64_t. - Template jagged_dense_elementwise_dense_output_kernel_ on a new IdxT used both as the PackedTensorAccessor stride width and as the type of the loop / index variables (outer, oidx, jidx, offset, iidx). This keeps the int32 fast path register-width consistent and lets the int64 specialization avoid all int32 wrap points. - INVOKE_KERNEL_WITH_DIM in jagged_dense_elementwise_dense_output_ now branches on whether x_values, y_reshaped, and output_reshaped all have numel < INT_MAX, dispatching either the existing PTA32+int32_t fast path (unchanged) or a new PTA64+int64_t specialization. Workloads that fit in int32 pay zero extra cost; only over-INT_MAX workloads pay the 64-bit indexing tax. Scope intentionally limited: - Does NOT touch the sibling jagged_dense_dense_elementwise_jagged_output_kernel_ or its INVOKE_KERNEL_WITH_DIM (in this file or in jagged_dense_dense_elementwise_add_jagged_output_forward.cu). - Does NOT touch check_shape_and_partition_, jagged_dense_elementwise_mul_backward.cu, jagged_to_padded_dense_forward.cu, jagged_tensor_ops_autograd.cpp, or cuda_prelude.cuh. - The kernels and call sites that ARE NOT modified retain their previous PTA32/int32 behavior unchanged. They have the same hazard at sufficiently large shapes; addressing them is out of scope for this diff and can be tracked separately if a need arises. Context: this bug class was identified during investigation of T264042859 (NaN at pos_encoding.grad in Instagram Reels MTML training at 20k UIH). For the IG Reels prod shapes (B_RO=1280, uih_max_len=20480, pe_emb_dim=64 - numel ~ 1.68B, below INT_MAX), this fix does NOT change kernel behavior - the int32 fast path is still selected. The IG Reels prod failure was resolved separately by D99880049, which avoids the dense_to_jagged path entirely with a model-side gather refactor. This diff lands the kernel-level fix as defense-in-depth against future callers whose shapes do cross INT_MAX (e.g. pe_emb_dim scaled to 128, or any other dense_to_jagged-on-broadcast pattern at scale). Reviewed By: q10 Differential Revision: D104247303 fbshipit-source-id: f27c91b6c17b529c8b577f3822ef85ac675cfa10
1 parent bd907c4 commit bf6dce3

2 files changed

Lines changed: 196 additions & 57 deletions

File tree

fbgemm_gpu/src/jagged_tensor_ops/common.cuh

Lines changed: 105 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -83,18 +83,22 @@ namespace {
8383
* @returns true if the flattend jagged idx points to zero'ed (masked out)
8484
* portion of the jagged tensor
8585
*/
86-
template <int NUM_JAGGED_DIM, typename index_t>
86+
// `PosT` is the position/offset type used for indexing into the jagged
87+
// storage tree. Templating it lets the int64 specialization of the calling
88+
// kernel pass int64_t while existing int callers keep their int32 behavior
89+
// via auto template deduction (no callsite change needed).
90+
template <int NUM_JAGGED_DIM, typename PosT, typename index_t>
8791
DEVICE_INLINE bool walk_down_tensor_storage_tree_(
88-
int& offset,
89-
const int flattened_jagged_idx,
92+
PosT& offset,
93+
const PosT flattened_jagged_idx,
9094
const StackArray<int64_t>& jagged_dims,
9195
const StackArray<index_t*>& x_offsets) {
9296
// compute coorindates
93-
int jagged_coords[NUM_JAGGED_DIM];
94-
int j_temp = flattened_jagged_idx;
97+
PosT jagged_coords[NUM_JAGGED_DIM];
98+
PosT j_temp = flattened_jagged_idx;
9599
#pragma unroll
96100
for (int d = NUM_JAGGED_DIM - 1; d >= 0; --d) {
97-
const int jagged_size = jagged_dims.vals[d];
101+
const PosT jagged_size = jagged_dims.vals[d];
98102
jagged_coords[d] = j_temp % jagged_size;
99103
j_temp /= jagged_size;
100104
}
@@ -103,8 +107,8 @@ DEVICE_INLINE bool walk_down_tensor_storage_tree_(
103107
bool is_zero = false;
104108
#pragma unroll
105109
for (int d = 0; d < NUM_JAGGED_DIM; ++d) {
106-
const int begin = x_offsets.vals[d][offset];
107-
const int end = x_offsets.vals[d][offset + 1];
110+
const PosT begin = x_offsets.vals[d][offset];
111+
const PosT end = x_offsets.vals[d][offset + 1];
108112
if (jagged_coords[d] >= end - begin) {
109113
is_zero = true;
110114
break;
@@ -128,34 +132,47 @@ DEVICE_INLINE bool walk_down_tensor_storage_tree_(
128132
// blockDim.x so the inner dense dimension should be similar to or bigger than
129133
// warp size.
130134
// We rely on compiler unrolling the compiler time constant NUM_JAGGED_DIM.
131-
template <int NUM_JAGGED_DIM, typename index_t, typename scalar_t, typename F>
135+
// `IdxT` is the index type used for the PackedTensorAccessor stride width
136+
// and outer-loop counter. The launcher picks int32_t for the common fast
137+
// path and int64_t when (B - 1) * (max_L * D) > INT_MAX, where the int32
138+
// PackedTensorAccessor would otherwise wrap `oidx * strides_[0]` and
139+
// scribble outside the destination buffer (T264042859). Loop / index
140+
// variables stay in IdxT so the compiler picks consistent register width
141+
// for the int32 fast path.
142+
template <
143+
int NUM_JAGGED_DIM,
144+
typename IdxT,
145+
typename index_t,
146+
typename scalar_t,
147+
typename F>
132148
__global__
133149
__launch_bounds__(kMaxThreads) void jagged_dense_elementwise_dense_output_kernel_(
134-
const pta::PackedTensorAccessor32<scalar_t, 2, at::RestrictPtrTraits>
150+
const pta::PackedTensorAccessor<scalar_t, 2, at::RestrictPtrTraits, IdxT>
135151
x_values,
136152
StackArray<index_t*> x_offsets,
137-
const pta::PackedTensorAccessor32<scalar_t, 3, at::RestrictPtrTraits> y,
138-
pta::PackedTensorAccessor32<scalar_t, 3, at::RestrictPtrTraits> output,
153+
const pta::PackedTensorAccessor<scalar_t, 3, at::RestrictPtrTraits, IdxT> y,
154+
pta::PackedTensorAccessor<scalar_t, 3, at::RestrictPtrTraits, IdxT> output,
139155
StackArray<int64_t> jagged_dims,
140156
F f,
141157
const scalar_t padding_value) {
142-
const int outer_dense_size = y.size(0);
143-
const int jagged_folded_size = y.size(1);
144-
const int inner_dense_size = y.size(2);
145-
146-
const auto outer_begin = blockIdx.x * blockDim.y + threadIdx.y;
147-
const auto outer_stride = gridDim.x * blockDim.y;
148-
for (int outer = outer_begin; outer < outer_dense_size * jagged_folded_size;
149-
outer += outer_stride) {
150-
const int oidx = outer / jagged_folded_size;
151-
const int jidx = outer % jagged_folded_size;
152-
153-
int offset = oidx;
158+
const IdxT outer_dense_size = y.size(0);
159+
const IdxT jagged_folded_size = y.size(1);
160+
const IdxT inner_dense_size = y.size(2);
161+
162+
const IdxT outer_begin =
163+
static_cast<IdxT>(blockIdx.x) * blockDim.y + threadIdx.y;
164+
const IdxT outer_stride = static_cast<IdxT>(gridDim.x) * blockDim.y;
165+
const IdxT total_outer = outer_dense_size * jagged_folded_size;
166+
for (IdxT outer = outer_begin; outer < total_outer; outer += outer_stride) {
167+
const IdxT oidx = outer / jagged_folded_size;
168+
const IdxT jidx = outer % jagged_folded_size;
169+
170+
IdxT offset = oidx;
154171
const bool is_zero = walk_down_tensor_storage_tree_<NUM_JAGGED_DIM>(
155172
offset, jidx, jagged_dims, x_offsets);
156173

157174
if (is_zero) {
158-
int iidx;
175+
IdxT iidx;
159176
for (iidx = threadIdx.x; iidx * 2 + 1 < inner_dense_size;
160177
iidx += blockDim.x) {
161178
output[oidx][jidx][2 * iidx] =
@@ -168,7 +185,7 @@ __launch_bounds__(kMaxThreads) void jagged_dense_elementwise_dense_output_kernel
168185
f(padding_value, y[oidx][jidx][2 * iidx]);
169186
}
170187
} else {
171-
int iidx;
188+
IdxT iidx;
172189
for (iidx = threadIdx.x; iidx * 2 + 1 < inner_dense_size;
173190
iidx += blockDim.x) {
174191
output[oidx][jidx][2 * iidx] =
@@ -256,35 +273,68 @@ void jagged_dense_elementwise_dense_output_(
256273
const Tensor y_reshaped = y.view({y.size(0), -1, y.size(-1)});
257274
Tensor output_reshaped = output.view(y_reshaped.sizes());
258275

259-
#define INVOKE_KERNEL_WITH_DIM(NUM_JAGGED_DIM) \
260-
{ \
261-
std::vector<Tensor> x_offsets_contig; \
262-
x_offsets_contig.resize(num_jagged_dim); \
263-
StackArray<index_t*> x_offset_ptrs; \
264-
x_offset_ptrs.ndim = num_jagged_dim; \
265-
for (int d = 0; d < num_jagged_dim; ++d) { \
266-
x_offsets_contig[d] = x_offsets[d].contiguous(); \
267-
x_offset_ptrs.vals[d] = \
268-
x_offsets_contig[d].template data_ptr<index_t>(); \
269-
} \
270-
\
271-
FBGEMM_LAUNCH_KERNEL( \
272-
(jagged_dense_elementwise_dense_output_kernel_< \
273-
NUM_JAGGED_DIM, \
274-
index_t, \
275-
scalar_t, \
276-
F>), \
277-
blocks, \
278-
threads, \
279-
0, \
280-
at::cuda::getCurrentCUDAStream(), \
281-
PTA_B(x_values, scalar_t, 2, 32), \
282-
x_offset_ptrs, \
283-
PTA_B(y_reshaped, scalar_t, 3, 32), \
284-
PTA_B(output_reshaped, scalar_t, 3, 32), \
285-
jagged_dims_tensor, \
286-
f, \
287-
padding_value); \
276+
#define INVOKE_KERNEL_WITH_DIM(NUM_JAGGED_DIM) \
277+
{ \
278+
std::vector<Tensor> x_offsets_contig; \
279+
x_offsets_contig.resize(num_jagged_dim); \
280+
StackArray<index_t*> x_offset_ptrs; \
281+
x_offset_ptrs.ndim = num_jagged_dim; \
282+
for (int d = 0; d < num_jagged_dim; ++d) { \
283+
x_offsets_contig[d] = x_offsets[d].contiguous(); \
284+
x_offset_ptrs.vals[d] = \
285+
x_offsets_contig[d].template data_ptr<index_t>(); \
286+
} \
287+
\
288+
/* Pick int32 fast path when every touched tensor's numel fits in int32; \
289+
* otherwise dispatch the int64 specialization. The int64 path defends \
290+
* against `oidx * strides_[0]` overflowing int32 inside \
291+
* PackedTensorAccessor32, which scribbles outside the buffer for high \
292+
* oidx and surfaces as silent NaN downstream (T264042859). */ \
293+
constexpr int64_t kInt32Limit = \
294+
static_cast<int64_t>(std::numeric_limits<int32_t>::max()); \
295+
const bool use_int32_indexing = x_values.numel() < kInt32Limit && \
296+
y_reshaped.numel() < kInt32Limit && \
297+
output_reshaped.numel() < kInt32Limit; \
298+
\
299+
if (use_int32_indexing) { \
300+
FBGEMM_LAUNCH_KERNEL( \
301+
(jagged_dense_elementwise_dense_output_kernel_< \
302+
NUM_JAGGED_DIM, \
303+
int32_t, \
304+
index_t, \
305+
scalar_t, \
306+
F>), \
307+
blocks, \
308+
threads, \
309+
0, \
310+
at::cuda::getCurrentCUDAStream(), \
311+
PTA_B(x_values, scalar_t, 2, 32), \
312+
x_offset_ptrs, \
313+
PTA_B(y_reshaped, scalar_t, 3, 32), \
314+
PTA_B(output_reshaped, scalar_t, 3, 32), \
315+
jagged_dims_tensor, \
316+
f, \
317+
padding_value); \
318+
} else { \
319+
FBGEMM_LAUNCH_KERNEL( \
320+
(jagged_dense_elementwise_dense_output_kernel_< \
321+
NUM_JAGGED_DIM, \
322+
int64_t, \
323+
index_t, \
324+
scalar_t, \
325+
F>), \
326+
blocks, \
327+
threads, \
328+
0, \
329+
at::cuda::getCurrentCUDAStream(), \
330+
PTA_B(x_values, scalar_t, 2, 64), \
331+
x_offset_ptrs, \
332+
PTA_B(y_reshaped, scalar_t, 3, 64), \
333+
PTA_B(output_reshaped, scalar_t, 3, 64), \
334+
jagged_dims_tensor, \
335+
f, \
336+
padding_value); \
337+
} \
288338
}
289339

290340
JAGGED_TENSOR_DISPATCH_DIMS();

fbgemm_gpu/test/jagged/dense_to_jagged_test.py

Lines changed: 91 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,14 @@
1919

2020
if open_source:
2121
# pyre-ignore[21]
22-
from test_utils import cpu_and_maybe_gpu, gpu_unavailable, optests
22+
from test_utils import cpu_and_maybe_gpu, gpu_memory_lt_gb, gpu_unavailable, optests
2323
else:
24-
from fbgemm_gpu.test.test_utils import cpu_and_maybe_gpu, gpu_unavailable, optests
24+
from fbgemm_gpu.test.test_utils import (
25+
cpu_and_maybe_gpu,
26+
gpu_memory_lt_gb,
27+
gpu_unavailable,
28+
optests,
29+
)
2530

2631

2732
@optests.generate_opcheck_tests(additional_decorators=additional_decorators)
@@ -160,6 +165,90 @@ def test_dense_to_jagged_opt_large_batch(
160165
precompute_total_L,
161166
)
162167

168+
@optests.dontGenerateOpCheckTests("regression test, not an op-shape check")
169+
@unittest.skipIf(*gpu_unavailable)
170+
@unittest.skipIf(*gpu_memory_lt_gb(16))
171+
def test_dense_to_jagged_backward_int32_overflow(self) -> None:
172+
"""Regression: int32 overflow in dense_to_jagged backward at
173+
B * max_L * D > INT_MAX, even when B * max_L itself fits in int32.
174+
175+
dense_to_jagged.backward dispatches to fbgemm::jagged_to_padded_dense
176+
forward, which fills a dense (B, max_L, D) gradient using
177+
jagged_dense_elementwise_dense_output_kernel_ in common.cuh. Pre-fix,
178+
that kernel uses PackedTensorAccessor32 for the dense `y` and
179+
`output` parameters: stride[0] is stored as int32 and the access
180+
`output[oidx][jidx][iidx]` lowers to `oidx * stride[0]` in int32.
181+
For shapes where (B - 1) * (max_L * D) > INT_MAX the multiply wraps
182+
negative and writes go to addresses outside the destination buffer.
183+
Outer-loop bound and grid sizing are unaffected (B * max_L need not
184+
overflow), so there is no FBGEMM_LAUNCH_KERNEL grid abort - the
185+
kernel runs to completion silently writing wrong memory, leaving
186+
~25% of high-oidx rows of the dense gradient at the at::empty bit
187+
pattern. Downstream NaN. (T264042859, observed in production at
188+
Instagram Reels MTML pos_encoding training.)
189+
190+
Workload: B=1024, max_L=40960, D=64, bf16. B * max_L = 42M < INT_MAX
191+
(so no outer-loop / grid hazard), B * max_L * D ~ 2.68B > INT_MAX
192+
(so PTA32 stride math wraps for oidx >= 819, i.e. the last ~25% of
193+
batches). Peak GPU memory ~ B * max_L * D * 2 bytes ~ 5 GB.
194+
195+
The single jagged value of 1.0 is placed in the LAST batch (at
196+
oidx = B - 1 = 1023, well into the wrap zone). On unfixed code the
197+
write to out[1023, 0, :] never happens; it stays at uninitialized
198+
memory. After the fix routes this shape to the int64 (PTA64) kernel
199+
path the write lands correctly.
200+
"""
201+
device = torch.accelerator.current_accelerator()
202+
B = 1024
203+
max_L = 40960
204+
D = 64
205+
dtype = torch.bfloat16
206+
assert B * max_L < (1 << 31), (
207+
"test must keep B * max_L below INT_MAX (we are testing the PTA32 "
208+
"stride-overflow path, not the outer-loop overflow path)"
209+
)
210+
assert (
211+
B * max_L * D > (1 << 31) - 1
212+
), "test must exceed INT_MAX on B * max_L * D"
213+
# PTA32 stride wraps for oidx >= ceil(INT_MAX / (max_L * D)).
214+
wrap_oidx = (((1 << 31) - 1) + (max_L * D) - 1) // (max_L * D)
215+
assert (
216+
B - 1 >= wrap_oidx
217+
), "last batch must fall inside the PTA32 stride-overflow zone"
218+
219+
# Tiny jagged: only the LAST batch has length=1, all others empty.
220+
# That keeps total_L = 1 (and thus the (total_L, D) values tensor
221+
# microscopic) so the only large allocation is the (B, max_L, D)
222+
# padded output.
223+
offsets = torch.zeros(B + 1, dtype=torch.long, device=device)
224+
offsets[B] = 1
225+
grad_jagged = torch.ones((1, D), dtype=dtype, device=device)
226+
227+
# Exact code path that dense_to_jagged.backward executes.
228+
out = torch.ops.fbgemm.jagged_to_padded_dense(
229+
grad_jagged, [offsets], [max_L], padding_value=0.0
230+
)
231+
232+
self.assertEqual(tuple(out.shape), (B, max_L, D))
233+
234+
# The strongest signal: the in-range position MUST equal the jagged
235+
# value. On unfixed code oidx=B-1 falls in the PTA32 wrap zone and
236+
# the write goes to a wrong address; this position stays as
237+
# whatever at::empty left in memory - anything but 1.0.
238+
self.assertEqual(out[B - 1, 0, 0].item(), 1.0)
239+
self.assertEqual(out[B - 1, 0, D - 1].item(), 1.0)
240+
241+
# Spot-check padding inside the wrap zone (oidx=B-1) and outside
242+
# it (oidx=0). 0.0 here means the kernel correctly wrote
243+
# padding_value; an unwritten cell would surface as garbage / NaN
244+
# after the autograd reduce.
245+
for pos in [1, max_L // 2, max_L - 1]:
246+
self.assertEqual(out[B - 1, pos, 0].item(), 0.0)
247+
self.assertEqual(out[0, pos, 0].item(), 0.0)
248+
# Spot-check a batch right at the wrap boundary.
249+
self.assertEqual(out[wrap_oidx, 0, 0].item(), 0.0)
250+
self.assertEqual(out[wrap_oidx, max_L - 1, 0].item(), 0.0)
251+
163252
@given(
164253
num_jagged_dim=st.integers(1, 5),
165254
outer_dense_size=st.integers(0, 5),

0 commit comments

Comments
 (0)