Comfy-Org/comfy-kitchen

Fork 0

mirror of https://github.com/Comfy-Org/comfy-kitchen.git synced 2026-11-04 23:37:42 +00:00

[PR #74] [MERGED] Add HIP backend for AMD RDNA2-RDNA4 to the existing wheels #123

New issue

Closed

opened 2026-10-26 22:20:36 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:20:36 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-kitchen/pull/74
Author: @0xDELUXA
Created: 7/13/2026
Status: ✅ Merged
Merged: 7/29/2026
Merged by: @comfyanonymous

Base: main ← Head: amd/hip-backend

📝 Commits (3)

94a5202 Add HIP backend
67da18a fix test_hip_wmma.py hang
7d2a04c Fix Windows ROCm pytest exit hang via cuda.synchronize in sessionfinish

📊 Changes

34 files changed (+7421 additions, -152 deletions)

View changed files

📝 .github/workflows/build-wheels.yml (+190 -30)
📝 README.md (+130 -15)
📝 comfy_kitchen/__init__.py (+6 -0)
➕ comfy_kitchen/backends/hip/CMakeLists.txt (+168 -0)
➕ comfy_kitchen/backends/hip/__init__.py (+1414 -0)
➕ comfy_kitchen/backends/hip/dlpack_bindings.cpp (+805 -0)
➕ comfy_kitchen/backends/hip/epilogue.h (+70 -0)
➕ comfy_kitchen/backends/hip/fp8_utils.h (+189 -0)
➕ comfy_kitchen/backends/hip/gemm_wmma.h (+219 -0)
➕ comfy_kitchen/backends/hip/hadamard.h (+183 -0)
➕ comfy_kitchen/backends/hip/mma.h (+310 -0)
➕ comfy_kitchen/backends/hip/ops/adaln.hip (+142 -0)
➕ comfy_kitchen/backends/hip/ops/apply_rope.hip (+110 -0)
➕ comfy_kitchen/backends/hip/ops/convrot_w4a4.hip (+122 -0)
➕ comfy_kitchen/backends/hip/ops/gemm_fp8.hip (+116 -0)
➕ comfy_kitchen/backends/hip/ops/gemm_int8.hip (+112 -0)
➕ comfy_kitchen/backends/hip/ops/gemv_awq.hip (+114 -0)
➕ comfy_kitchen/backends/hip/ops/per_tensor_fp8.hip (+114 -0)
➕ comfy_kitchen/backends/hip/ops/quantize_int8.hip (+142 -0)
➕ comfy_kitchen/backends/hip/ops/rms_rope.hip (+156 -0)

...and 14 more files

📄 Description

Supersedes #72. That PR is sitting at ~100 review comments and is gfx12-only, so rather than force-push another rewrite on top of it I opened a fresh one. This picks up the two things that actually blocked it:

@comfyanonymous on #72:

We need to figure out how to package this in the current comfy-kitchen wheels, I don't want to have separate wheels for nvidia/amd/intel/etc...

This builds one wheel with both backends in it, and keeps the abi3 tag while doing it.

#72 only ran on RDNA4.

This adds RDNA3, RDNA3.5 and RDNA2.

The kernels are #72's, rebased on current main. The packaging follows the approach in dev/hip1 by @rattus128.

Packaging

setup.py detects both toolchains and emits backends/cuda/_C* and backends/hip/_C* into a single wheel. The Linux job installs CUDA and ROCm into the same manylinux container; the Windows job installs the ROCm toolchain from AMD's release directory (https://repo.radeon.com/), the same host the Linux job takes its RPMs from. auditwheel excludes the ROCm runtime libs the same way it already excludes the CUDA ones, and both jobs fail if the wheel does not contain both extensions. Each backend withdraws itself at import when its runtime is missing, so a CUDA-only or ROCm-only machine gets the same wheel and just sees one backend register.

The HIP extension is built against the Python limited API on 3.12+, like the CUDA one. That matters for the packaging goal: an extension that is not abi3 drags the whole wheel back to version-specific builds, so adding AMD would otherwise have multiplied the wheel matrix. It does not. Nothing outside backends/hip/ changes in the build, and the wheel matrix does not grow: one wheel, both backends, same abi3 tag as today.

Architectures

Generation	gfx targets	Matrix cores	What runs
RDNA4	`gfx1200`, `gfx1201`	WMMA + fp8	All HIP-supported kernels, fp8 GEMM natively
RDNA3.5	`gfx1150`-`gfx1153`	WMMA, no fp8	All HIP-supported kernels; fp8 GEMM widened to bf16
RDNA3	`gfx1100`-`gfx1103`	WMMA, no fp8	All HIP-supported kernels; fp8 GEMM widened to bf16
RDNA2	`gfx1030`-`gfx1036`	none	Non-WMMA kernels incl. AWQ GEMV; WMMA GEMMs decline

v_wmma_*_w32_gfx12 has no gfx11 encoding, and the difference is not only the intrinsic name: gfx12 splits a K-step across the half-waves and takes 8-byte operands, while gfx11 gives each lane the whole K-step and duplicates it across the half-waves (rocWMMA calls this input duplication). The fragment type and the bytes of a K-row one MMA consumes therefore moved into an Mma policy in the new mma.h, and the tile GEMM stays byte-addressed and shared between the two.

gfx11 has no fp8 WMMA at all, so MmaFp8/MmaBf8 widen the operands to bf16 there. The widening is exact (e4m3 and e5m2 mantissas both fit bf16's 7 bits) and both paths accumulate in f32, so RDNA3 fp8 should be as accurate as RDNA4's native path, just slower.

RDNA2 has no matrix cores. It runs the kernels that never needed them (RoPE, AdaLN, the fp8/int8 quantizers, stochastic rounding, the AWQ GEMV) and does not advertise the four GEMMs, which fall through to triton/eager. The GEMM kernels still compile for gfx103x, where they trap, so the registry gates them by architecture instead: with a mix of GPUs in one process the capability set is the intersection, since kernels launch on the tensor's own device. scaled_mm_fp8 is reached directly rather than through the registry, so it tests hip.has_wmma() explicitly.

The default arch list is every RDNA2/3/3.5/4 target ROCm supports. COMFY_HIP_ARCHS still overrides it, and a local build for the one GPU it can see is much faster.

Testing

tests/test_hip_wmma.py passes on a gfx1200 (RX 9060 XT), Windows, ROCm 7.15, torch 2.12. The suite is arch-aware: the GEMM tests skip themselves on a device without matrix cores.
No regressions, measured on one tree by toggling COMFY_KITCHEN_DISABLE_HIP.
All 17 gfx targets compile, and I checked the generated ISA per target rather than trusting the build: for ops/gemm_int8.hip, each of the 10 gfx11xx/gfx12xx targets emits 336 v_wmma instructions and each of the 7 gfx103x targets emits zero plus the traps. A missing arch macro would not fail the build, it would silently drop that GPU into the no-WMMA stub, which is how I caught that gfx1103 was missing.
bdist_wheel on Windows produces a wheel containing backends/hip/_C.abi3.pyd, so the abi3 tag does survive the HIP extension.
ruff clean.
The toolchain used by CI (ROCm 7.2.1 from AMD's release directory) builds every target end to end, RDNA2 through RDNA4: I built the extension against it for gfx1030;gfx1100;gfx1200 and disassembled all 17 targets - the 10 gfx11xx/gfx12xx emit v_wmma, the 7 gfx103x emit none, as intended.
The Linux job builds one manylinux wheel holding both backends/cuda/_C*.so and backends/hip/_C*.so, and the "Verify Linux wheels contain CUDA and HIP extensions" step - which hard-fails if either is missing - passes; the Windows jobs do the same for .pyd. The CPU-only test jobs then install that wheel on runners with neither GPU runtime and pass, which exercises the "each backend withdraws itself at import" path.

Kernel changes to improve RDNA3 performance can only be properly validated on an actual RDNA3 GPU. If you have access to RDNA3 hardware and would like to help, feel free to fork the amd/hip-backend branch, implement and test RDNA3-specific improvements there, and open a PR against that branch. Once those changes are merged, they'll automatically become part of this PR.

RDNA2 users should still use Triton for int8, as rocBLAS doesn't contain int kernels, and hipBLASLt doesn't support RDNA2. As things are today, the INT8-Fast-ROCM node is the best option.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfy-kitchen/pull/74 **Author:** [@0xDELUXA](https://github.com/0xDELUXA) **Created:** 7/13/2026 **Status:** ✅ Merged **Merged:** 7/29/2026 **Merged by:** [@comfyanonymous](https://github.com/comfyanonymous) **Base:** `main` ← **Head:** `amd/hip-backend` --- ### 📝 Commits (3) - [`94a5202`](https://github.com/Comfy-Org/comfy-kitchen/commit/94a5202933957c7ae46e82b4f55763fc5d63a2c3) Add HIP backend - [`67da18a`](https://github.com/Comfy-Org/comfy-kitchen/commit/67da18a97880792d48b0a0762db92efe0784df62) fix test_hip_wmma.py hang - [`7d2a04c`](https://github.com/Comfy-Org/comfy-kitchen/commit/7d2a04c1b500a5531b35afbe6212f0548ad4e918) Fix Windows ROCm pytest exit hang via cuda.synchronize in sessionfinish ### 📊 Changes **34 files changed** (+7421 additions, -152 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/build-wheels.yml` (+190 -30) 📝 `README.md` (+130 -15) 📝 `comfy_kitchen/__init__.py` (+6 -0) ➕ `comfy_kitchen/backends/hip/CMakeLists.txt` (+168 -0) ➕ `comfy_kitchen/backends/hip/__init__.py` (+1414 -0) ➕ `comfy_kitchen/backends/hip/dlpack_bindings.cpp` (+805 -0) ➕ `comfy_kitchen/backends/hip/epilogue.h` (+70 -0) ➕ `comfy_kitchen/backends/hip/fp8_utils.h` (+189 -0) ➕ `comfy_kitchen/backends/hip/gemm_wmma.h` (+219 -0) ➕ `comfy_kitchen/backends/hip/hadamard.h` (+183 -0) ➕ `comfy_kitchen/backends/hip/mma.h` (+310 -0) ➕ `comfy_kitchen/backends/hip/ops/adaln.hip` (+142 -0) ➕ `comfy_kitchen/backends/hip/ops/apply_rope.hip` (+110 -0) ➕ `comfy_kitchen/backends/hip/ops/convrot_w4a4.hip` (+122 -0) ➕ `comfy_kitchen/backends/hip/ops/gemm_fp8.hip` (+116 -0) ➕ `comfy_kitchen/backends/hip/ops/gemm_int8.hip` (+112 -0) ➕ `comfy_kitchen/backends/hip/ops/gemv_awq.hip` (+114 -0) ➕ `comfy_kitchen/backends/hip/ops/per_tensor_fp8.hip` (+114 -0) ➕ `comfy_kitchen/backends/hip/ops/quantize_int8.hip` (+142 -0) ➕ `comfy_kitchen/backends/hip/ops/rms_rope.hip` (+156 -0) _...and 14 more files_ </details> ### 📄 Description Supersedes #72. That PR is sitting at ~100 review comments and is gfx12-only, so rather than force-push another rewrite on top of it I opened a fresh one. This picks up the two things that actually blocked it: - @comfyanonymous on [#72](https://github.com/Comfy-Org/comfy-kitchen/pull/72#issuecomment-4953900342): > We need to figure out how to package this in the current comfy-kitchen wheels, I don't want to have separate wheels for nvidia/amd/intel/etc... This builds one wheel with both backends in it, and keeps the `abi3` tag while doing it. - [#72](https://github.com/Comfy-Org/comfy-kitchen/pull/72) only ran on RDNA4. This adds RDNA3, RDNA3.5 and RDNA2. The kernels are #72's, rebased on current main. The packaging follows the approach in [dev/hip1](https://github.com/Comfy-Org/comfy-kitchen/tree/dev/hip1) by @rattus128. ### Packaging `setup.py` detects both toolchains and emits `backends/cuda/_C*` and `backends/hip/_C*` into a single wheel. The Linux job installs CUDA and ROCm into the same manylinux container; the Windows job installs the ROCm toolchain from AMD's release directory (https://repo.radeon.com/), the same host the Linux job takes its RPMs from. `auditwheel` excludes the ROCm runtime libs the same way it already excludes the CUDA ones, and both jobs fail if the wheel does not contain both extensions. Each backend withdraws itself at import when its runtime is missing, so a CUDA-only or ROCm-only machine gets the same wheel and just sees one backend register. The HIP extension is built against the Python limited API on 3.12+, like the CUDA one. That matters for the packaging goal: an extension that is not abi3 drags the whole wheel back to version-specific builds, so adding AMD would otherwise have multiplied the wheel matrix. It does not. Nothing outside `backends/hip/` changes in the build, and the wheel matrix does not grow: one wheel, both backends, same `abi3` tag as today. ### Architectures | Generation | gfx targets | Matrix cores | What runs | |------------|---------------------|--------------|-----------------------------------------| | RDNA4 | `gfx1200`, `gfx1201`| WMMA + fp8 | All HIP-supported kernels, fp8 GEMM natively | | RDNA3.5 | `gfx1150`-`gfx1153` | WMMA, no fp8 | All HIP-supported kernels; fp8 GEMM widened to bf16 | | RDNA3 | `gfx1100`-`gfx1103` | WMMA, no fp8 | All HIP-supported kernels; fp8 GEMM widened to bf16 | | RDNA2 | `gfx1030`-`gfx1036` | none | Non-WMMA kernels incl. AWQ GEMV; WMMA GEMMs decline | `v_wmma_*_w32_gfx12` has no gfx11 encoding, and the difference is not only the intrinsic name: gfx12 splits a K-step across the half-waves and takes 8-byte operands, while gfx11 gives each lane the whole K-step and duplicates it across the half-waves (rocWMMA calls this input duplication). The fragment type and the bytes of a K-row one MMA consumes therefore moved into an `Mma` policy in the new `mma.h`, and the tile GEMM stays byte-addressed and shared between the two. gfx11 has no fp8 WMMA at all, so `MmaFp8`/`MmaBf8` widen the operands to bf16 there. The widening is exact (e4m3 and e5m2 mantissas both fit bf16's 7 bits) and both paths accumulate in f32, so RDNA3 fp8 should be as accurate as RDNA4's native path, just slower. RDNA2 has no matrix cores. It runs the kernels that never needed them (RoPE, AdaLN, the fp8/int8 quantizers, stochastic rounding, the AWQ GEMV) and does not advertise the four GEMMs, which fall through to triton/eager. The GEMM kernels still compile for gfx103x, where they trap, so the registry gates them by architecture instead: with a mix of GPUs in one process the capability set is the intersection, since kernels launch on the tensor's own device. `scaled_mm_fp8` is reached directly rather than through the registry, so it tests `hip.has_wmma()` explicitly. The default arch list is every RDNA2/3/3.5/4 target ROCm supports. `COMFY_HIP_ARCHS` still overrides it, and a local build for the one GPU it can see is much faster. ### Testing - `tests/test_hip_wmma.py` passes on a gfx1200 (RX 9060 XT), Windows, ROCm 7.15, torch 2.12. The suite is arch-aware: the GEMM tests skip themselves on a device without matrix cores. - No regressions, measured on one tree by toggling `COMFY_KITCHEN_DISABLE_HIP`. - All 17 gfx targets compile, and I checked the generated ISA per target rather than trusting the build: for ops/gemm_int8.hip, each of the 10 gfx11xx/gfx12xx targets emits 336 `v_wmma` instructions and each of the 7 gfx103x targets emits zero plus the traps. A missing arch macro would not fail the build, it would silently drop that GPU into the no-WMMA stub, which is how I caught that `gfx1103` was missing. - `bdist_wheel` on Windows produces a wheel containing `backends/hip/_C.abi3.pyd`, so the `abi3` tag does survive the HIP extension. - `ruff` clean. - The toolchain used by CI (ROCm 7.2.1 from AMD's release directory) builds every target end to end, RDNA2 through RDNA4: I built the extension against it for gfx1030;gfx1100;gfx1200 and disassembled all 17 targets - the 10 gfx11xx/gfx12xx emit `v_wmma`, the 7 gfx103x emit none, as intended. - The Linux job builds one manylinux wheel holding both `backends/cuda/_C*.so` and `backends/hip/_C*.so`, and the "Verify Linux wheels contain CUDA and HIP extensions" step - which hard-fails if either is missing - passes; the Windows jobs do the same for `.pyd`. The CPU-only test jobs then install that wheel on runners with neither GPU runtime and pass, which exercises the "each backend withdraws itself at import" path. Kernel changes to improve RDNA3 performance can only be properly validated on an actual RDNA3 GPU. If you have access to RDNA3 hardware and would like to help, feel free to fork the [amd/hip-backend](https://github.com/0xDELUXA/comfy-kitchen_win-rocm/tree/amd/hip-backend) branch, implement and test RDNA3-specific improvements there, and open a PR against that branch. Once those changes are merged, they'll automatically become part of this PR. RDNA2 users should still use Triton for int8, as rocBLAS doesn't contain int kernels, and hipBLASLt doesn't support RDNA2. As things are today, the [INT8-Fast-ROCM](https://github.com/patientx/ComfyUI-INT8-Fast-ROCM) node is the best option. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:20:36 +00:00

closed this issue
added the
pull-request
label

No labels

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Comfy-Org/comfy-kitchen#123

No description provided.

Rows
Columns