review(mk-gemm v2): PR #305 리뷰 13건 반영 -- 스윕 기반 ksr 규칙·장치 SM 수·정확 역수·복사 언롤·정적 에필로그, 벤치 기본 레인, 트레이스 도구 메인 스트림; 원장 30차 §4~5 - #308
Merged
Conversation
…ved ksr rule, device SM count, exact reciprocal, unrolled ring copy, static epilogue indices, one ts reader; bench judges the served lane by default; trace tool picks the main stream by kernel time Kernel (mk_gemm2_kernel): - mk_choose_ksr2 from the 30차 sweeps: one exact wave of the resident slots (48 tiles x 2, 32 x 3, 16 x 6), the longest slices under one wave when the tile count does not divide it, the finest slices above half the slots (51 tiles: a 102-unit plan measured +30 us). SM count from the device (was a hard-coded 48). v2 x2 now >= v1 x2 on every m <= 16 shape. - __frcp_rn for the row scale's reciprocal (exact for a power of two; the IEEE divide compiled to a slow-path call per row per k-block), the ring copy loop unrolled (the swizzled destinations are per-thread constants), static acc indices in the epilogue (a runtime bound put acc in local memory: 64 B stack frame -> 0), one store walk with two store lambdas. ptxas: 124 registers, 0 spills, 0 B stack. - One read-and-clear template for the four stamp readers; the v2 header states the cross-stream no-overlap contract its shared buffers inherit. Bench: --gemm2 defaults to the served lane (env), 'both' is explicit (the VERDICT authorises boots); the ksr sweep measures only at the ksr the host launches (clamps otherwise mislabelled a cell) and reuses the main loop's tensors; probe_exact builds the fixture, pack and reference once for all lanes; --stamps2 takes medians over iters launches. Trace tool: main stream = the stream with the most kernel time (the 09-01 stock trace has the forward on 210 and the prep/side pair on 17, the armed trace the other way), per-stream summary line, overlap lookback = the segment's longest kernel, finite-gap medians, exposure medians over all steps with the warm-up step skipped. Probe: the MoE-overlap probe reuses moe_decode_stream_probe's fixtures and calls the wrapper without out= (the stock flashinfer wrapper has no such keyword; the bench runner mounts the megakernel files only). Ledger 30차 §4-5: first bench round, the ksr=3 sweep, the rule, the review facts. README/runbook updated. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_efe1e6d8-2ee4-464c-9b93-79f18911053f) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
The updated trace tool introduces a robustness issue (empty-step crash) and an avoidable O(N²) stream-summary computation that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR follows up on MK-GEMM v2 (PR #305) by incorporating post-merge review feedback and first-round GPU measurements, refining the v2 k-slice selection rule, applying several CUDA kernel micro-optimizations derived from SASS inspection, and updating bench/probe/tooling defaults and reporting to better match serving/boot-verdict workflows.
Changes:
- Update
mk_choose_ksr2to a sweep-derived “one exact wave” rule using device SM count, with refined slicing behavior near/above half-slot tile counts. - Apply kernel-level optimizations in
mk_gemm2_kernel(unrolled raw copy,__frcp_rnfor pow2 reciprocal, static-index epilogue) and deduplicate stamp readers viamk_read_and_clear. - Adjust benches/probes/tools: default
--gemm2toenv, reuse fixtures/tensors for sweeps and exact gates, fix MoE overlap probe wrapper API, and improve trace stream classification and exposure/gap stats.
File summaries
| File | Description |
|---|---|
| tools/trace_launch_positions.py | Refines main-stream selection and exposure/gap reporting for trace analysis. |
| tests/test_logic.py | Updates string-based contract checks to reflect new kernel/bench logic. |
| RUNBOOK_KERNEL_CAMPAIGN2.md | Updates operational gate instructions to match new bench defaults and results. |
| probes/mk_gemm_moe_overlap_probe.py | Reuses served MoE fixture and removes unsupported out= usage for wrapper compatibility. |
| probes/megakernel_glm53_bench.py | Reuses tensors/packs during ksr sweeps and improves stamps2 sampling/reporting. |
| overlay/modules/glm53_megakernel/README.md | Updates v2 slice-rule documentation and adds explicit non-overlap contract notes. |
| overlay/modules/glm53_megakernel/glm53_megakernel.cu | Implements new ksr rule, device SM query, kernel micro-opts, and stamp-reader template. |
| MEASUREMENTS.md | Fills in measurement sections with sweep outcomes and rule justification. |
| build/glm53/glm53_megakernel.cu | Propagates megakernel CUDA changes into build artifact copy. |
| build/dsv4/glm53_megakernel.cu | Propagates megakernel CUDA changes into build artifact copy. |
Review details
- Files reviewed: 10/10 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.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8407351d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
choiceoh
pushed a commit
that referenced
this pull request
Sep 4, 2026
…31차로 PR #304 는 #301(672430e)에서 갈라져 나왔고, 그 뒤 main 이 #300·#302·#303·#305· #306·#307·#308(#309)로 앞서 나가면서 충돌 상태(dirty)가 됐다. 코드 파일은 겹치지 않았고(KDA 원패스·kpool 은 #304 단독), 겹친 것은 문서·프로필·테스트 넷이다. **RUNBOOK 충돌 2건** (1) 새 EXP 절의 삽입 위치: main 이 EXP-16 뒤에 EXP-22(로컬 양자화)·EXP-21(v2 레인)을 넣었고 #304 는 같은 자리에 EXP-20(미세 융합 묶음 2)을 넣었다. 경쟁이 아니라 가산이므로 셋 다 두되 기존 배열(최신 먼저)을 따라 22 → 21 → 20 순으로 둔다. (2) "순서와 근거" 절: main 은 평평한 번호 목록을 지우고 자기가 새로 만든 상태 표로 보내는 안내문으로 바꿨고(`채택·기각·닫힘은 위 상태 표에 있다`), #304 는 그 목록에 18번(EXP-20)을 덧붙였다. **main 의 재구성을 취하되**, 사라질 뻔한 #304 의 항목을 main 의 "다음 부팅 창에서" 목록에 8번으로 옮겨 실험이 목록과 함께 증발하지 않게 했다. **원장 번호 충돌 (자동 병합이 못 보는 것)** #304 는 자기 측정을 `★29차` 로 적었는데, 그 사이 main 에 다른 29차(메가커널 로컬 양자화, #307)와 30차(비상주 v2 레인, #305)가 먼저 들어왔다. 텍스트로는 충돌하지 않아 병합 결과에 **서로 다른 29차 둘**이 남았다. 28차 항목이 세운 선례("원장 번호 27 은 PR #290 이 쓰고 있어 28 로 적는다")대로 다음 빈 번호인 **31차** 로 옮겼고, 왜 옮겼는지 절 머리에 한 줄 남겼다. 측정값 자체는 손대지 않았다. 같은 번호를 가리키던 참조 6곳을 함께 고쳤다: MEASUREMENTS 절 제목, RUNBOOK EXP-20 표의 기각 축, `glm53_kda_onepass/README.md`, `moe_gate_sm121/README.md`, `profiles/glm53.env`, `tests/test_logic.py`. main 이 자기 29차를 가리키는 참조 (`VLLM_GLM53_MK_LOCALQ` 주석)는 그대로 두었다. **자동 병합분 검증** - `profiles/glm53.env`: main 의 `VLLM_GLM53_MK_LOCALQ` 와 #304 의 세 노브 (`KDA_DUAL_GEMM`·`KDA_ONEPASS`·`KPOOL_UPDATE_DIRECT_POS`) 전부 기본 0 으로 잔존. `MODULES=` 목록은 main 의 항목 하나도 잃지 않고 `glm53_kda_onepass` 만 늘었다. - overlay ↔ build 사본 3쌍(`glm53_kda_onepass.py`, `glm5next_kda.py`, `sparse_attn_indexer_kpool.py`) 모두 동일. - `tests/test_logic.py`: 양쪽 테스트 공존, `micro-fusion bundle 2 contracts` 통과. 검증: `tests/test_logic.py` all OK (6106 checks; 이 호스트엔 torch 부재로 일부 SKIP). 충돌 마커 0, 변경된 .py 전부 파싱 OK, `run_micro_fusion_check.sh` bash -n OK. GPU 검증(프로브 VERDICT·트레이스 물리확인)은 하지 않았다 — 이 환경에 GPU 가 없다. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JTghtTjyfT23vuhZwHzBSM
choiceoh
pushed a commit
that referenced
this pull request
Sep 4, 2026
…ro-fusions) into ostcode/glm-flash-kernel-improvement-5a65a8 profile MODULES: union (glm53_dev_lab kept beside the new module). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
choiceoh
added a commit
that referenced
this pull request
Sep 4, 2026
review(mk-gemm v2): #308 봇 스레드 5건(현재 장치 SM 수, 트레이스 도구 워밍업 감지·메인 스트림·빈 세그먼트) + 원장 30차 §6 노출 프로브·규칙 검증·스탬프
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.
무엇
PR #305(비상주 v2 GEMM 레인) 머지 뒤 코드리뷰(10 각도, 13건 보고)의 반영과 첫 GPU 측정 두 라운드.
커널
mk_gemm2_kernelmk_choose_ksr2를 30차 스윕(ksr 1/2/3/4/6/8 × 11 형상, 단독·연속)에서 도출: 상주 슬롯(SM 당 블록 × SM 수 = 96) 딱 한 웨이브(48타일×2, 32×3, 16×6), 나누어떨어지지 않으면 한 웨이브 이하의 가장 긴 슬라이스(8×8), 슬롯 절반 초과는 최세분(51타일: 102유닛의 짧은 2차 웨이브가 +30 µs). SM 수는 장치에서(상수 48 제거). 규칙 적용 시 m ≤ 16 전 형상에서 v2 x2 ≥ v1 x2(in_proj 79.4/80.4, 4096² 49.7/56.3, k3072 39.9/46.1, 공유 전문가 21.5·14.8 / 22.5·15.4).__frcp_rn(2의 거듭제곱 역수는 정확; IEEE 나눗셈이 행마다 슬로패스 호출), 링 복사 루프 언롤(스위즐 목적지는 스레드 상수), 에필로그 정적 인덱스(런타임 바운드가 acc 를 로컬 메모리로: 스택 64 → 0 B), 저장 워크 하나에 저장 람다 둘. ptxas 124 reg, 스필 0.mk_read_and_clear템플릿 하나. v2 헤더에 공유 버퍼의 스트림 간 비중첩 계약 명시.벤치·도구·프로브
--gemm2기본을 서빙 레인(env)으로 — VERDICT 가 부팅을 승인하므로 부팅이 돌리지 않을 레인으로 판정하지 않는다;both는 명시. ksr 스윕은 호스트가 실제로 띄우는 ksr 에서만 측정(클램프된 셀은-), 본 루프의 텐서 재사용.probe_exact는 픽스처·팩·참조를 한 번만.--stamps2는 iters 발사의 중앙값.tools/trace_launch_positions.py: 메인 스트림 = 커널 시간이 가장 큰 스트림(09-01 스톡은 forward 가 210·준비/쌍이 17, 무장은 반대), 스트림 요약 줄, 겹침 lookback = 세그먼트 최장 커널, 유한 gap 중앙값, 노출 중앙값은 워밍업 스텝 제외·부재 스텝 0.moe_decode_stream_probe의 픽스처 재사용, 스톡 래퍼에 없는out=제거(1차 실행 실패 원인).원장 30차 §4~5: 1차 벤치(exact 두 레인 × ksr 5종 PASS, 비분할 형상 비트 동일), ksr 3 스윕과 규칙 확정, m = 32 는 v2 가 뒤짐(블록별 x 양자화가 m 에 비례 — C=1 전용 팔, 다음 라운드 후보), LUT 쌍 공유(k 순열)는 비트 동일을 깨므로 스탬프 뒤.
검증
tests/test_logic.pyall OK (44,636 checks).VLLM_GLM53_MK_GEMM2기본 0). 노출 프로브·스탬프는 srv2gap_bench3.sh가 다음 GPU 창에 실행.🤖 Generated with Claude Code
Note
Medium Risk
Touches the W4 GEMM v2 dispatch path and k-slice grid sizing used when
VLLM_GLM53_MK_GEMM2=1; numerics stay gated but wrong ksr or stream overlap could affect correctness or latency on large batch (m≥32).Overview
Follow-up to mk_gemm v2 (PR #305): replaces the old “two waves”
mk_choose_ksr2heuristic with a 30차 sweep–derived rule (one full wave of resident slots = blocks/SM × device SM count, fine slicing when tile count exceeds half the slots, min 4 k-blocks/slice). Docs and MEASUREMENTS §4–5 record exact gates PASS and m ≤ 16 shapes matching or beating v1 on back-to-back runs; m = 32 still regresses (documented C=1 limitation).mk_gemm2_kernelmicro-optimizations from SASS review: unrolled raw-recordstage_rawcopy,__frcp_rnfor pow2 scales, static-indexstore_tileepilogue (avoids acc spilling to local stack), plusmk_read_and_clearfor phase stamps and explicit comments that v2 partials must not overlap concurrent MK GEMM launches across streams.Bench/probes/tools:
--gemm2defaults toenv(served lane for boot verdicts); ksr sweep reuses tensors and skips clamped ksr cells;probe_exactshares one fixture across lanes;--stamps2reports medians over iterations; MoE overlap probe sharesmoe_decode_stream_probefixture and drops unsupportedout=;trace_launch_positionspicks main stream by total kernel time and fixes exposure/gap statistics.Reviewed by Cursor Bugbot for commit c840735. Bugbot is set up for automated code reviews on this repo. Configure here.