Commit eea2762
committed
Port pulsar to ROCm/HIP
Re-enables the pulsar subrenderer on ROCm-built PyTorch. Pulsar was excluded from the ROCm build in the parent commit (PR #2039) because its 74 source files use a mix of CUDA intrinsics with no direct HIP equivalent. This commit makes it build and run on AMD MI250X (gfx90a, warpSize=64, HIP 7.2) using only HIP runtime APIs and standard math — no `amdgcn` inline assembly.
Most of the lift is build-system reversal. The pulsar source filter in `setup.py`, the `#ifndef USE_ROCM` wraps around the pulsar `#include`s and pybind block in `pytorch3d/csrc/ext.cpp`, and the `hasattr(_C, "PulsarRenderer")` guards in the three `pytorch3d/renderer/.../__init__.py` files all go away. The only C++ surgery is in `pytorch3d/csrc/pulsar/gpu/commands.h`:
The CUDA `_rn`-suffixed FP rounding intrinsics (`__fadd_rn`, `__fdiv_rn`, `__fsqrt_rn`, `__fmaf_rn`, `__frcp_rn`) and `__saturatef` have no HIP equivalents. AMD's GPU ISA has no instruction-level rounding-mode override — the rounding mode lives in the wavefront `MODE` register, defaulting to round-to-nearest-even. So under `USE_ROCM` these macros expand to plain operators / `sqrtf` / `fmaf` / `1.0f/x` / `fmaxf(0,fminf(1,x))`, which are rounding-mode-equivalent on HIP. The HIP/clang compiler may fuse `a+b*c` into a single-rounding FMA where CUDA's `_rn` would have prevented it; the upstream pulsar authors already accept compiler-discretion FMA fusion for `FMUL`/`FSUB` (the `_rn` variants are commented out in the source), and pulsar's tests pass under the same trade-off on HIP. If FMA-fusion drift ever surfaces as a numerical issue, add `-ffp-contract=off` to pulsar's HIPCC flags.
`__powf` is replaced with `powf` for clarity on the ROCm arm.
`atomicAdd_block` has no HIP function-name equivalent, but the *semantic* equivalent is `__hip_atomic_fetch_add(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WORKGROUP)`. Plain HIP `atomicAdd` is device-scope (`__HIP_MEMORY_SCOPE_AGENT`, see `amd_hip_atomic.h:218`), which is strictly stronger than block-scope — correct but forces L2-coherent atomics on what are block-local counters at `renderer.render.device.h:186,297` (sphere-loading counter and per-pixel-done counter). The workgroup-scoped builtin gets us back to the cheaper LDS atomic path that CUDA's `atomicAdd_block` uses.
Verified absent-from-HIP and explicitly replaced: `__fdiv_rn`, `__fadd_rn`, `__fsqrt_rn`, `__fmaf_rn`, `__frcp_rn`, `__saturatef`, `atomicAdd_block` (semantic replacement via `__hip_atomic_fetch_add` + `__HIP_MEMORY_SCOPE_WORKGROUP`).
Verified present-in-HIP and left alone: `hsqrt` (`amd_hip_fp16.h`), `__int2float_rn` (`amd_device_functions.h:536`, defined as `(float)x`), `__hadd`/`__hsub2`/`__hmul2` (`amd_hip_fp16.h`), `__clz`/`__popc`/`__popcll`/`__mul24` (`amd_device_functions.h`), native `atomicMin(float*,float)`/`atomicMax(float*,float)` (`amd_hip_atomic.h`), and the existing `__HIP_PLATFORM_AMD__` `__ballot`+`__popcll` warpSize=64 workaround at `include/renderer.render.device.h:289-295`.
The unreferenced 32-lane WARP_CUMSUM/WARP_MAX/WARP_SUM helpers at `gpu/commands.h:66-123` remain gated out on ROCm — the TODO comment about ROCM-6.2 was technically right that HIP supports `__shfl_*_sync` now, but since nothing in pulsar actually calls these helpers, there's no live warpSize=32 assumption to fix.
Test results on AMD Instinct MI250X (gfx90a, HIP 7.2):
tests/pulsar/test_forward.py 5 passed
tests/pulsar/test_channels.py 1 passed
tests/pulsar/test_depth.py 1 passed
tests/pulsar/test_hands.py 1 passed
tests/pulsar/test_ortho.py 1 passed
tests/pulsar/test_small_spheres.py 1 passed
tests/test_render_points.py::test_simple_sphere_pulsar 1 passed
tests/test_render_points.py::test_unified_inputs_pulsar 1 passed
tests/test_camera_conversions.py::test_pulsar_conversion 1 passed
Pulsar tests need `FB_TEST=1` because `tests/pulsar/test_forward.py::test_principal_point` writes a 1-channel debug PNG via `imageio.imsave`, and current `imageio`'s pillow backend refuses 1-channel writes (`ValueError: Can't write images with one color channel.`). The test gates the imsave block on `not os.environ.get("FB_TEST", False)`; the actual GPU render and `np.allclose` assertion that follow have nothing to do with imageio. This is an imageio/pillow versioning issue, not the HIP port.
Parent PR's 123 core CUDA-kernel tests still pass on the same host — no regression.
.gitignore now also excludes `tests/pulsar/test_out/` (PNG debug dumps written when FB_TEST is unset).
Co-Authored-By: Claude Opus 4.7 (1M context)1 parent 776489a commit eea2762
7 files changed
Lines changed: 39 additions & 51 deletions
File tree
- pytorch3d
- csrc
- pulsar/gpu
- renderer
- points
- pulsar
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
17 | 20 | | |
18 | 21 | | |
19 | 22 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
10 | | - | |
11 | | - | |
12 | 9 | | |
13 | 10 | | |
14 | 11 | | |
15 | 12 | | |
16 | 13 | | |
17 | | - | |
18 | 14 | | |
19 | 15 | | |
20 | 16 | | |
| |||
102 | 98 | | |
103 | 99 | | |
104 | 100 | | |
105 | | - | |
106 | | - | |
107 | 101 | | |
108 | 102 | | |
109 | 103 | | |
| |||
189 | 183 | | |
190 | 184 | | |
191 | 185 | | |
192 | | - | |
193 | 186 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
125 | 125 | | |
126 | 126 | | |
127 | 127 | | |
128 | | - | |
129 | 128 | | |
130 | 129 | | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
131 | 146 | | |
132 | 147 | | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
133 | 154 | | |
134 | 155 | | |
135 | | - | |
136 | 156 | | |
137 | 157 | | |
138 | 158 | | |
139 | 159 | | |
140 | 160 | | |
141 | | - | |
142 | 161 | | |
143 | 162 | | |
144 | 163 | | |
145 | 164 | | |
146 | 165 | | |
147 | | - | |
148 | | - | |
149 | 166 | | |
150 | | - | |
151 | 167 | | |
152 | 168 | | |
153 | 169 | | |
| |||
201 | 217 | | |
202 | 218 | | |
203 | 219 | | |
204 | | - | |
| 220 | + | |
| 221 | + | |
205 | 222 | | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
206 | 231 | | |
207 | 232 | | |
208 | 233 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
75 | 75 | | |
76 | 76 | | |
77 | 77 | | |
| 78 | + | |
78 | 79 | | |
79 | 80 | | |
80 | | - | |
81 | | - | |
82 | | - | |
83 | | - | |
84 | | - | |
85 | | - | |
86 | | - | |
87 | | - | |
88 | 81 | | |
89 | 82 | | |
90 | 83 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
12 | | - | |
13 | 11 | | |
| 12 | + | |
14 | 13 | | |
15 | 14 | | |
16 | 15 | | |
17 | 16 | | |
18 | 17 | | |
19 | | - | |
20 | | - | |
21 | | - | |
22 | | - | |
23 | | - | |
24 | | - | |
25 | | - | |
26 | 18 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
10 | | - | |
11 | | - | |
12 | | - | |
13 | | - | |
14 | | - | |
15 | | - | |
16 | | - | |
17 | | - | |
18 | | - | |
| 9 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
59 | 59 | | |
60 | 60 | | |
61 | 61 | | |
62 | | - | |
63 | | - | |
64 | | - | |
| 62 | + | |
65 | 63 | | |
66 | 64 | | |
67 | 65 | | |
| |||
74 | 72 | | |
75 | 73 | | |
76 | 74 | | |
77 | | - | |
78 | | - | |
79 | | - | |
80 | | - | |
81 | | - | |
82 | | - | |
83 | | - | |
84 | 75 | | |
85 | 76 | | |
86 | 77 | | |
| |||
0 commit comments