Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe W4A16 grouped GEMM path now caches cluster-aligned and ChangesW4A16 cluster alignment
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Refactor Merge Risk: ⚪ Minimal · up to No confirmed correctness or stability issue remains in the changed kernel paths. The targeted boundary tests and validation support merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📌 Description
@HumansAnd
Reduce split CuTe DSL W4A16 routing and bounds overhead, and use complete output subtiles for ordinary scratch stores.
cutlass.Constexpr, include it in the private kernel cache key, and omit the runtime argument. This removes runtime division from routing-scale lookup and output scatter.Environment and source
hu-pdx-43); GPU0 exclusively used. Driver590.48.01; explicit containernvcr.io/nvidia/pytorch:26.05-py3.488ffbd9fe4455c3b4c3030cf668b5603c6698e4; current88ff7db6a558e8acbc0cd964275410cf94e5c22c. GPU runs used the baseline checkout plus the exact candidate patch; measured implementation/test hashes match this commit. No source changes followed validation.CUDA_VISIBLE_DEVICES=0,TRTLLM_ENABLE_PDL=1,FLASHINFER_DIST_AWARE_AUTOTUNE=0; project-local JIT caches, isolated venv preserving the image's Torch installation. Both sides use the same GPU/compiler stack.Validation
(128,64,256), cluster(1,1), M-major, N256/K512, 8 experts, PDL/CLC disabled. Top-k specialization removes runtimediv.s32/reciprocal sequences for top-k 2/3 (1640→1584 SASS instructions). At M256, bounds specialization further reduces the checked fused top-k2 case to 1568 and ordinary output from 1664→1624. At M288, both fallback SASS files are byte-identical to their immediate controls; these partial-width exports were compiled but not launched. The scratch-store change keeps the checked ordinary N64 instruction count at 1624; its N16 ordinary and N64 fused control SASS are byte-identical. Registers 128, stack 0 and local memory 0 remain unchanged.cp.async.bulk.tensor/UTMASTG.2D, with no TMA reduction; the fixed-order FP32 combiner is unchanged.--set fullcaptured current non-fused T2048 with its saved tactic, without retuning. FC2 uses grid2×1×74, cluster2, 512 threads, 128 registers/thread and222208 bytes dynamic shared memory (plus1024 driver bytes), with no local-memory accesses. NCU used40 kernel-replay passes; six CTC metrics were unavailable and separate native exports provide source line information. Instrumented durations are diagnostic, not benchmark evidence.git diff --checkpassed.Warnings are existing CuTe DSL deprecations. Reproduce with:
CUDA_VISIBLE_DEVICES=0 TRTLLM_ENABLE_PDL=1 FLASHINFER_DIST_AWARE_AUTOTUNE=0 \ python -m pytest \ tests/moe/test_cute_dsl_fused_moe.py::TestCuteDslMoeW4A16::test_route_tile_boundary_accuracy \ tests/moe/test_cute_dsl_fused_moe.py::TestCuteDslFusedMoeFunctional::test_finalize_handles_cluster_padding_and_partial_tiles \ tests/moe/test_cute_dsl_fused_moe.py::TestExpertParallelism::test_functional_with_ep \ tests/moe/test_cute_dsl_fused_moe.py::TestCuteDslMoEWrapper::test_wrapper_cuda_graph \ -k 'w4a16 or test_finalize_handles_cluster_padding_and_partial_tiles' -v --tb=shortFinal performance
Non-fused finalize (primary)
One saved upstream sweep versus the latest complete current-source sweep on the same unchanged node/stack. Speedup is Before/After. Geometric-mean speedup over 8–8192 is 1.0056× (0.55% lower latency); this is a small measured difference, not a statistical-significance claim. The scratch-store change repeatedly improves 1024–4096 against its immediate control; 8192 varied across repeats, so its final row does not establish a stable gain.
Fused finalize
Two saved upstream controls and two sweeps at
b90b34744491c36d0039f44cba664191f7bb0eb0on the same unchanged GPU/compiler stack. The later scratch-store condition is excluded from the fused SwiGLU path; source review and native control identity support reusing these measurements. These are not fresh timings of the new commit. Speedup issqrt((Before R1 * Before R2) / (After R1 * After R2)); no row minima or fixed-tactic substitutions.All four runs select the same GEMM1/GEMM2 tactic at 8192 tokens. Other rows can select different tactics under normal autotuning.
Reproduction
In the pinned container, run from the selected revision's checkout:
Run this from each revision's repository root for each finalize mode. The published non-fused table uses one sweep per source; the fused table uses two. It calls the existing benchmark's W4A16 arm directly, preserving its input generation, routing, wrapper, tuner and timer while avoiding other backends. Release inputs between token counts; both benchmarked revisions use identical code below.
🔍 Related Issues
Follow-up to #4048. The compile-time/division audit in the MegaMoE work (#5019) motivated checking the split kernel boundary.
🚀 Pull Request Checklist
Thank you for contributing to FlashInfer! Before we review your pull request, please make sure the following items are complete.
✅ Pre-commit Checks
pre-commitby runningpip install pre-commit(or used your preferred method).pre-commit install.pre-commit run --all-filesand fixed any reported issues.🧪 Tests
unittest, etc.).🔬 Experimental Track
flashinfer/experimental/and/or an@flashinfer_experimental_api. Tracking issue: #tests/experimental/and were validated on the intended hardware; a runnable example is included.flashinfer/aot.py, and no experimental backend is reachable frombackend="auto"withoutFLASHINFER_ALLOW_EXPERIMENTAL_AUTO_BACKENDS=1. (Calling an@flashinfer_experimental_apior naming a backend explicitly is itself the opt-in and needs no environment variable.)Reviewer Notes
Summary by CodeRabbit
Bug Fixes
Tests