[PR #252] [MERGED] HIP port of #251: SiTU input activation in the INT8 ConvRot quantizer #260

New issue

Closed

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

bsakaguchi commented

2026-10-26 22:22:12 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-kitchen/pull/252
Author: @0xDELUXA
Created: 10/16/2026
Status: ✅ Merged
Merged: 10/16/2026
Merged by: @comfyanonymous

Base: main ← Head: amd/hip-situ-input-act

📝 Commits (2)

a880913 HIP port of #251: SiTU input activation in the INT8 ConvRot quantizer
b2e5299 Fix HIP rope split-half rounding, fp8 NaN decode and a width-check test

📊 Changes

10 files changed (+139 additions, -65 deletions)

View changed files

📝 comfy_kitchen/backends/hip/__init__.py (+7 -4)
📝 comfy_kitchen/backends/hip/dlpack_bindings.cpp (+3 -3)
📝 comfy_kitchen/backends/hip/fp8_utils.h (+8 -4)
📝 comfy_kitchen/backends/hip/hadamard.h (+27 -11)
📝 comfy_kitchen/backends/hip/ops/apply_rope.hip (+4 -4)
📝 comfy_kitchen/backends/hip/ops/quantize_int8.hip (+8 -2)
📝 comfy_kitchen/backends/hip/ops/rms_rope.hip (+4 -4)
📝 comfy_kitchen/backends/hip/rope_math.h (+12 -10)
📝 tests/test_hip_wmma.py (+65 -22)
📝 tests/test_int8_input_act.py (+1 -1)

📄 Description

Performance port of #251. situ is now folded into the ConvRot quantizer next to swiglu, in the G=16/64/256 kernels and the global spill path, so MOSS-Image's ffn_down (as called by ComfyUI #17099) no longer materializes the activation. HIP has no fused M=1 GEMV, so the quantizer is the only place it lands. The tanh caps use the same expm1f form as CUDA's stable_tanh, since this build is -ffast-math too. Non-convrot routes still apply the activation eagerly, as on CUDA.

Measured on gfx1200, bf16, MOSS-Image ffn_down ([M, 16384] in, K=8192, N=3072, ConvRot G=256), the current ComfyUI expression plus int8_linear against the fused call: M=4096 goes 6.08 -> 3.17 ms and M=1024 1.47 -> 0.90 ms. The quantizer alone at M=4096 goes 5.01 -> 1.04 ms.

Also fixes the three HIP test failures on main. Split-half apply_rope and rms_rope lost bit parity with eager after e4106dd moved apply_rope_split_half1 to addcmul_; the kernels now round the same product eager does. dequantize_per_tensor_fp8 decoded the e4m3 NaN code as 480 with a newer clang, which drops the constant-NaN branch under -ffinite-math-only; the bits now pass through an empty asm. test_swiglu_row_is_half_the_input now accepts the ValueError HIP raises for the width mismatch.

The swiglu tests in test_hip_wmma.py now cover both gated pairs across G=16/64/256, global spill, no convrot and M=1, in fp32, fp16 and bf16, with near-zero rows. Compile-verified for gfx1030, gfx1100 and gfx1200 with no scratch in the changed files. Run-verified on gfx1200 only.

_{🔄 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/252 **Author:** [@0xDELUXA](https://github.com/0xDELUXA) **Created:** 10/16/2026 **Status:** ✅ Merged **Merged:** 10/16/2026 **Merged by:** [@comfyanonymous](https://github.com/comfyanonymous) **Base:** `main` ← **Head:** `amd/hip-situ-input-act` --- ### 📝 Commits (2) - [`a880913`](https://github.com/Comfy-Org/comfy-kitchen/commit/a880913349e375736ef6240a76e2413e116f96c3) HIP port of #251: SiTU input activation in the INT8 ConvRot quantizer - [`b2e5299`](https://github.com/Comfy-Org/comfy-kitchen/commit/b2e529999e635e834551619151e751ff37975db0) Fix HIP rope split-half rounding, fp8 NaN decode and a width-check test ### 📊 Changes **10 files changed** (+139 additions, -65 deletions) <details> <summary>View changed files</summary> 📝 `comfy_kitchen/backends/hip/__init__.py` (+7 -4) 📝 `comfy_kitchen/backends/hip/dlpack_bindings.cpp` (+3 -3) 📝 `comfy_kitchen/backends/hip/fp8_utils.h` (+8 -4) 📝 `comfy_kitchen/backends/hip/hadamard.h` (+27 -11) 📝 `comfy_kitchen/backends/hip/ops/apply_rope.hip` (+4 -4) 📝 `comfy_kitchen/backends/hip/ops/quantize_int8.hip` (+8 -2) 📝 `comfy_kitchen/backends/hip/ops/rms_rope.hip` (+4 -4) 📝 `comfy_kitchen/backends/hip/rope_math.h` (+12 -10) 📝 `tests/test_hip_wmma.py` (+65 -22) 📝 `tests/test_int8_input_act.py` (+1 -1) </details> ### 📄 Description Performance port of #251. `situ` is now folded into the ConvRot quantizer next to `swiglu`, in the G=16/64/256 kernels and the global spill path, so MOSS-Image's `ffn_down` (as called by ComfyUI [#17099](https://github.com/Comfy-Org/ComfyUI/pull/17099)) no longer materializes the activation. HIP has no fused M=1 GEMV, so the quantizer is the only place it lands. The tanh caps use the same `expm1f` form as CUDA's `stable_tanh`, since this build is `-ffast-math` too. Non-convrot routes still apply the activation eagerly, as on CUDA. Measured on gfx1200, bf16, MOSS-Image `ffn_down` (`[M, 16384]` in, K=8192, N=3072, ConvRot G=256), the current ComfyUI expression plus `int8_linear` against the fused call: M=4096 goes 6.08 -> 3.17 ms and M=1024 1.47 -> 0.90 ms. The quantizer alone at M=4096 goes 5.01 -> 1.04 ms. Also fixes the three HIP test failures on `main`. Split-half `apply_rope` and `rms_rope` lost bit parity with eager after e4106dd moved `apply_rope_split_half1` to `addcmul_`; the kernels now round the same product eager does. `dequantize_per_tensor_fp8` decoded the e4m3 NaN code as 480 with a newer clang, which drops the constant-NaN branch under `-ffinite-math-only`; the bits now pass through an empty asm. `test_swiglu_row_is_half_the_input` now accepts the `ValueError` HIP raises for the width mismatch. The swiglu tests in `test_hip_wmma.py` now cover both gated pairs across G=16/64/256, global spill, no convrot and M=1, in fp32, fp16 and bf16, with near-zero rows. Compile-verified for gfx1030, gfx1100 and gfx1200 with no scratch in the changed files. Run-verified on gfx1200 only. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:22:12 +00:00