fix low accuracy of Qwen3.5-27B/Qwen3.6-27B#437
Conversation
- Add feat_valid guards to chunk_causal_conv1d_tiled_xe2 Phase 2 and 3 to prevent out-of-bounds writes - Add conv_elems bounds checks in causal_conv1d update_states_kernel and chunk_causal_conv1d_xe2 chunk_update_states_kernel to guard the over-provisioned last work-group when conv_elems is not a multiple of elems_per_group Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: huanxing <huanxing.shen@intel.com>
5952fc5 to
39c5284
Compare
There was a problem hiding this comment.
Pull request overview
This PR cherry-picks a set of XE2 GDN-attention kernel fixes intended to address severe accuracy regressions for Qwen3.5-27B (and related models) on B60, especially when max_model_len >= 32K.
Changes:
- Refactors
chunk_prepare_kernelwork partitioning across(v_head, chunk)tasks and adds per-sequence last-chunk handling. - Adds bounds checks to conv-state update kernels to prevent out-of-range writes when
conv_elemsisn’t an even multiple of the copy tiling. - Removes a barrier-divergent early return in the tiled conv1d kernel by guarding work with
feat_validinstead, ensuring all work-items participate in required barriers.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| csrc/xpu/gdn_attn/xe_2/chunk_gated_delta_rule_kernels_xe2.hpp | Updates chunk preparation scheduling and partial-chunk handling for GDN attention. |
| csrc/xpu/gdn_attn/xe_2/chunk_causal_conv1d_xe2.hpp | Adds i < conv_elems guard to avoid out-of-bounds conv-state copies. |
| csrc/xpu/gdn_attn/xe_2/chunk_causal_conv1d_tiled_xe2.hpp | Replaces barrier-divergent early return with feat_valid-guarded computation and writes. |
| csrc/xpu/gdn_attn/causal_conv1d.hpp | Adds i < conv_elems guard to avoid out-of-bounds conv-state copies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| int pre_chunks = 0; | ||
| int current_chunk_size = chunk_size; | ||
| for (int b_id = 0; b_id < batch_size; ++b_id) { | ||
| const int seq_start_offset = query_start_loc[b_id]; | ||
| const int seq_end_offset = query_start_loc[b_id + 1]; |
| for (int c = local_num - 1; c >= 0; --c) { | ||
| const_cast<float*>(a) | ||
| [(chunk_start_offset + sg_local_id * local_num + c) + | ||
| v_head_id * total_virtual_seqlen] = g_local_sum; | ||
| g_local_sum -= g_local[c]; |
The nested loop computed v_head_id = total_sg_id / chunk_range where chunk_range = total_sg_range / num_v_heads (integer division). When total_sg_range is not divisible by num_v_heads (e.g. 640 / 12 = 53), orphan sub-groups get v_head_id >= num_v_heads, causing OOB reads from A_log[] and OOB writes to a[], corrupting memory and producing NaN. Replace with a flattened task_id loop that naturally bounds-checks via task_id < total_tasks (num_v_heads * total_chunks). This fixes GSM8K accuracy drop to 0 for Qwen3.5-27B TP=4 on BMG/B60. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: huanxing <huanxing.shen@intel.com>
|
This is a updated PR based #435. |
| const int total_chunks = total_virtual_seqlen / chunk_size; | ||
| const int total_tasks = num_v_heads * total_chunks; | ||
|
|
||
| for (int task_id = total_sg_id; task_id < total_tasks; task_id += total_sg_range) { |
There was a problem hiding this comment.
Is the change in this file necessary?
There was a problem hiding this comment.
It is to fix the gsm8k=0 when max_model_len = 32K.
Look at the chunk_prepare_kernel on our cherry-pick branch (line 162-165 equivalent):
const int chunk_range = total_sg_range / num_v_heads; // integer division!
int chunk_id = total_sg_id % chunk_range;
const int v_head_id = total_sg_id / chunk_range; // ← CAN EXCEED num_v_heads!
For Qwen3.5-27B TP=4: num_v_heads = 12
With B60 (say sm_count=20): total_sg_range = 20 × 32 = 640
- chunk_range = 640 / 12 = 53 (integer division)
- Max valid: 12 × 53 - 1 = 635
- Sub-groups 636–639 get v_head_id = 12 → OUT OF BOUNDS!
The code change replaces this with a bounds-checked flattened loop.
chunk_prepare_kernel writes cumulative gate sums back into the 'a' buffer, making it an in-out parameter. The parameter was incorrectly declared as const float* with const_cast used to write through it. Make the mutability explicit by declaring 'a' as float* in both the kernel and its launcher signature, and remove the const_cast. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: huanxing <huanxing.shen@intel.com>
| // ======================================================================== | ||
| // Phase 2: Each item computes conv1d for its own feature lanes | ||
| // ======================================================================== | ||
| if (!feat_valid) return; |
There was a problem hiding this comment.
what is the functional difference between early return and wrapping the work into if (feat_valid) {...} ?
There was a problem hiding this comment.
It is to make all the subgroup to reach barrier, instead of early-return.
|
please fix conflicts. |
|
@ccrhx4 could we add some new unit tests to guard the fix? |
This PR is to fix low accuracy of Qwen3.5-27B on B60. VLLMZ-1515
There are two accuracy issues in the 27B model:
Before: GSM8k score ~= 0 when max_model_len >= 32K (vllm-xpu-kernels 0.1.10.1)
vllm ({'pretrained': '/hf_models/Qwen3.5-27B/', 'enforce_eager': True, 'tensor_parallel_size': 4, 'add_bos_token': True, 'max_model_len': 32768}), gen_kwargs: ({'max_gen_toks': 4096}), limit: 200.0, num_fewshot: 5, batch_size: auto
vllm ({'pretrained': '/hf_models/Qwen3.6-27B/', 'enforce_eager': True, 'tensor_parallel_size': 4, 'add_bos_token': True}), gen_kwargs: ({'max_gen_toks': 4096}), limit: 200.0, num_fewshot: 5, batch_size: auto
After:
vllm ({'pretrained': '/hf_models/Qwen3.5-27B/', 'enforce_eager': True, 'tensor_parallel_size': 4, 'add_bos_token': True}), gen_kwargs: ({'max_gen_toks': 4096}), limit: 200.0, num_fewshot: 5, batch_size: auto
vllm ({'pretrained': '/hf_models/Qwen3.6-27B/', 'enforce_eager': True, 'tensor_parallel_size': 4, 'add_bos_token': True}), gen_kwargs: ({'max_gen_toks': 4096}), limit: 200.0, num_fewshot: 5, batch_size: auto
FULL GSM8K with this PR
vllm ({'pretrained': '/hf_models/Qwen3.5-27B/', 'enforce_eager': True, 'tensor_parallel_size': 4, 'add_bos_token': True}), gen_kwargs: ({'max_gen_toks': 4096}), limit: None, num_fewshot: 5, batch_size: auto
vllm ({'pretrained': '/hf_models/Qwen3.6-27B/', 'enforce_eager': True, 'tensor_parallel_size': 4, 'add_bos_token': True}), gen_kwargs: ({'max_gen_toks': 4096}), limit: None, num_fewshot: 5, batch_size: auto
vllm ({'pretrained': '/hf_models/Qwen3.5-9B/', 'enforce_eager': True, 'tensor_parallel_size': 2, 'add_bos_token': True}), gen_kwargs: ({'max_gen_toks': 4096}), limit: None, num_fewshot: 5, batch_size: auto