Conversation
Port the vendor fork's PPUInt8ScaledMMLinearKernel (acext branch): row-major int8 weights fed straight to acext.int8_gemm, per-token dynamic activation quant via the _C::dynamic_scaled_int8_quant bridge. Symmetric path only; asymmetric configs report can_implement=False and fall back to the triton kernel. Registered first in _POSSIBLE_INT8_KERNELS[OOT] so it wins the kernel oracle on thead when acext is installed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Register the kernel after register_fl_w8a8_linear_kernel(), not before. That helper prepends FLW8A8DynamicLinearKernel at index 0 of the OOT INT8 candidates, and its is_supported() does not gate on vendor, so an earlier insert leaves FLW8A8 ahead of acext and acext is never selected on PPU -- its can_implement() accepts the same channelwise/dynamic/symmetric config DeepSeek-V4 INT8 uses. Also moved out of the 'OOT not in _POSSIBLE_INT8_KERNELS' guard, which skipped acext entirely whenever the OOT list was already populated, and made the insert idempotent so repeated calls cannot stack duplicates. Sort the import block to satisfy ruff I001.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unaligned shapes can fail at runtime, scale handling needs correction, and key behavior lacks automated coverage.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a T-Head PPU acext INT8 W8A8 scaled-MM kernel and prioritizes it in kernel selection.
Changes:
- Implements row-major weight handling and
acext.int8_gemmintegration. - Gates usage on T-Head hardware and
acextavailability. - Registers the kernel idempotently ahead of existing INT8 candidates.
File summaries
| File | Review findings |
|---|---|
vllm_fl/quantization/acext_int8_linear.py |
Critical (2 votes): Unaligned dimensions can abort instead of falling back. Moderate (1 vote): Normalize [N, 1] scales to a contiguous FP32 vector. Nit (2 votes): Add automated coverage for gating, weight handling, argument ordering, and idempotent insertion. |
vllm_fl/quantization/quant_linear.py |
Nit (1 vote): Add automated coverage verifying acext registry priority and idempotency. |
Review details
Suppressed comments (2)
vllm_fl/quantization/acext_int8_linear.py:103
- This forwards the checkpoint
weight_scaleshape unchanged. The canonical compressed-tensors scale is[N, 1](tests/unit_tests/quantization/test_w8a8_linear.py:209-212), while the neighboring scaled-MM adapter normalizes per-output scales to[N](vllm_fl/quantization/w8a8/linear.py:78-90). Passing the 2-D tensor toacext.int8_gemmcan make the vendor kernel reject or misinterpret the scale; flatten it to a contiguous fp32 vector before storing it.
weight_scale = getattr(layer, w_s_name)
if is_fused_module and not self.config.is_channelwise:
weight_scale = convert_to_channelwise(weight_scale, layer.logical_widths)
vllm_fl/quantization/quant_linear.py:100
- The ordering and idempotency branch is the behavior this PR is intended to guarantee, but the existing quantization tests do not exercise it:
test_oot_quant_registry_inherits_mxfp8_candidatesmonkeypatchesregister_fl_w8a8_linear_kerneland only checks the MXFP8 registry. Add a unit test that stubs the acext import/platform and asserts[AcextInt8ScaledMMLinearKernel, FLW8A8DynamicLinearKernel, ...]remains unchanged after a second call; otherwise a registry change can silently shadow the vendor kernel while hardware-only checks miss it.
int8_candidates = _POSSIBLE_INT8_KERNELS.setdefault(PlatformEnum.OOT, [])
if AcextInt8ScaledMMLinearKernel not in int8_candidates:
int8_candidates.insert(0, AcextInt8ScaledMMLinearKernel)
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| out_dtype: torch.dtype, | ||
| bias: torch.Tensor | None = None, | ||
| ) -> torch.Tensor: | ||
| assert b.shape[0] % 16 == 0 and b.shape[1] % 16 == 0 |
Comment on lines
+84
to
+87
| def can_implement(cls, c: Int8ScaledMMLinearLayerConfig) -> tuple[bool, str | None]: | ||
| if not c.input_symmetric: | ||
| return False, "acext kernel supports symmetric quantization only." | ||
| return True, None |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds
AcextInt8ScaledMMLinearKernel, an INT8 W8A8 scaled-MM linear kernel backedby the T-Head
acextvendor library, and registers it as the preferred INT8candidate on PPU.
Unlike the cutlass/triton kernels,
acext.int8_gemmtakes weights in row-major[N, K], soprocess_weights_after_loadingkeeps the checkpoint layout as-israther than transposing.
is_supported()gates on two things —acextimportable andvendor_name == "thead"— so the kernel is inert on every other platform.Registration order matters here, and it is easy to get wrong
The kernel is registered after
register_fl_w8a8_linear_kernel(), not before.That helper prepends
FLW8A8DynamicLinearKernelat index 0, and itsis_supported()does not gate on vendor, so registering acext any earlier leavesFLW8A8 ahead of it — and
can_implement()accepts the samechannelwise + dynamic + symmetric config DeepSeek-V4 INT8 uses, so acext would
never be selected on PPU and the vendor kernel would be silently unused.
Verified both ways on hardware:
The insert also sits outside the
OOT not in _POSSIBLE_INT8_KERNELSguard, whichwould otherwise skip acext whenever the OOT list was already populated, and is
idempotent so repeated calls cannot stack duplicates.
Testing
Runtime assertions on PPU (
vendor_name=thead,acext.int8_gemmpresent):add_oot_quant_kernel()FLW8A8DynamicLinearKernelis retained as a fallback at index 1add_oot_quant_kernel()call does not duplicate acextInt8ScaledMMLinearLayerConfig(is_static_input_scheme=False, is_channelwise=True, input_symmetric=True),the first kernel passing both
is_supported()andcan_implement()is acextruff check/ruff format --checkclean.I have not re-run an end-to-end accuracy or throughput comparison against the
FlagGems W8A8 kernel for this PR — the change verified here is selection order and
gating.
Environment: T-Head PPU-ZW810E, torch 2.10.0, vLLM 0.24.0.