Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,45 @@ After bumping the pin, re-validate the in-tree fused Metal kernel launchers in `

All three should produce output within RMS < 5e-3 of the graph reference on Apple Silicon.

#### Reconciling CUDA, Metal, and CPU source overlays

The overlays in `src/lib/mlx-cpp/patches/` and `src/lib/mlx-cpp/patches-cuda/` are applied whole-file with `configure_file(... COPYONLY)`, so any upstream commit to an overlaid file is silently reverted when the pin moves unless the overlay is rebased first. Every file under those two directories therefore carries a single machine-readable base line in its header comment:

```cpp
// Modified from upstream MLX <sha> <upstream/relative/path>
```

`<sha>` is the upstream MLX commit (7–40 lowercase hex characters, resolvable with `git rev-parse` in an `ml-explore/mlx` clone) that the overlay was copied from or last rebased onto, established by diffing against upstream rather than stamping the current pin onto untouched files.

Before and after bumping `GIT_TAG`, run the drift check against an `ml-explore/mlx` checkout (such as `$OUT_DIR/build/_deps/mlx-src` or `~/.cache/mlxcel/mlxcelverse/mlx.git`):

```bash
MLX_SRC=<ml-explore/mlx clone, or $OUT_DIR/build/_deps/mlx-src>
PIN=$(bash scripts/ci/mlx_pinned_commit.sh)
for f in $(find src/lib/mlx-cpp/patches src/lib/mlx-cpp/patches-cuda -type f); do
case "$f" in
*/patches-cuda/*) up="mlx/$(basename "$f")" ;;
*) up="${f#src/lib/mlx-cpp/patches/}" ;;
esac
base=$(sed -n 's|^// *Modified from upstream MLX \([0-9a-f]\{7,40\}\).*|\1|p' "$f" | head -1)
[ -n "$base" ] || { echo "NO BASE $f"; continue; }
n=$(git -C "$MLX_SRC" log --oneline "$base..$PIN" -- "$up" | wc -l)
[ "$n" -eq 0 ] || echo "DRIFT $n $f (base $base)"
done
```

For each file reported as `DRIFT`, 3-way merge the overlay from its declared `<base>` onto the new pin:

```bash
git -C "$MLX_SRC" show "<base>:<up>" > /tmp/mlx-base
git -C "$MLX_SRC" show "$PIN:<up>" > /tmp/mlx-theirs
git merge-file <overlay> /tmp/mlx-base /tmp/mlx-theirs
```

Resolve any conflicts, diff `<overlay>` line by line against `/tmp/mlx-theirs` to verify that every mlxcel delta documented in the file's header survived and no upstream change was dropped, and update that overlay's `// Modified from upstream MLX <sha> <path>` line to the new pin. When done, the drift check must produce no `NO BASE` and no `DRIFT` lines against `$PIN`. If upstream now carries the fix natively, retire the overlay instead of rebasing it.

#### Reconciling the ROCm overlay

The ROCm overlay in [`src/lib/mlx-cpp/patches-rocm/`](src/lib/mlx-cpp/patches-rocm/README.md) follows the pin too, and `make verify-rocm-overlay` (part of `make verify`) fails until it does: `patches-rocm/UPSTREAM` must name the new pin. The procedure is [`docs/mlxcelverse/rocm-overlay.md`](docs/mlxcelverse/rocm-overlay.md): `scripts/mlxcelverse/rocm_overlay.py retarget` 3-way merges the overlay's MLX core files onto the new pin; on an AMD host `scripts/mlxcelverse/check_api_drift.sh` lists the backend's compile errors and the primitives left without a ROCm `eval_gpu` (undefined symbols, since `libmlx` is static), which get a fix or a `NO_GPU` stub; `rocm_overlay.py drift` compares each core file line by line with the new pin plus the fork's own change and must match the reviewed `patches-rocm/CORE_RESIDUAL.diff`, which is what catches an overlay that was already wrong before the bump. Record each fix in `patches-rocm/LOCAL_FIXES.md`, run `make verify-rocm` on the AMD host (it includes the generation smoke from #1811), and add the "ROCm overlay" section described in that document to the PR body and to the technical report if there is one.

## Development environment
Expand Down
65 changes: 65 additions & 0 deletions TECHNICAL_REPORTS/2298-bump-mlx-pin-overlay-bases-20261014.en.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# Technical Report: PR #2298 - Bump MLX Pin and Declare Overlay Bases

- **Date**: 2026-10-14
- **Status**: Completed; implementation reviewed and all contract gates passed
- **Risk Level**: Low (all upstream changes 3-way merged, all 32 overlays verified at 0 drift, local ROCm overlay and unit test suites green)

## Executive Summary

PR #2298 updates the pinned upstream `ml-explore/mlx` commit from `81ba1c6a0e50a9268b931579c2d4f1158b9aab5a` to `59d600b5e64c238427d0f8d897ab7c682ef4d3d2` (14 upstream commits) in `src/lib/mlx-cpp/CMakeLists.txt`. It establishes a machine-readable `// Modified from upstream MLX <sha> <path>` base-declaration header across all 32 overlay files in `src/lib/mlx-cpp/patches/` and `src/lib/mlx-cpp/patches-cuda/`, resolving each base commit by diff against upstream history. All 9 conflicting or drifted overlays across that range were 3-way merged onto the new pin while preserving all mlxcel custom kernel and runtime deltas. Upstream fix `dfe17bafb` (#3912) was confirmed intact. The ROCm overlay was retargeted to the new pin with fallback stubs for the new upstream `fast::GatedDeltaUpdate` primitive, and `CONTRIBUTING.md` was extended with documentation for the CUDA/Metal/CPU overlay lifecycle and 3-way merge workflow.

## 1. Problem Statement

### 1.1 Background
The `mlxcel` repository applies whole-file source overlays onto upstream `ml-explore/mlx` via `configure_file(... COPYONLY)`. When moving the upstream `GIT_TAG` forward, any upstream modifications to an overlaid file are silently overwritten unless the overlay is explicitly rebased against upstream. Previously, only the ROCm overlay tracked its base commits and drift systematically, while CUDA, Metal, and CPU overlays lacked a machine-readable declaration of their copy or rebase commits.

### 1.2 Identified Deficiencies
- **Missing Base Declarations**: 29 of 32 overlay files lacked a machine-readable `Modified from upstream MLX` header, preventing automated drift checking.
- **Outdated Reducers**: Three CUDA reduction overlays (`all_reduce.cu`, `col_reduce.cu`, `row_reduce.cu`) were pinned to `v0.31.1` (`ce45c525`), missing upstream const-correctness and 64-bit planning improvements.
- **Upstream Pin Advancement**: Upstream MLX advanced with new features, including global-scale support for quantized operations (#4507), Gated Delta Net kernels (#4020), and non-multiple-of-32 quantized dimension fixes (#3912).
- **ROCm Divergence**: Upstream addition of `fast::GatedDeltaUpdate` required ROCm fallback stubs to prevent undefined symbol errors during static linking.

## 2. Implementation Details

### 2.1 Phase 1: Machine-Readable Base Declarations
All 32 overlay files under `src/lib/mlx-cpp/patches/` and `src/lib/mlx-cpp/patches-cuda/` now carry a header comment of the form:
```cpp
// Modified from upstream MLX <sha> <upstream/relative/path>
```
Each SHA was established by diffing against upstream `ml-explore/mlx` history. All 32 SHAs resolve in the upstream git repository.

### 2.2 Phase 2: 3-Way Rebase of Drifted Overlays
Nine overlays touched between their base and `59d600b5e` were 3-way merged onto `59d600b5e`:
1. `patches/mlx/backend/metal/quantized.cpp`: Integrated upstream global-scale loading while preserving the three `MLXCEL_QMV_WIDE` runtime off-switch hunks (#1186, #1187).
2. `patches/mlx/backend/cuda/device/qmm_naive.cuh`: Preserved Volta sm_70 `MmaPath` parameterization (#1543).
3. `patches/mlx/backend/cuda/quantized/qmm/qmv.cu`: Added `global_scale` parameter threading while preserving multirow qmv (#725), row-window caps (#906), and pre-Ampere accumulator optimizations (#1539).
4. `patches/mlx/backend/cuda/quantized/quantized.cpp`: Preserved 65535 grid chunking (#648), input contiguity checks, and sorted MoE prefill grouped-GEMM routes (#629).
5. `patches-cuda/fast.cpp`: Added `gated_delta_update` fallback implementation.
6. `patches-cuda/ops.cpp`: Updated `gather_qmm` GPU device validation and `GatherQQMM` argument list.
7. `patches/mlx/backend/cuda/reduce/{all_reduce,col_reduce,row_reduce}.cu`: Incorporated upstream `const_cast` removal and large-input 64-bit all-reduce planning.

The drift check confirms 0 upstream commits between declared bases and `59d600b5e` across all 32 overlays. Upstream `dfe17bafb` (#3912) is intact: neither `fp_quantized.h` nor `quantized.h` is overlaid, so both are consumed unmodified from the new pin.

### 2.3 Phase 3: Pin Bump and ROCm Overlay Retargeting
- `GIT_TAG` in `src/lib/mlx-cpp/CMakeLists.txt` was updated to `59d600b5e64c238427d0f8d897ab7c682ef4d3d2`.
- `patches-rocm/UPSTREAM` `retargeted_to_mlx_pin` was updated to `59d600b5e`.
- `patches-rocm/mlx/backend/rocm/primitives.cpp` added `fast::GatedDeltaUpdate::use_fallback` returning `true` and `NO_GPU_MULTI(GatedDeltaUpdate)`, documented as item 6 in `LOCAL_FIXES.md`.
- `patches-rocm/CORE_RESIDUAL.diff` was refreshed against `59d600b5e`.

### 2.4 Phase 4: Documentation
`CONTRIBUTING.md` was extended with the `Reconciling CUDA, Metal, and CPU source overlays` subsection, detailing:
- The `// Modified from upstream MLX <sha> <path>` header requirement.
- The automated shell loop for detecting overlay drift.
- The 3-way merge command sequence using `git merge-file`.
- Guidelines for retiring overlays absorbed upstream.

## 3. Verification

The following verification steps were executed and confirmed passing:
1. `bash scripts/ci/mlx_pinned_commit.sh` prints `59d600b5e64c238427d0f8d897ab7c682ef4d3d2`.
2. `cargo test -p mlxcel-mlx-pin` passes all 27 tests (26 unit tests + 1 cross-parser integration test).
3. Overlay drift check across all 32 files reports 0 `NO BASE` lines and 0 `DRIFT` lines.
4. `make verify-rocm-overlay` passes with 29 checks OK, validating 109 backend + 15 core files, 45 `LOCAL_FIXES` entries, and 4 noted residuals against pin `59d600b5`.
5. Contract gates passed: `make verify-versions verify-kernel-dtype-keys verify-kernel-port-dispatch verify-llama-compat verify-python-tooling verify-fmt`.
6. `git diff --check origin/main..HEAD` confirms no whitespace errors or leftover conflict markers.
7. Greedy generation validation on AMD `gfx1151` produces byte-identical tokens before and after the bump.
65 changes: 65 additions & 0 deletions TECHNICAL_REPORTS/2298-bump-mlx-pin-overlay-bases-20261014.ko.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# 기술 보고서: PR #2298 - MLX 핀 업데이트 및 오버레이 베이스 커밋 선언

- **날짜**: 2026-10-14
- **상태**: 완료; 구현 검토 및 전체 컨트랙트 게이트 통과
- **위험 수준**: 낮음 (모든 업스트림 변경 3-way 병합 완료, 32개 오버레이 드리프트 0 검증, 로컬 ROCm 오버레이 및 단위 테스트 전원 통과)

## 개요

PR #2298은 `src/lib/mlx-cpp/CMakeLists.txt`에서 고정된 업스트림 `ml-explore/mlx` 커밋을 `81ba1c6a0e50a9268b931579c2d4f1158b9aab5a`에서 `59d600b5e64c238427d0f8d897ab7c682ef4d3d2`로 업데이트합니다 (14개 업스트림 커밋). `src/lib/mlx-cpp/patches/` 및 `src/lib/mlx-cpp/patches-cuda/` 하위의 32개 전체 오버레이 파일에 기계 판독 가능한 `// Modified from upstream MLX <sha> <path>` 헤더를 선언하고, 각 베이스 커밋을 업스트림 히스토리와의 diff를 통해 정밀하게 식별하였습니다. 해당 구간에서 충돌하거나 드리프트가 발생한 9개 오버레이는 mlxcel의 커스텀 커널 및 런타임 변경점을 온전히 보존하면서 새 핀으로 3-way 병합되었습니다. 업스트림 수정사항인 `dfe17bafb` (#3912)가 온전히 유지됨을 확인하였습니다. ROCm 오버레이는 새 핀으로 리타깃되었고 신규 업스트림 `fast::GatedDeltaUpdate` 프리미티브에 대한 폴백 스텁이 추가되었으며, `CONTRIBUTING.md`에 CUDA/Metal/CPU 오버레이 수명 주기와 3-way 병합 절차가 문서화되었습니다.

## 1. 문제 정의

### 1.1 배경
`mlxcel` 저장소는 `configure_file(... COPYONLY)`를 사용하여 업스트림 `ml-explore/mlx`에 파일 단위 소스 오버레이를 적용합니다. 업스트림 `GIT_TAG`를 갱신할 때 오버레이를 명시적으로 리베이스하지 않으면 업스트림의 소스 변경사항이 조용히 덮어씌워져 유실될 위험이 있습니다. 기존에는 ROCm 오버레이만 체계적으로 베이스와 드리프트를 추적하고 있었으며, CUDA, Metal, CPU 오버레이에는 복사 또는 리베이스 시점의 커밋이 명시되지 않았습니다.

### 1.2 문제점
- **오버레이 베이스 선언 부재**: 32개 오버레이 중 29개 파일에 기계 판독 가능한 `Modified from upstream MLX` 헤더가 없어 자동 드리프트 감지가 불가능했습니다.
- **오래된 리듀서 오버레이**: 3개의 CUDA reduction 오버레이(`all_reduce.cu`, `col_reduce.cu`, `row_reduce.cu`)가 `v0.31.1` (`ce45c525`)에 머물러 있어 업스트림의 const-correctness 및 64비트 계획 개선사항이 누락되어 있었습니다.
- **업스트림 핀 갱신 필요**: 업스트림 MLX에 양자화 연산의 글로벌 스케일 지원(#4507), Gated Delta Net 커널(#4020), 32의 배수가 아닌 양자화 차원 지원(#3912) 등 주요 개선사항이 추가되었습니다.
- **ROCm 빌드 심볼 정합성**: 신규 추가된 `fast::GatedDeltaUpdate` 프리미티브에 대해 ROCm 백엔드 스텁이 없으면 정적 링킹 시 정의되지 않은 심볼 에러가 발생합니다.

## 2. 구현 상세

### 2.1 1단계: 기계 판독 가능한 베이스 선언
`src/lib/mlx-cpp/patches/` 및 `src/lib/mlx-cpp/patches-cuda/` 하위의 32개 전체 오버레이 파일에 다음 형태의 헤더 주석을 선언하였습니다:
```cpp
// Modified from upstream MLX <sha> <upstream/relative/path>
```
각 SHA는 `ml-explore/mlx` 업스트림 히스토리와의 diff를 통해 확정되었으며, 32개 SHA 모두 업스트림 저장소에서 유효하게 확인됩니다.

### 2.2 2단계: 드리프트 오버레이 3-way 리베이스
기존 베이스와 `59d600b5e` 사이에서 변경된 9개 오버레이를 `59d600b5e`로 3-way 병합하였습니다:
1. `patches/mlx/backend/metal/quantized.cpp`: 업스트림 글로벌 스케일 로딩을 통합하고 3개의 `MLXCEL_QMV_WIDE` 런타임 오프스위치 헝크(#1186, #1187)를 유지.
2. `patches/mlx/backend/cuda/device/qmm_naive.cuh`: Volta sm_70 `MmaPath` 템플릿 매개변수화(#1543) 유지.
3. `patches/mlx/backend/cuda/quantized/qmm/qmv.cu`: `global_scale` 파라미터 전달을 추가하고 multirow qmv(#725), 행 윈도우 캡(#906), pre-Ampere 누적 최적화(#1539)를 온전히 유지.
4. `patches/mlx/backend/cuda/quantized/quantized.cpp`: 65535 그리드 청킹(#648), 입력 연속성 검증, 정렬된 MoE 프리필 grouped-GEMM 경로(#629) 유지.
5. `patches-cuda/fast.cpp`: `gated_delta_update` 폴백 구현 추가.
6. `patches-cuda/ops.cpp`: `gather_qmm` GPU 디바이스 유효성 검사 및 `GatherQQMM` 인수 목록 갱신.
7. `patches/mlx/backend/cuda/reduce/{all_reduce,col_reduce,row_reduce}.cu`: 업스트림 `const_cast` 제거 및 대규모 입력에 대한 64비트 all-reduce 계획 반영.

드리프트 검증 결과 32개 오버레이 전원에 대해 베이스와 `59d600b5e` 간의 커밋 차이가 0임을 확인하였습니다. 업스트림 `dfe17bafb` (#3912)는 `fp_quantized.h` 및 `quantized.h`가 오버레이되지 않아 새 핀에서 온전히 반영됨을 확인했습니다.

### 2.3 3단계: 핀 갱신 및 ROCm 오버레이 리타깃
- `src/lib/mlx-cpp/CMakeLists.txt`의 `GIT_TAG`를 `59d600b5e64c238427d0f8d897ab7c682ef4d3d2`로 갱신.
- `patches-rocm/UPSTREAM`의 `retargeted_to_mlx_pin`을 `59d600b5e`로 갱신.
- `patches-rocm/mlx/backend/rocm/primitives.cpp`에 `fast::GatedDeltaUpdate::use_fallback` (`true` 반환) 및 `NO_GPU_MULTI(GatedDeltaUpdate)` 추가 (LOCAL_FIXES.md 항목 6에 기록).
- `patches-rocm/CORE_RESIDUAL.diff` 갱신.

### 2.4 4단계: 문서화
`CONTRIBUTING.md`에 `Reconciling CUDA, Metal, and CPU source overlays` 서브섹션을 추가하여 다음 내용을 기술하였습니다:
- `// Modified from upstream MLX <sha> <path>` 헤더 규약.
- 오버레이 드리프트 자동 탐지 셸 루프.
- `git merge-file`을 활용한 3-way 병합 절차.
- 업스트림에 반영된 오버레이의 제거 지침.

## 3. 검증 결과

다음 검증 항목들이 모두 정상 통과되었습니다:
1. `bash scripts/ci/mlx_pinned_commit.sh` 실행 시 `59d600b5e64c238427d0f8d897ab7c682ef4d3d2` 출력.
2. `cargo test -p mlxcel-mlx-pin` 전체 27개 테스트 통과 (26개 단위 테스트 + 1개 크로스 파서 통합 테스트).
3. 32개 오버레이 전체 드리프트 검사 시 0 `NO BASE`, 0 `DRIFT` 출력.
4. `make verify-rocm-overlay` 실행 시 29개 점검 항목 정상 통과 (109개 백엔드 + 15개 코어 파일, 45개 LOCAL_FIXES 항목, 4개 잔차 일치 확인).
5. 컨트랙트 게이트 통과: `make verify-versions verify-kernel-dtype-keys verify-kernel-port-dispatch verify-llama-compat verify-python-tooling verify-fmt`.
6. `git diff --check origin/main..HEAD` 공백 에러 및 충돌 마커 없음 확인.
7. AMD `gfx1151` 환경에서 탐욕적 텍스트 생성 테스트 결과 핀 업데이트 전후 토큰 단위 동일 출력 확인.
2 changes: 1 addition & 1 deletion src/lib/mlx-cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ else()
# `)` as ending the declaration, so one there would hide the tag
# instead of being read past.
# Both parsers fail loudly rather than guess when the shape does not hold.
GIT_TAG 81ba1c6a0e50a9268b931579c2d4f1158b9aab5a)
GIT_TAG 59d600b5e64c238427d0f8d897ab7c682ef4d3d2)

# Use FetchContent_Populate + add_subdirectory so we can apply source
# overlays before the MLX build system processes the files.
Expand Down
2 changes: 1 addition & 1 deletion src/lib/mlx-cpp/patches-cuda/dtype.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//
// CUDA patch: bf16 type promotion preservation
//
// Modified from upstream MLX v0.31.1 mlx/dtype.cpp
// Modified from upstream MLX ce45c525 mlx/dtype.cpp
//
// Changes to the type_rules promotion table:
// - bf16 + float32 → bf16 (was float32)
Expand Down
Loading
Loading