Cast tl.program_id to int64 in llama4_rope and qwen2vl_mrope - #1342
Cast tl.program_id to int64 in llama4_rope and qwen2vl_mrope#1342adityasingh2400 wants to merge 8 commits into
Conversation
Both kernels index the flattened batch*seq token dimension with a raw tl.program_id(0), which Triton materializes as int32. The resulting pointer arithmetic (base_offset * q_row_stride in llama4_rope, pid * (n_qh * hd) in qwen2vl_mrope) overflows once the q tensor exceeds 2**31 elements, which is the same out-of-bounds class that linkedin#804 fixed in rope.py and rms_norm.py. Applies the identical .to(tl.int64) widening. llama4_rope.py was added after linkedin#804 as a fresh kernel body rather than reusing _triton_rope, so it never inherited the fix, and qwen2vl_mrope.py predates linkedin#804 entirely. The Ascend port of qwen2vl_mrope has the same gap, widened at the pid used for addressing so loop control stays int32. Fixes linkedin#1335
|
Updated the branch onto Worth flagging that no CI has run on this yet. All four workflows have been sitting at
Approving the workflow run is all it needs to produce a signal. |
Tcc0403
left a comment
There was a problem hiding this comment.
Could you help adding a mentioned test case to existing test suite?
|
Added in One thing you should know before deciding whether to keep it, because it constrains what a real test can look like. The kernel indexes with At bfloat16 that is 4.3 GB for The test writes into the last token only, since that is the row whose offset wraps, then checks the output is finite, nonzero, and matches Worth noting that #804, which made the same int64 change in Separately, CI still has not run on this branch. All four workflows have been at |
| # weird shapes | ||
| (3, 423, 73, 213, 92), | ||
| (3, 423, 73, 155, 92), |
There was a problem hiding this comment.
can you add the overflow test case as a pytest.param with marks=pytest.mark.skipif total memory less than 20GB?
there's a util func you can reuse for this purpose
Liger-Kernel/src/liger_kernel/utils.py
Line 202 in a3f3654
if there's any concern, feel free to chat about it!
There was a problem hiding this comment.
Done in 1b120b9, using get_total_gpu_memory as you suggested.
pytest.param(
1, _SEQ_LEN, _N_Q_HEADS, 1, _HEAD_DIM,
marks=pytest.mark.skipif(
infer_device() == "cpu" or get_total_gpu_memory() < 20,
reason="This test requires a GPU with at least 20GB of memory",
),
),One thing I added on purpose. The mark is evaluated at collection time, and get_total_gpu_memory raises RuntimeError when infer_device() returns cpu, so calling it unguarded would fail collection for the whole file on a cpu only machine. The or short circuits before that. I checked it locally where infer_device() is cpu and the util is never reached.
The test body now takes the parametrized values rather than the module constants, so the shape lives entirely in the param. ruff passes.
Summary
llama4_rope.pyandqwen2vl_mrope.pyindex the flattenedbatch * seqtoken dimension with a rawtl.program_id(0). Triton materializes that as int32, so the pointer arithmetic built on top of it wraps once the offset passes2**31. This is the same out-of-bounds class that #803 reported and #804 fixed inrope.pyandrms_norm.py, and neither of these two files was touched by that PR.llama4_rope.pywas added afterwards as a fresh kernel body rather than reusing_triton_rope, so it never inherited the fix, andqwen2vl_mrope.pypredates #804.Fixes #1335
Details
Where it overflows.
llama4_rope.pylaunches on a(batch_size * seq_len, n_heads_max)grid and passesq.stride(1)asq_row_stride, which for the contiguous(B, S, H, D)layout isH * D. The kernel then computes:so the largest element offset is
(B*S - 1) * H * D, one row short ofnumel(q). The int32 product wraps once that passes int32 max, which as a token budget for a single call meanstotal_tokens > int32_max / (H * D):H * D)Using #803's own long-context motivation,
B=4, S=420000, H=40, D=128gives1679999 * 5120 = 8601594880, four times past int32 max. Nothing that extreme is required though. A 64-head model atB=8, S=65536reaches4294959104, already twice over.qwen2vl_mrope.pyhas the same shape of arithmetic,q_ptr + pid * (n_qh * hd)withpidup toB*S - 1, so the same token thresholds apply.The per-head term is safe in both and is left alone.
pid_h * q_head_strideis bounded byn_heads * head_dim, and widening the batch term promotes the sum anyway.The fix is the exact pattern used by #804 and by the Ascend port of
llama4_ropefrom #1053, which already carriestl.program_id(0).to(tl.int64).Ascend backend.
backends/_ascend/ops/qwen2vl_mrope.pyhas the same gap, which #1335 also flags. That kernel derives its token index inside a loop rather than straight from the program id, so I widened atpid, the value actually used for addressing, and leftstart_row,rows_per_programandactual_rowsas int32 so the loop bound andtl.rangeare unchanged.Cost is one
.to(tl.int64)on a single scalar per program, the same tradeoff #804 accepted for the two hottest kernels in the repo.Related, but deliberately not included.
_block_rms_norm_forward_kernelinrms_norm.py, the blocked forward added after #804, computesrow_idx = tl.program_id(0) * BLOCK_ROW + tl.arange(0, BLOCK_ROW)with no cast, while_block_rms_norm_backward_kernelin the same file does carrytl.program_id(0).cast(tl.int64). That path is gated ton_cols <= 256, so it needs on the order of 8.4M rows to wrap, far beyond what the rope kernels need. I kept it out to hold this PR to the scope of #1335. Glad to fold it in here or send it separately, whichever you prefer.Testing Done
I do not have GPU access, so I have not run the Triton suites, and I would rather say that than tick boxes I cannot honestly tick.
What I did verify:
ruff checkandruff format --checkpass on all three files.tl.program_idundersrc/liger_kernel/ops/to confirm these are the token-indexing kernels still missing the cast.The change is a pure type widening. It is the identity for every offset below
2**31, so existing shapes stay bit-identical, and reproducing the actual illegal memory access needs a q tensor above 4 GiB, which is not CI-runnable. #804 landed under the same constraint and added no test for the same reason.make testto ensure correctnessmake checkstyleto ensure code stylemake test-convergenceto ensure convergence