Skip to content

Per channel quantization for page attention - #16

Open
xuchen-intel wants to merge 3 commits into
ceciliapeng2011:dev/quant_by_channelfrom
xuchen-intel:xuchen/pageatten/per_channel_int8
Open

Per channel quantization for page attention#16
xuchen-intel wants to merge 3 commits into
ceciliapeng2011:dev/quant_by_channelfrom
xuchen-intel:xuchen/pageatten/per_channel_int8

Conversation

@xuchen-intel

Copy link
Copy Markdown
Collaborator

Implement per channel quantization for page attention.

@xuchen-intel xuchen-intel changed the title [Draft] Per channel quantization for page attention Per channel quantization for page attention Jan 14, 2026
@xuchen-intel

Copy link
Copy Markdown
Collaborator Author

@ceciliapeng2011 Could you please take a look?

@xuchen-intel
xuchen-intel force-pushed the xuchen/pageatten/per_channel_int8 branch from 3bc266a to 6b8b600 Compare January 16, 2026 06:08
for seq_len in range(32768, 32810):
for trunk_num in range(1, 21):
for compressed_kvcache in [True,False,]:
for compressed_kvcache in [1,0,]:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 for fp16, 1 for per token, 2 for per channel... but why there is no test cases for 2? Have you benchmark them all here to cover any corner case? And is there any test cases to cover different
sub_block_sz?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Extra per channel test cases with different sub_block_sz are added. Thanks!

Comment thread opencl/tests/pageatten/cm_pa_kernel.hpp Outdated

#if CMPA_KVCACHE_U8
uint kv_offset = hkv*(head_size+4)*pa_block_sz;
uint k_offset = CMPA_KVCACHE_U8 == 1 ? hkv * (head_size + 4) * pa_block_sz : hkv * head_size * (pa_block_sz + pa_block_sz / sub_block_sz * 4);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better use constexpr here.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and static_assert to check pa_block_sz % sub_block_sz == 0

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any restriction to the possible values of sub_block_sz ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Compile-time evaluation is now assured. Restrictions about pa_block_sz and sub_block_sz are added.

Comment thread opencl/tests/pageatten/cm_pa_common.hpp Outdated
auto cur_block_id = block_indices[kv_pos / CMPA_BLOCK_SZ];
uint32_t dscale_offset = cur_block_id*quan_blk_stride + \
CMPA_BLOCK_SZ * head_size * sizeof(uint8_t) + kv_pos%CMPA_BLOCK_SZ*sizeof(half);
uint32_t k_dscale_offset = CMPA_KVCACHE_U8 == 1 ?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked assembly here - is there going to be a condition branch inst generated? m.b. constexpr is better?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No condition branch is found in assembly. But you are right. I've revised it to assure compile-time evaluation. Thanks!

@xuchen-intel
xuchen-intel force-pushed the xuchen/pageatten/per_channel_int8 branch from 6b8b600 to 294f655 Compare February 28, 2026 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants