Repository navigation
cuda: reserve the f16 K/V scratch for the short-query MMA route (#346) - #350
professorpalmer wants to merge 1 commit into
Conversation
…mML-Eng#346) With a quantized V and 3-4 queries, ggml_cuda_flash_attn_ext_mma_f16_switch_ncols1 takes the 64-column f16 tiles (PrismML-Eng#189), and launch_fattn converts K and V to f16 in the op's extra buffer. ggml_cuda_flash_attn_ext_get_alloc_size reserved nothing beyond dst when the in-place quantized K/V kernel exists for the head size, so that conversion wrote past the op's allocation. Server prompt checkpoints evaluate the last 4 prompt tokens as their own batch, which reaches this route. - ggml_cuda_fattn_mma_short_query_f16(): the route's condition, used by the dispatch and by get_alloc_size. - get_alloc_size reserves the f16 K/V for that route also on the in-place path. - The MMA dispatch asserts that the buffer reserved the f16 K/V the route writes, so a mismatch aborts with a message instead of corrupting memory.
|
Closing: #355 fixes the same root cause in the base. #355 passes Check on the current base ( One note: on MSVC, Thanks to @Powerful-Plantain for the reproduction and the memcheck runs on #346. |
Overview
Fixes #346: an out-of-bounds write in
launch_fattnwith quantized K/V when prompt checkpoints are on.Cause. With a quantized V and 3-4 queries,
ggml_cuda_flash_attn_ext_mma_f16_switch_ncols1takes the 64-column f16 tiles (#189). That route callsggml_cuda_flash_attn_ext_mma_f16_casewithouttype_KV, solaunch_fattnconverts K and V to f16 in the op's extra buffer, also when the in-place quantized K/V kernel exists for the head size (q4_0 / q8_0, D = 128 / 256, from #221).ggml_cuda_flash_attn_ext_get_alloc_sizereserves nothing beyonddston that in-place path, so the f16 K/V is written past the op's allocation. Server prompt checkpoints evaluate the last 4 prompt tokens as their own batch, which reaches this route; with--ctx-checkpoints 0it is not reached. This matches the memcheck trace in #346 (dequantize_block_q4_0fromlaunch_fattn,ggml_cuda_flash_attn_ext_mma_f16_case<256, 256, 8, 8, F16>).Change (
ggml/src/ggml-cuda/fattn.cuonly):ggml_cuda_fattn_mma_short_query_f16(): the route's condition, now used by the dispatch and byget_alloc_size.get_alloc_sizereserves the f16 K/V for that route also on the in-place path.The #189 routing itself is unchanged.
Additional information
RTX 4070 (sm_89), CUDA 13, Windows, on
prism-b10770-6684606(the build in #346) with the same change:FLASH_ATTN_EXT(hsk=256,hsv=256,nh=4,nr23=[6,1],kv=16384,nb=3,...,type_K=q4_0,type_V=q4_0)test-backend-ops -o FLASH_ATTN_EXT -b CUDA0get_alloc_sizechange)GGML_ASSERT ... f16 K/V scratch not reserved for the short-query routeOn Ada the overflow did not fault in my earlier repro attempts (the write lands in memory the process owns), so the assert is what makes it visible here. @Powerful-Plantain, a test of this branch on your 3070 setup (checkpoints on, same command) would confirm it on the card that crashed.
Requirements