Skip to content

Commit b3c2c25

Browse files
author
Skyler Ruiter
committed
fixes hip compatability with new modules
1 parent c67eda5 commit b3c2c25

6 files changed

Lines changed: 51 additions & 19 deletions

File tree

‎include/backend/atomics.h‎

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,27 +12,52 @@
1212
* known to be in the same block, so scoping the atomic down from "device"
1313
* saves the extra memory-fence cost of a full device-wide atomic.
1414
*
15-
* ROCm's HIP declares `atomicAdd_block`/`atomicOr_block` with identical names
16-
* and signatures to CUDA's (`hip/amd_detail/amd_hip_atomic.h`) — unlike the
17-
* warp shuffle/ballot family (see warp.h), there is no warp/wavefront-width
18-
* dependency in a block-scope atomic, so naively this should "just hipify".
19-
* That said, this codebase has been burned before by CUDA intrinsics that
20-
* looked source-portable but silently diverged under HIP (see warp.h's
21-
* `__shfl_down_sync` width story), and — unlike every other intrinsic family
22-
* touched during the HIP port — these have **not yet been exercised or
23-
* verified on real AMD hardware** (no existing call site in this codebase
24-
* uses any block-scoped atomic). This header exists so there is exactly one
25-
* place to patch if that verification turns up a divergence, rather than
26-
* scattering raw `atomicAdd_block`/`atomicOr_block` calls through the RARE/RAZE
27-
* kernels. Treat the HIP branch below as "expected to work, not yet confirmed
28-
* on hardware" until it has been built and run on the project's MI100 target.
15+
* The `_block` suffix family is CUDA-only spelling. Contrary to this header's
16+
* original expectation ("ROCm declares them with identical names"), ROCm 6.4.1
17+
* declares neither `atomicAdd_block` nor `atomicOr_block` anywhere under
18+
* `hip/` — verified by grep against the installed toolchain, and by the
19+
* `use of undeclared identifier 'atomicAdd_block'` error the first HIP build
20+
* of the RARE/RAZE stages produced. This is exactly the divergence the header
21+
* was created to absorb, so the patch lands here and the call sites are
22+
* untouched.
23+
*
24+
* HIP's equivalent is the scoped-atomic builtin family
25+
* (`__hip_atomic_fetch_add`/`_or` + `__HIP_MEMORY_SCOPE_WORKGROUP`), where
26+
* "workgroup" is AMD's name for a CUDA block. Memory ordering is
27+
* `__ATOMIC_RELAXED` to match CUDA's `atomic*_block`, which carry no ordering
28+
* guarantees beyond the atomicity of the read-modify-write itself — the RARE/
29+
* RAZE call sites (per-block histogram accumulation; OR-ing a straddling
30+
* partial word into shared memory) separately `__syncthreads()` before reading
31+
* the accumulated result, so they rely on that barrier for ordering, not on
32+
* the atomic.
33+
*
34+
* Unlike the warp shuffle/ballot family (see warp.h), a block-scoped atomic has
35+
* no wavefront-width dependency, so there is no 32-vs-64-lane subtlety here.
2936
*/
3037

3138
#include "backend/api.h"
3239

3340
namespace fz {
3441
namespace backend {
3542

43+
#if defined(FZGMOD_BACKEND_HIP)
44+
45+
/** Block-scoped atomic add. `T` must be an arithmetic type the HIP scoped-atomic builtin accepts (int, unsigned, unsigned long long, float, double). */
46+
template <typename T>
47+
__device__ inline T atomicAddBlock(T* addr, T val) {
48+
return __hip_atomic_fetch_add(addr, val, __ATOMIC_RELAXED,
49+
__HIP_MEMORY_SCOPE_WORKGROUP);
50+
}
51+
52+
/** Block-scoped atomic bitwise-OR. `T` must be an integral type (int, unsigned, unsigned long long). */
53+
template <typename T>
54+
__device__ inline T atomicOrBlock(T* addr, T val) {
55+
return __hip_atomic_fetch_or(addr, val, __ATOMIC_RELAXED,
56+
__HIP_MEMORY_SCOPE_WORKGROUP);
57+
}
58+
59+
#else // CUDA
60+
3661
/** Block-scoped atomic add. `T` must be one of the types CUDA's `atomicAdd_block` overloads on (int, unsigned, unsigned long long, float, double). */
3762
template <typename T>
3863
__device__ inline T atomicAddBlock(T* addr, T val) {
@@ -45,5 +70,7 @@ __device__ inline T atomicOrBlock(T* addr, T val) {
4570
return atomicOr_block(addr, val);
4671
}
4772

73+
#endif
74+
4875
} // namespace backend
4976
} // namespace fz

‎modules/coders/adaptive_bitpack/adaptive_bitpack_kernels.cu‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,12 @@ __device__ __forceinline__ uint32_t warpBitTranspose32(uint32_t v, unsigned lane
4949
case 3: m = 0xFF00FF00u; break;
5050
default: m = 0xFFFF0000u; break;
5151
}
52-
const uint32_t t = __shfl_xor_sync(0xFFFFFFFFu, v, s);
52+
// Width pinned to 32: this butterfly is a 32-lane algorithm, and that is
53+
// what CUDA's implicit `width = warpSize` already meant. Under HIP the same
54+
// default is 64, which would pull lanes 32-63 into each exchange and corrupt
55+
// the transpose — see backend/warp.h failure mode 2. The wrapper also
56+
// supplies a 64-bit-wide mask, which HIP's __shfl_xor_sync static_asserts on.
57+
const uint32_t t = fz::backend::shflXor(v, static_cast<int>(s), 32);
5358
v = (lane & s) ? ((v & m) | ((t & m) >> s))
5459
: ((v & ~m) | ((t & ~m) << s));
5560
}

‎modules/coders/clog/clog_stage.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929

3030
#include "stage/stage.h"
3131
#include "fzm_format.h"
32-
#include <cuda_runtime.h>
32+
#include "backend/types.h"
3333
#include <cstdint>
3434
#include <cstring>
3535
#include <stdexcept>

‎modules/coders/hclog/hclog_stage.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232

3333
#include "stage/stage.h"
3434
#include "fzm_format.h"
35-
#include <cuda_runtime.h>
35+
#include "backend/types.h"
3636
#include <cstdint>
3737
#include <cstring>
3838
#include <stdexcept>

‎modules/coders/rare/rare_stage.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333

3434
#include "stage/stage.h"
3535
#include "fzm_format.h"
36-
#include <cuda_runtime.h>
36+
#include "backend/types.h"
3737
#include <cstdint>
3838
#include <cstring>
3939
#include <stdexcept>

‎modules/coders/raze/raze_stage.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232

3333
#include "stage/stage.h"
3434
#include "fzm_format.h"
35-
#include <cuda_runtime.h>
35+
#include "backend/types.h"
3636
#include <cstdint>
3737
#include <cstring>
3838
#include <stdexcept>

0 commit comments

Comments
 (0)