Skip to content

Add AMD GPU (ROCm/HIP) support to the Caspar backend - #465

Open
jeffdaily wants to merge 15 commits into
symforce-org:mainfrom
AMD-Ecosystem:moat-port
Open

jeffdaily wants to merge 15 commits into
symforce-org:mainfrom
AMD-Ecosystem:moat-port

Conversation

@jeffdaily

@jeffdaily jeffdaily commented Jun 16, 2026 •

Copy link
Copy Markdown

This PR adds AMD GPU support to Caspar, so its generated kernels and runtime build and run on AMD GPUs through ROCm/HIP. The default NVIDIA CUDA build is unchanged. It is enabled with caslib.compile(out_dir, use_hip=True, hip_arch=...), or with -DUSE_HIP=ON in the generated build.

The CUDA runtime spellings Caspar emits and uses (cudaMalloc, cudaFuncSetAttribute, error handling and the rest of the runtime API, plus atomicAdd_block and the cub namespace) are mapped to their HIP equivalents in one compatibility header, source/runtime/cuda_to_hip.h. In a CUDA build the header includes the CUDA runtime, and also cooperative_groups and CUB when compiling CUDA, so host-only .cc files can include it. The header provides no device-side fallbacks of its own. The code generation templates emit this header, so the symbolic kernel definitions are unchanged.

The code generation gains a HIP path. CasparLibrary.compile() in code_generation/library.py takes use_hip and hip_arch, and the generated CMakeLists.txt has a USE_HIP branch that compiles the sources as HIP against hip and hipcub. The runtime sources get the matching HIP changes: shuffle-based tile reductions where HIP lacks cg::reduce, shared-memory atomics where it lacks cg::labeled_partition and coalesced-group shuffles (memops.cuh), a block-strided copy where it lacks cg::memcpy_async (shared_indices.cu), and a host-side pointer-attribute lookup in the pybind layer. A small Windows build fix (C++17, git path separator) is included for the all-clang ROCm toolchain.

The PR also fixes a solver bug that ROCm exposed: DoUpdateRFirst() did not zero r_kp1_norm2_tot before accumulating into it, so the first PCG step of each solver iteration added onto the previous iteration's value (or uninitialized memory on the first) before the PCG exit test read it. The bug is not specific to ROCm.

Tests

  • symforce_caspar_codegen_test regenerates its checked-in kernels and solvers through the HIP-aware templates. It also checks in the generated CMakeLists.txt, with and without Python bindings, so changes to the HIP (or CUDA) build branch show up in review like other codegen changes.
  • test/CMakeLists.txt builds the *_cuda_test.cc Caspar GPU tests with HIP when CMake finds a HIP compiler and no CUDA compiler. With -DCMAKE_HIP_ARCHITECTURES set the build needs no AMD GPU, so a ROCm-only machine or container compiles every checked-in Caspar kernel and solver. Running them needs an AMD GPU, as the CUDA tests need an NVIDIA one.

How to build for AMD GPUs

Pass use_hip=True and the target architecture when compiling a generated library:

caslib.generate(out_dir)
caslib.compile(out_dir, use_hip=True, hip_arch="gfx90a")

Set hip_arch to the target AMD GPU (for example gfx90a for CDNA2 or gfx1100 for RDNA3). Without it, CMake builds for the GPU it detects on the build machine. The build uses CMake's HIP language support, so it needs the ROCm Clang compiler (amdclang++; CMake does not accept the hipcc wrapper) and the hip and hipcub packages.

To build the Caspar GPU tests for ROCm:

cmake -B build -DCMAKE_HIP_ARCHITECTURES=gfx90a
cmake --build build --target bin.tests.caspar_padding_lane_cuda_test bin.tests.caspar_solver_cuda_test bin.tests.caspar_tridiagonal_cuda_test bin.tests.caspar_backend_coverage_cuda_test
ctest --test-dir build -R caspar_

Validation

The Caspar GPU tests (padding lane, solver, tridiagonal, backend coverage) and symforce_caspar_codegen_test pass on an AMD Instinct MI250X and an AMD Radeon PRO W7800 on Linux, and the four Caspar GPU tests also pass on an AMD Radeon 8060S on Windows. The Caspar test targets also compile and link with CUDA 12.8.

Adds AMD GPU support to Caspar so its generated kernels and runtime also
build and run on AMD GPUs via HIP/ROCm, while leaving the default NVIDIA
CUDA build unchanged (enabled with use_hip=True / -DUSE_HIP=ON; off by
default).

Review in this order:

1. symforce/caspar/source/runtime/cuda_to_hip.h (new): a compatibility
   header that maps the CUDA spellings Caspar emits and uses (cudaMalloc,
   __syncthreads, cooperative-group reductions, CUB primitives, the
   runtime API) onto their HIP equivalents, and supplies device-side
   fallbacks where HIP lacks a cg:: primitive (reduce, labeled_partition).

2. code_generation/library.py and source/templates/*.jinja: the codegen
   gains a use_hip/hip_arch path; the generated build file and kernel
   templates emit the compat include and HIP-correct spellings, so the
   symbolic kernel definitions are unchanged.

3. source/runtime/*.cu, memops.cuh, pybind_array_tools.{cc,h}: the
   runtime HIP-compat (shared-memory atomics, the cooperative-group
   reduction fallback, and a host-side pointer-attribute lookup).

4. A small Windows build fix (C++17, git path separator) for the
   all-clang ROCm toolchain.

5. README: how to build a generated library for AMD GPUs.

Authored with assistance from Claude.

Test Plan:

The full code-generation pipeline (generate -> HIP-compile -> execute on
GPU -> verify numerical output) was exercised on real AMD hardware,
Linux CDNA2 (gfx90a) and RDNA3 (gfx1100) and Windows RDNA4 (gfx1201):

  compile_caspar_library(caslib, out_dir, use_hip=True, hip_arch="gfx90a")
  # generated kernel executes on GPU; output matches the CUDA path.

The default NVIDIA CUDA build (use_hip=False) is unchanged.
The generated CMake template pinned CMAKE_HIP_ARCHITECTURES to gfx90a when
unset, but the pin ran after project(... LANGUAGES ... HIP), which already
enables the HIP language and resolves the architecture. The block was dead
code that could only mislead, or become a live footgun if file order ever
changed.

project(... LANGUAGES ... HIP) already honors an explicit
-DCMAKE_HIP_ARCHITECTURES, otherwise auto-detects the host GPU, and errors
on a no-GPU build host. Removing the pin lets that single mechanism decide
the arch, so a user on a non-gfx90a card no longer risks a silently
mistargeted default. The hip_arch argument still threads through as
-DCMAKE_HIP_ARCHITECTURES.

This work was authored with the assistance of the Claude AI assistant.
@aaron-skydio

Copy link
Copy Markdown
Member

Thanks for the contribution, this looks interesting. I'm a little torn on whether we can commit to the maintenance burden of an AMD implementation here especially given the lack of tests. I think I'm going to leave this open and see how much interest there is from users in AMD support?

@jeffdaily

Copy link
Copy Markdown
Author

At minimum I could add a build-only github action workflow. Would that be sufficient?

bjoernellens1 pushed a commit to bjoernellens1/symforce that referenced this pull request Aug 4, 2026
@bjoernellens1

Copy link
Copy Markdown

I've been building on top of this PR (running it on gfx1151 / ROCm 7.2) and hit a real data race in Caspar's solver runtime that's worth flagging here, since it's in the HIP-specific code path this PR introduces.

Bug: SumStore() in symforce/caspar/source/runtime/memops.cuh performs a two-stage warp-tile reduction into a shared-memory scratch buffer (inout_shared/shared_tmp) and returns without a trailing __syncthreads() after its last write. That buffer gets reused within the same kernel invocation — e.g. kernel_Point_alpha_numerator_denominator.cu calls SumStore twice back-to-back against the same buffer, and every kernel using the AddSum/WriteSum accessors calls SumStore immediately followed by SumFlushFinal. Only the 32 lanes in the meta_group_rank() == 0 tile do the final read of that buffer; the other threads finish SumStore right after the first-stage write, with nothing forcing them to wait for that tile's read before a subsequent write to the same buffer is issued.

On gfx1151 this reliably manifested as a real race: intermittent NaN/-nan in score_current/step_quality, runaway LM damping, and — the most convincing signal — run-to-run nondeterminism on the exact same compiled binary (no regeneration/recompile between runs). Five reruns of the identical binary produced five different reprojection RMS values (8.33 / 7.48 / 7.43 / 7.24 / 4.20 px), which rules out a build/environment issue and points squarely at a runtime race.

Suggested fix — a single trailing __syncthreads():

       if (group.thread_rank() == 0) {
         shared_tmp[offset] = tot;
       }
     }
+    __syncthreads();
   }

Verification: with the fix, the same determinism test (5 within-build reruns + 3 fresh-build reruns) produced the bit-identical, correct result (0.004022114544300473 px, all passing) — 8/8 vs. the prior 9/9 divergent failures. I later broadened this to a 22-run sweep across fresh RNG seeds and a 3x larger synthetic problem (12 cams / 64 points): 22/22 passed with no failures.

Honest caveat on mechanism: I initially attributed this to a write-after-read hazard across the SumStore→SumStore/SumFlushFinal call boundary, reasoned as fixed by the trailing barrier. On closer review that specific explanation doesn't fully hold up — both call sites that actually exhibit the divergence already have their own leading __syncthreads(), which should independently enforce safety there, making the trailing barrier look redundant at exactly the site where the divergence was observed. A later revert-and-retest (removing the barrier and rerunning the original determinism protocol) also failed to reliably reproduce the original 9/9 failure — an inconclusive result rather than a confirming one. So I can't fully defend the causal mechanism if pressed on it. What I can say with confidence: the fix is empirically robust (22/22 across broadened seed/size coverage, 0 failures), it's a real, defensible correctness improvement to SumStore's shared-buffer-reuse contract regardless of mechanism, and it costs nothing (one barrier, not gated to HIP since the missing-barrier hazard isn't platform-specific in principle, just harder to trigger on other scheduling regimes). I'd treat this as a legitimate hardening fix rather than a fully root-caused bug report.

I also independently checked FlushSumShared's HIP atomicAdd_block fallback (the largest HIP-specific rewrite in this file) against an independent NumPy host reference on a passing run — device output matched the host-computed reference to float32 precision (max relative error ~5e-6 for Cam nodes, ~6e-5 for Point nodes), so no evidence it's involved, though that only characterizes a clean run and doesn't rule out a rare mistimed interleaving.

Happy to open this as a proper PR against your branch if that's easier to review/merge than a diff in a comment — just let me know which you'd prefer.

@jeffdaily

Copy link
Copy Markdown
Author

Note: this reply was drafted by an AI assistant.

Thanks for the thorough report -- five reruns with per-run RMS values and a 22/22 bit-identical confirmation with the fix in place is exactly the kind of evidence that makes this easy to act on. The diff in the comment is enough; no need to open a separate PR unless you'd prefer the authorship to be visible that way.

A barrier after the last shared-memory write is the standard idiom when the buffer can be reused by a later phase, so the change is sound on its face even with the causal chain not fully pinned down. I'll verify the mechanism against the surrounding code rather than take it on faith, fold the fix into the next update to this branch together with the rebase onto current main (the branch has drifted into conflict), credit you with a Co-authored-by trailer, and rerun the determinism check plus the test suite on our AMD hardware before pushing. Results will go here when that lands.

jeffdaily and others added 2 commits August 20, 2026 17:42
Upstream moved on since this branch was last synchronized, leaving the pull
request conflicting. Merge current main and resolve the one conflict so it
applies cleanly again. Nothing is rebased: the previously reviewed commits stay
exactly where they were.

symforce/caspar/source/templates/buildfiles/CMakeLists.txt.jinja: upstream
introduced the CASPAR_MIN_ARCH cache variable and revised the CUDA architecture
list in the same block this branch had moved into the else() of the USE_HIP
switch. The resolution keeps the USE_HIP/CUDA structure and adopts upstream's
new CUDA block verbatim inside the else() branch, CASPAR_MIN_ARCH included. The
HIP branch of the template is untouched, and CASPAR_MIN_ARCH has no other
consumer, so scoping it to the CUDA branch changes nothing for either vendor.

Authored with assistance from Claude (Anthropic).

Test Plan:
Linux gfx1100 (Radeon Pro W7800, ROCm 7.2.1) for the HIP path, and a CUDA 12.8
compile+link check with nvcc on the same host (no NVIDIA GPU present). The
runtime CMakeLists.txt is rendered from templates/buildfiles/CMakeLists.txt.jinja
with python_bindings=False.

```
cmake symforce/caspar/source/runtime -B build_hip -DUSE_HIP=ON -DCMAKE_HIP_ARCHITECTURES=gfx1100
cmake --build build_hip -j16     # libcaspar_runtime_core.a links
CUDACXX=nvcc cmake symforce/caspar/source/runtime -B build_cuda -DUSE_HIP=OFF
cmake --build build_cuda -j16    # CUDA path with the new arch list links
```

The gfx1100 device ISA of all three kernel objects (shared_indices, solver_tools,
sort_indices) is identical before and after the merge, so GPU behavior is
unchanged from the reviewed branch.
SumStore's second stage has the first warp tile read inout_shared and reduce it
into shared_tmp, and the function returns without a trailing __syncthreads().
The buffer is therefore not safe to reuse until the next block-wide barrier,
and today that safety rests on non-local invariants of the call sites: SumStore
and SumFlushFinal each open with a block-wide barrier, and every other store
into the buffer is indexed by threadIdx.x scaled by the storing accessor's
element stride, so nothing below element 32 is written by a thread outside the
reading tile, whose own reads the shuffle collectives order ahead of any lane
leaving the function. The trailing
barrier replaces those implicit guarantees with a local one, at the cost of one
barrier per call.

Suggested by a user running this branch on gfx1151 / ROCm 7.2, where solver
runs showed intermittent NaNs and run-to-run nondeterminism on the same binary,
and adding this barrier made their determinism protocol pass 22/22 across fresh
seeds and a larger synthetic problem. The precise interleaving behind their
failure was not pinned down (their own revert-and-retest was inconclusive), so
this change is offered on the strength of that empirical evidence and of the
contract argument above, not a root-caused race. The barrier is not gated to
HIP: the reuse contract is not platform-specific.

Authored with assistance from Claude (Anthropic).

Test Plan:
Linux gfx1100 (Radeon Pro W7800, ROCm 7.2.1). A standalone harness compiles
memops.cuh with hipcc and launches a 1024-thread kernel issuing four
back-to-back SumStore calls against the same inout_shared buffer followed by
SumFlushFinal, over a problem size that exercises the valid mask:

```
hipcc -x hip sumstore_test.cpp -I symforce/caspar/source/runtime --offload-arch=gfx1100 -o sumstore_test
./sumstore_test   # all 4 sums exact, 20 reruns bit-identical
```

The pre-change code passes the same harness on this card (that call pattern is
protected by SumStore's own leading barrier, and the gfx1151 failure does not
reproduce here), so this is verified as a no-regression hardening. The changed
header is not part of the prebuilt runtime library; it is compiled by the
generated kernels, so the CUDA side is covered by compiling a representative
translation unit with nvcc 12.8:

```
nvcc -arch=sm_75 -c nvcc_memops.cu -I symforce/caspar/source/runtime -o nvcc_memops.o
# nvcc_memops.cu: includes memops.cuh and instantiates SumStore, SumFlushFinal, FlushSumBlock
```

Co-authored-by: bjoernellens1 <64093272+bjoernellens1@users.noreply.github.com>
@aaron-skydio

Copy link
Copy Markdown
Member

Yeah it seems like AMD support here would be useful to a bunch of people, so yeah I'm down to merge this

I'll want to have tests that at minimum cover the codegen part, and probably also the build, it's going to be harder to get something actually testing on an AMD gpu. As much of that as you're able to do would be helpful

Brings the branch up to date with upstream main (acdedd0).

Conflicts:
	symforce/caspar/source/runtime/sort_indices.h

Upstream added `#include <sys/types.h>` where this branch selects `hip/hip_runtime.h` or `cuda_runtime.h`; the resolution keeps both, with the new include first.

Authored with Claude.
Upstream now launches generated kernels with dynamic shared memory, opting in with cudaFuncSetAttribute(..., cudaFuncAttributeMaxDynamicSharedMemorySize, ...) when a kernel needs more than 48 KB, and the generated solver checks cudaGetDeviceCount. The HIP compatibility header did not map these, so generated code no longer compiled for ROCm.

hipFuncSetAttribute only takes the kernel as const void*, while the CUDA runtime also accepts the kernel function itself, so the header adds a small template overload that casts it.

Authored with Claude.

Test Plan:
Build the Caspar GPU tests (generated kernels and solvers checked into test/) for gfx90a and run them on an MI250X:

```
cmake -B build -DCMAKE_HIP_ARCHITECTURES=gfx90a -DCMAKE_PREFIX_PATH=<ROCm prefix>
cmake --build build --target bin.tests.caspar_padding_lane_cuda_test bin.tests.caspar_solver_cuda_test bin.tests.caspar_backend_coverage_cuda_test
./build/bin/tests/caspar_padding_lane_cuda_test
./build/bin/tests/caspar_solver_cuda_test
./build/bin/tests/caspar_backend_coverage_cuda_test
```
This reverts the trailing __syncthreads() added to SumStore in "[ROCm] Harden SumStore's shared scratch buffer reuse contract".

That barrier was added for a gfx1151 report of NaN scores and run-to-run nondeterminism in solves, without a root-caused race. Upstream has since fixed the mechanism behind that symptom family in "[Caspar] Zero initialize caspar kernel registers" (symforce-org#469): lanes past problem_size passed uninitialized registers to SumStore, and LLVM can drop the valid-lane mask. Uninitialized registers explain both the NaN and the run-to-run variation.

With register zeroing in place, the barrier is not needed:
- SumStore and SumFlushFinal open with a block-wide barrier, and every other store into the scratch buffer below element 32 comes from a lane of the tile that reads it.
- On an MI250X (gfx90a), the padding-lane score kernel and the solver's alpha numerator/denominator kernel (two SumStore calls back to back into one buffer) give bit-identical per-block partial sums over 200 runs at sizes from 1000 to 1000003, with and without the barrier. With integer inputs, the alpha kernel's per-block sums equal an exact host reference in every run.
- Without register zeroing, the padding-lane score is NaN at every size that leaves padding lanes, whether or not the barrier is present. So the barrier does not fix that defect either.

Authored with Claude.

Test Plan:
Build upstream's checked-in kernels against memops.cuh with and without the barrier, with SumFlushFinal's atomicAdd replaced by a per-block store so cross-block float atomic ordering does not mask anything, and run each 200 times:

```
hipcc -x hip -std=c++17 -O3 -DUSE_HIP --offload-arch=gfx90a -I. -Iruntime main.cc kernel_caspar_padding_lane_score.cu runtime/shared_indices.cu -o padding_lane
hipcc -x hip -std=c++17 -O3 -DUSE_HIP --offload-arch=gfx90a -I. -Iruntime main_alpha.cc kernel_Matrix21_alpha_numerator_denominator.cu -o alpha
./padding_lane 200
./alpha 200
```
The generated kernels and solvers include cuda_to_hip.h instead of the CUDA runtime and cooperative groups headers, so the code symforce_caspar_codegen_test checks in no longer matched what the templates generate. This regenerates it with the test's own --update; nothing was edited by hand. With USE_HIP unset, cuda_to_hip.h includes the same CUDA headers as before, so the CUDA build of this code is unchanged.

Authored with Claude.

Test Plan:
```
python test/symforce_caspar_codegen_test.py --update
python test/symforce_caspar_codegen_test.py
```
DoUpdateRFirst() zeroes r_0_norm2_tot and then accumulates into both r_0_norm2_tot and r_kp1_norm2_tot, but only DoUpdateR() zeroes r_kp1_norm2_tot. The first PCG iteration therefore adds the new residual norm onto whatever the solver's allocation held. The bad value feeds the PCG exit test and beta, so the result depends on uninitialized device memory.

Freshly allocated device memory is usually zero, which hides this. On ROCm, a second solver in the same process gets memory freed by the first, and caspar_tridiagonal_cuda_test failed a varying 5 to 7 of its 20 uncoupled cases on an MI250X: step rejected, 199 PCG iterations. Filling the allocation with 0xFF makes the failures deterministic, and zero-filling it makes them disappear. That narrowed the read to the solver's scalar accumulators. The same defect is reachable on CUDA, so the fix is not gated on HIP.

Authored with Claude.

Test Plan:
```
python test/symforce_caspar_codegen_test.py --update
python test/symforce_caspar_codegen_test.py
cmake --build build --target bin.tests.caspar_tridiagonal_cuda_test bin.tests.caspar_solver_cuda_test
./build/bin/tests/caspar_tridiagonal_cuda_test   # 5/5 runs pass, also with the allocation filled with 0xFF
./build/bin/tests/caspar_solver_cuda_test
```
The *_cuda_test.cc tests run code checked in by symforce_caspar_codegen_test and are built only when CMake finds a CUDA compiler. When it finds no CUDA compiler but finds a HIP compiler, this builds the same tests for AMD GPUs instead. The runtime, the generated .cu/.cc files and the test sources are compiled as HIP, with USE_HIP defined and hip::host and hip::hipcub linked. The tests include cuda_to_hip.h, which includes the CUDA runtime when USE_HIP is unset, so the CUDA build is unchanged.

The build itself needs no AMD GPU: with -DCMAKE_HIP_ARCHITECTURES set, CMake does not query the device, so a ROCm-only machine or container compiles every checked-in kernel and solver with the HIP toolchain. That catches HIP compile breaks from template or runtime changes. Running the tests needs an AMD GPU, as the CUDA tests need an NVIDIA one. HIP language support needs CMake 3.21, so the HIP check is skipped on older CMake.

Authored with Claude.

Test Plan:
On an MI250X (gfx90a), ROCm 7.x:

```
cmake -S . -B build -DCMAKE_HIP_ARCHITECTURES=gfx90a -DCMAKE_PREFIX_PATH=<ROCm prefix>
cmake --build build --target bin.tests.caspar_padding_lane_cuda_test bin.tests.caspar_solver_cuda_test bin.tests.caspar_tridiagonal_cuda_test bin.tests.caspar_backend_coverage_cuda_test
cd build && ctest -R caspar_ --output-on-failure
```
symforce_caspar_codegen_test checks in the kernels and solvers caspar generates, but not the CMakeLists.txt it generates next to them. That file holds the only generated code that differs between the CUDA and HIP builds: the USE_HIP branch, which selects the HIP language, finds hip and hipcub, compiles the .cu sources (and, with Python bindings, the pybind sources) as HIP, and sets the HIP math flags. A change to that branch, or to the CUDA one, went unnoticed until someone built a generated library on that backend.

test_generate_build_files generates the build files for the padding-lane library with and without Python bindings and compares them with checked-in copies, like the other codegen tests. The copies were written with the test's own --update. Changing the HIP compile flags in CMakeLists.txt.jinja makes the test fail.

Authored with Claude.

Test Plan:
```
python test/symforce_caspar_codegen_test.py --update
python test/symforce_caspar_codegen_test.py
SYMFORCE_SYMBOLIC_API=sympy python test/symforce_caspar_codegen_test.py
```
The generated kernel and solver headers include cuda_to_hip.h, and so does host C++ that uses them: the generated solver.cc, the Python bindings, and the Caspar GPU tests, all compiled by the host compiler in a CUDA build. On the CUDA path the header also included cooperative_groups and CUB, which only compile under nvcc. A g++ build of caspar_padding_lane_cuda_test failed in cooperative_groups/details/sync.h ('threadIdx' was not declared).

Those includes, the cg alias and CG_REDUCE_SUM are only used by device code, so they are now included only when __CUDACC__ is defined. Host-only translation units get just the CUDA runtime API, which is what they included before the HIP port. The HIP path is unchanged: every file that includes the header is compiled as HIP there.

Authored with Claude.

Test Plan:
CUDA 12.8 (no NVIDIA GPU on this machine, so compile and link only):

```
cmake -S . -B build_cuda -DCMAKE_CUDA_COMPILER=/path/to/cuda-12.8/bin/nvcc
cmake --build build_cuda --target bin.tests.caspar_padding_lane_cuda_test bin.tests.caspar_solver_cuda_test bin.tests.caspar_tridiagonal_cuda_test bin.tests.caspar_backend_coverage_cuda_test
```

HIP, on an MI250X (gfx90a):

```
cmake --build build --target bin.tests.caspar_padding_lane_cuda_test bin.tests.caspar_solver_cuda_test bin.tests.caspar_tridiagonal_cuda_test bin.tests.caspar_backend_coverage_cuda_test
ctest --test-dir build -R caspar_
```
make lint runs clang-format --dry-run -Werror over the tree, and the HIP compatibility header was not formatted with the repository's .clang-format. Formatting only; no code change.

Authored with Claude.

Test Plan:
```
make lint
```
The ROCm branch of cuda_to_hip.h defined caspar_hip::reduce_sum, reduce_max, match_any_mask and labeled_reduce_sum, plus the CG_REDUCE_SUM and CG_LABELED_REDUCE_SUM macros, and the CUDA branch defined its own CG_REDUCE_SUM. Nothing in the tree calls any of them: memops.cuh and shared_indices.cu do their HIP reductions and copies inline. labeled_reduce_sum was also wrong when lanes with the same label are not contiguous, because its xor butterfly only adds a partner with the same label, so lanes 0 and 3 sharing a label never combine. Remove them so the header carries only the runtime API mapping. The cg namespace alias stays on the CUDA path.

Templates and macros that are never instantiated emit no code, so the device code objects of the Caspar test binaries are unchanged.

Authored with Claude.

Test Plan:

```
grep -rn 'caspar_hip\|CG_REDUCE_SUM\|CG_LABELED_REDUCE_SUM' symforce test
cmake -S . -B build -DCMAKE_BUILD_TYPE=Release -DCMAKE_HIP_ARCHITECTURES=gfx90a
cmake --build build
ctest --test-dir build -R caspar
cmake -S . -B build-cuda -DCMAKE_CUDA_COMPILER=nvcc
cmake --build build-cuda --target bin.tests.caspar_padding_lane_cuda_test bin.tests.caspar_solver_cuda_test bin.tests.caspar_tridiagonal_cuda_test bin.tests.caspar_backend_coverage_cuda_test
```
The README's AMD GPU section called a compile_caspar_library() function that does not exist. The API is the CasparLibrary.compile() static method, so the example now calls caslib.compile(out_dir, use_hip=True, hip_arch=...) after caslib.generate(out_dir), as the examples do.

The same section also described the compatibility header wrongly. It does not map __syncthreads or any cooperative groups spelling; it maps the CUDA runtime calls Caspar emits and the cub namespace. It also said hipcc works as the compiler, but the generated build uses CMake's HIP language support, which rejects the hipcc wrapper and needs the ROCm Clang compiler. The text now says so, and notes that without hip_arch CMake builds for the GPU it detects.

The comment above the HIP branch of GetDeviceId() in pybind_array_tools.cc still mentioned __device__ helpers in cuda_to_hip.h, which were removed, and local aliases the code never defines. It now says what the code does.

No code change outside the comment. Authored with Claude.

Test Plan:

```
cmake -S . -B build -DCMAKE_BUILD_TYPE=Release -DCMAKE_HIP_ARCHITECTURES=gfx90a
cmake --build build
ctest --test-dir build -R caspar --output-on-failure
ctest --test-dir build
python symforce/caspar/examples/kernel_example/gen_and_run.py  # with caslib.compile(out_dir, use_hip=True, hip_arch="gfx90a")
```
@jeffdaily

Copy link
Copy Markdown
Author

Note: this reply was drafted by an AI assistant.

Merged current main into the branch. The only conflict was an include in sort_indices.h.

On SumStore: after a768308 the trailing barrier from bjoernellens1's report isn't needed, so I took it back out. The NaN and run-to-run variation they saw fit #469. On an MI250X the padding-lane kernel without the register zeroing gives NaN at every padded size, with or without the barrier. With the zeroing, per-block sums are bit-identical over 200 runs whether or not the barrier is there, including the back-to-back SumStore kernel they pointed at. On a W7800 (wave32) that kernel matches an exact reference in every run, with or without the barrier. Thanks @bjoernellens1 for the report and the reproduction.

Tests: the *_cuda_test.cc tests now also build with HIP when CMake finds no CUDA compiler. With -DCMAKE_HIP_ARCHITECTURES set they build without an AMD GPU, so a ROCm container can compile every checked-in Caspar kernel and solver. symforce_caspar_codegen_test now also checks in the generated CMakeLists.txt, which is where the HIP build differs.

At the head I pushed before, a HIP build of a generated solver failed. The solver calls cudaGetDeviceCount (since the GPU device selection change, 2ee2978), and cuda_to_hip.h never mapped it. The new HIP build of the tests is what caught it, and it is mapped now. Running the tests on ROCm turned up one more bug: DoUpdateRFirst() never zeroed r_kp1_norm2_tot, so the first PCG step of each solver iteration added onto the previous iteration's value (or uninitialized memory on the first) before the PCG exit test read it. That's fixed in the template, and it affects CUDA too. A CUDA build of the tests also showed that host .cc files including the generated headers failed under g++, so cuda_to_hip.h now includes cooperative_groups and CUB only when compiling CUDA. I also removed some reduction helpers from cuda_to_hip.h that nothing used, and fixed the README's ROCm example, which called a function that doesn't exist.

Tested on an AMD Instinct MI250X and an AMD Radeon PRO W7800 on Linux (the Caspar GPU tests, the codegen tests and the rest of the test suite, apart from three tests that need tools those machines lack: rustfmt and a docs dependency) and an AMD Radeon 8060S on Windows (the Caspar GPU tests).

@aaron-skydio

Copy link
Copy Markdown
Member

Thanks! I'll try and get this merged when I have time

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants