[PR #202] [MERGED] HIP port of #192: zero_pad and strided NDHWC views for conv3d and GroupNorm #216

New issue

Closed

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

bsakaguchi commented

2026-10-26 22:21:41 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: amd/hip-seedvr2-views

📝 Commits (1)

0030df9 HIP port of #192: zero_pad and strided NDHWC views for conv3d and GroupNorm

📊 Changes

5 files changed (+398 additions, -76 deletions)

View changed files

📝 comfy_kitchen/backends/hip/__init__.py (+99 -28)
📝 comfy_kitchen/backends/hip/dlpack_bindings.cpp (+18 -10)
📝 comfy_kitchen/backends/hip/ops/conv3d_fp16.hip (+20 -13)
📝 comfy_kitchen/backends/hip/ops/group_norm_pad3d.hip (+44 -22)
📝 tests/test_hip_wmma.py (+217 -3)

📄 Description

Performance port of #192. Instead of routing zero_pad to eager and implementing _out as compute-then-copy, this moves all three changes into the HIP kernels.

ops/group_norm_pad3d.hip: adds kZeroPad, writing the zero border in the fused kernel.
ops/conv3d_fp16.hip: reads NDHWC inputs directly via strides, supporting packed tensors and single-batch frame windows like CUDA. Other out layouts use a temporary + copy, avoiding the much slower torch fallback on ROCm.
GroupNorm out follows CUDA's _writes_packed_ndhwc rule.
Raises the depth gate from 8192 to 16384 to match CUDA. HIP still accumulates in fp32, preserving the existing ~2e-4 relative error for the newly covered 512-channel 3x3x3 cases.

RDNA4 fp16 benchmarks:

512ch 3x3x3, 5x130x130: 80.1 ms -> 17.7 ms
512ch 3x3x3, 5x66x130: 23.1 ms -> 8.9 ms
GroupNorm+SiLU zero_pad, 128ch 5x256x256: 3.54 ms -> 1.04 ms
Existing convs unchanged: 1.33 -> 1.30 ms and 4.63 -> 4.64 ms

Compile-verified across all 17 manifest targets as one fat binary, with ISA checks on gfx1030/gfx1100/gfx1151/gfx1200/gfx1201. No scratch usage, conv VGPR counts are unchanged or lower, and the eight new GroupNorm instantiations match across families. Runtime-verified on gfx1200 only.

Full suite: 2149 passing vs. 2133 before; 102 failures unchanged, all pre-existing CUDA-extension-absent cases.

_{🔄 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/202 **Author:** [@0xDELUXA](https://github.com/0xDELUXA) **Created:** 9/25/2026 **Status:** ✅ Merged **Merged:** 9/25/2026 **Merged by:** [@comfyanonymous](https://github.com/comfyanonymous) **Base:** `main` ← **Head:** `amd/hip-seedvr2-views` --- ### 📝 Commits (1) - [`0030df9`](https://github.com/Comfy-Org/comfy-kitchen/commit/0030df917b38018619d3a9e0a565f8c00b50e518) HIP port of #192: zero_pad and strided NDHWC views for conv3d and GroupNorm ### 📊 Changes **5 files changed** (+398 additions, -76 deletions) <details> <summary>View changed files</summary> 📝 `comfy_kitchen/backends/hip/__init__.py` (+99 -28) 📝 `comfy_kitchen/backends/hip/dlpack_bindings.cpp` (+18 -10) 📝 `comfy_kitchen/backends/hip/ops/conv3d_fp16.hip` (+20 -13) 📝 `comfy_kitchen/backends/hip/ops/group_norm_pad3d.hip` (+44 -22) 📝 `tests/test_hip_wmma.py` (+217 -3) </details> ### 📄 Description Performance port of #192. Instead of routing `zero_pad` to eager and implementing `_out` as compute-then-copy, this moves all three changes into the HIP kernels. - `ops/group_norm_pad3d.hip`: adds `kZeroPad`, writing the zero border in the fused kernel. - `ops/conv3d_fp16.hip`: reads NDHWC inputs directly via strides, supporting packed tensors and single-batch frame windows like CUDA. Other `out` layouts use a temporary + copy, avoiding the much slower torch fallback on ROCm. - GroupNorm `out` follows CUDA's `_writes_packed_ndhwc` rule. - Raises the depth gate from 8192 to 16384 to match CUDA. HIP still accumulates in fp32, preserving the existing ~2e-4 relative error for the newly covered 512-channel 3x3x3 cases. RDNA4 fp16 benchmarks: - 512ch 3x3x3, 5x130x130: 80.1 ms -> 17.7 ms - 512ch 3x3x3, 5x66x130: 23.1 ms -> 8.9 ms - GroupNorm+SiLU `zero_pad`, 128ch 5x256x256: 3.54 ms -> 1.04 ms - Existing convs unchanged: 1.33 -> 1.30 ms and 4.63 -> 4.64 ms Compile-verified across all 17 manifest targets as one fat binary, with ISA checks on gfx1030/gfx1100/gfx1151/gfx1200/gfx1201. No scratch usage, conv VGPR counts are unchanged or lower, and the eight new GroupNorm instantiations match across families. Runtime-verified on gfx1200 only. Full suite: 2149 passing vs. 2133 before; 102 failures unchanged, all pre-existing CUDA-extension-absent cases. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:21:41 +00:00