refactor: de-duplicate the dispatch helpers into shared headers

CLAUDE.md

@@ -59,7 +59,11 @@ impl/kernels ─ impl/cpu ─ api/cpu + src/lib-cpu ┐
  tensor's device and forwards to `FF_CPU::` or `FF_CUDA::`, the latter guarded
  by `FF_WITH_CUDA`.
 - **`include/fastfields/core/`** — `dlpack.h` (vendored, do not edit),
- `autocast.h` (32-bit index narrowing), `defines.h`, `cuda_switch.h`.
+ `autocast.h` (32-bit index narrowing), `defines.h`, `cuda_switch.h`,
+ `dispatch.h` (the `FF_VOIDPTR` / `FF_CANUSE32BITS` / `FF_CHECK_*` helpers
+ both dtype-dispatch layers share). `core/` is where anything used by
+ **both** `src/lib-cpu` (host compiler) and `src/lib-cuda` (nvcc) has to
+ live: it is the only directory that is backend-agnostic by contract.
 
 Also: `make/common.mk` (the shared makefile fragment), `tools/consolidate.sh`
 (the frozen consolidation rules), `ci/legacy/` and `docs/legacy/` (the six
@@ -158,6 +162,17 @@ pushpull's fully-static order×bound compile is nightly
  table lives above `MODULES` in `src/lib-cuda/Makefile`; read it before
  changing `-j`, `-O`, the bound/spline policy, or anything that makes a
  regulariser heavier. Do not recombine or "tidy" the split.
+- **Macros in installed headers are `FF_`-prefixed.** Anything `#define`d under
+ `include/` and not `#undef`'d before the end of that header is inherited by
+ every downstream translation unit, so it must be namespaced by prefix. Macros
+ private to a single `.cpp` are exempt — but the moment one is hoisted into a
+ header it stops being private, which is why de-duplicating a helper and
+ prefixing it are the same change. Two documented exemptions live in
+ `core/cuda_switch.h` and must keep their spelling to work at all: the
+ `__CUDACC_RTC__` fixed-width integer definitions (NVRTC ships no standard
+ library, so those *are* `<cstdint>` there) and the non-nvcc `__device__` /
+ `__host__` fallbacks. Prefer an `inline` function to a macro where one will
+ do — a function in `ff::` is collision-safe without any prefix.
 - `include/fastfields/core/dlpack.h` is vendored upstream code: do not edit it,
  and it is skipped by `codespell` (see `.codespellrc`).
 

include/fastfields/api/checks.h

@@ -22,6 +22,31 @@ FF_NAMESPACE_BEGIN(FF)
  */
 inline void require_same_device(const DLTensor & /*ref*/) {}
 
+/**
+ * The hub's device predicates.
+ *
+ * Every `src/lib/<module>.cpp` carried its own `IS_CPU` / `IS_CUDA` macro pair
+ * -- nine identical copies, and two more unprefixed macros leaking out of the
+ * translation units that defined them. They are plain predicates over a POD
+ * field, so they are stated here as inline functions instead: a function in
+ * `ff::` cannot collide with a downstream identifier the way a bare `IS_CPU`
+ * macro can, and so it needs no prefix to be safe. Two of the 31 unprefixed
+ * macros are thus removed rather than renamed.
+ *
+ * `kDLCUDAHost` is pinned (page-locked) *host* memory: addressable by the CPU,
+ * so it belongs on the CPU side of the dispatch, not the CUDA one.
+ */
+inline bool is_cuda(const DLTensor & t)
+{
+ return t.device.device_type == DLDeviceType::kDLCUDA;
+}
+
+inline bool is_cpu(const DLTensor & t)
+{
+ return t.device.device_type == DLDeviceType::kDLCPU ||
+ t.device.device_type == DLDeviceType::kDLCUDAHost;
+}
+
 template <class... Rest>
 inline void require_same_device(const DLTensor & ref, const DLTensor & t, const Rest &... rest) {
  if (t.device.device_type != ref.device.device_type ||

include/fastfields/api/cpu/pushpull_dispatch.h

@@ -17,6 +17,7 @@
 #include <stdexcept>
 #include <cstdint>
 #include "fastfields/core/autocast.h"
+#include "fastfields/core/dispatch.h"
 #include "fastfields/core/dlpack.h"
 #include "fastfields/core/cuda_switch.h"
 #include "fastfields/impl/kernels/utils.h"
@@ -25,36 +26,10 @@
 FF_NAMESPACE_BEGIN(FF)
 FF_NAMESPACE_BEGIN(FF_DEVICE)
 
-#define VOIDPTR(x) (static_cast<void*>(static_cast<char*>(x.data) + x.byte_offset))
-#define CVOIDPTR(x) (static_cast<const void*>(static_cast<const char*>(x.data) + x.byte_offset))
-#define CANUSE32BITS(x) (canUse32BitIndexMath(x.ndim, x.shape, x.strides))
-
 // reduce/accumulation type used by the sampling kernels. Match jitfields
 // (float64) for CPU accuracy.
 typedef double reduce_t;
 
-/***********************************************************************
- * CHECKS *
- ***********************************************************************/
-
-#define CHECK_NO_LANES(tensor) \
- if (tensor.dtype.lanes > 1) \
- throw std::invalid_argument("Only scalar data types are supported");
-
-#define CHECK_SAME(X, Y, msg) \
- if (X != Y) throw std::invalid_argument(msg);
-
-#define CHECK_SAME_DTYPE(X, Y) \
- if ((X.dtype.code != Y.dtype.code) || \
- (X.dtype.bits != Y.dtype.bits) || \
- (X.dtype.lanes != Y.dtype.lanes)) \
- throw std::invalid_argument("Tensors do not have the same data type");
-
-#define CHECK_SAME_BATCH(X, Y, D) \
- for (int32_t d=0; d < D; ++d) \
- if (X.shape[d] != Y.shape[d]) \
- throw std::invalid_argument("Tensors do not have the same batch shape");
-
 /***********************************************************************
  * DISPATCH *
  ***********************************************************************/

include/fastfields/api/cuda/pushpull_dispatch.h

@@ -17,6 +17,7 @@
 #include <stdexcept>
 #include <cstdint>
 #include "fastfields/core/autocast.h"
+#include "fastfields/core/dispatch.h"
 #include "fastfields/core/dlpack.h"
 #include "fastfields/core/cuda_switch.h"
 #include "fastfields/impl/kernels/utils.h"
@@ -25,36 +26,10 @@
 FF_NAMESPACE_BEGIN(FF)
 FF_NAMESPACE_BEGIN(FF_DEVICE)
 
-#define VOIDPTR(x) (static_cast<void*>(static_cast<char*>(x.data) + x.byte_offset))
-#define CVOIDPTR(x) (static_cast<const void*>(static_cast<const char*>(x.data) + x.byte_offset))
-#define CANUSE32BITS(x) (canUse32BitIndexMath(x.ndim, x.shape, x.strides))
-
 // reduce/accumulation type used by the sampling kernels. Match jitfields
 // (float64) for CPU accuracy.
 typedef double reduce_t;
 
-/***********************************************************************
- * CHECKS *
- ***********************************************************************/
-
-#define CHECK_NO_LANES(tensor) \
- if (tensor.dtype.lanes > 1) \
- throw std::invalid_argument("Only scalar data types are supported");
-
-#define CHECK_SAME(X, Y, msg) \
- if (X != Y) throw std::invalid_argument(msg);
-
-#define CHECK_SAME_DTYPE(X, Y) \
- if ((X.dtype.code != Y.dtype.code) || \
- (X.dtype.bits != Y.dtype.bits) || \
- (X.dtype.lanes != Y.dtype.lanes)) \
- throw std::invalid_argument("Tensors do not have the same data type");
-
-#define CHECK_SAME_BATCH(X, Y, D) \
- for (int32_t d=0; d < D; ++d) \
- if (X.shape[d] != Y.shape[d]) \
- throw std::invalid_argument("Tensors do not have the same batch shape");
-
 /***********************************************************************
  * DISPATCH *
  ***********************************************************************/

include/fastfields/api/cuda/stream.h

@@ -0,0 +1,33 @@
+#pragma once
+#ifndef FF_CUDA_STREAM
+#define FF_CUDA_STREAM
+
+/**
+ * The public ABI carries a CUDA stream as an `intptr_t` (no CUDA types leak
+ * into the exported signatures); the cuda-impl launchers take a real
+ * `cudaStream_t`. This is the one-line conversion between them, which had been
+ * copied verbatim into all four regulariser dispatch sources
+ * (`reg_field.cpp`, `reg_field_rls.cpp`, `reg_flow.cpp`, `reg_flow_rls.cpp`).
+ *
+ * CUDA-only by construction -- it names `cudaStream_t` -- so it lives under
+ * `api/cuda/` rather than in `core/`, and only `src/lib-cuda` includes it.
+ * `pushpull` has its own `_pp_stream` in the cuda-impl layer; the two are left
+ * separate deliberately, as that one sits a layer below.
+ */
+
+#include <cstdint>
+#include "fastfields/core/cuda_switch.h"
+
+FF_NAMESPACE_BEGIN(FF)
+FF_NAMESPACE_BEGIN(FF_DEVICE)
+
+// intptr_t -> cudaStream_t (0 == the default stream).
+static inline cudaStream_t _reg_stream(intptr_t stream)
+{
+ return reinterpret_cast<cudaStream_t>(static_cast<std::intptr_t>(stream));
+}
+
+FF_NAMESPACE_END(FF_DEVICE)
+FF_NAMESPACE_END(FF)
+
+#endif // FF_CUDA_STREAM

include/fastfields/core/dispatch.h

@@ -0,0 +1,149 @@
+#pragma once
+#ifndef FF_CORE_DISPATCH
+#define FF_CORE_DISPATCH
+
+/**
+ * The helpers shared by both dtype-dispatch layers (`src/lib-cpu` and
+ * `src/lib-cuda`): unpack the public pointer ABI's arguments into what the
+ * templated impl layer wants, and reject the combinations it cannot express.
+ *
+ * Every `src/lib-cpu/<module>.cpp` and `src/lib-cuda/<module>.cpp` did this
+ * the same way -- add `byte_offset` to `data`, decide whether the shape and
+ * stride arrays fit in 32 bits, validate the operands -- and each one carried
+ * its own copy of the macros. There were 19 copies of `VOIDPTR`, 19 of
+ * `CANUSE32BITS`, 19 of `CHECK_NO_LANES` and 17 of `CHECK_SAME`, and they had
+ * already drifted apart; see the three preserved divergences below.
+ *
+ * Why `core/` and not `api/`
+ * --------------------------------------------------------------------------
+ * `src/lib-cuda` is compiled by **nvcc** while `src/lib-cpu` and `src/lib` use
+ * the host compiler, so anything all of them share has to be backend-agnostic.
+ * That is exactly what `include/fastfields/core/` is for. `api/checks.h` is
+ * the hub's (host-only) validation header and is the wrong home for macros
+ * nvcc must also digest. Note the CI consequence, which is intended: a change
+ * under `core/` triggers every job, CUDA included.
+ *
+ * Usage notes
+ * --------------------------------------------------------------------------
+ * * `FF_CANUSE32BITS` calls `canUse32BitIndexMath` unqualified, so it must be
+ * expanded from inside `ff::cpu` / `ff::cuda` (i.e. `ff::FF_DEVICE`, where
+ * `core/autocast.h` declares it). Every dispatch source already is. It is
+ * left unqualified on purpose: qualifying it would change name lookup, and
+ * this header's contract is that the macros expand token-for-token to what
+ * the 19 local copies expanded to.
+ * * The `FF_CHECK_*` macros expand to bare `if` / `for` statements, NOT to a
+ * `do { ... } while (0)`. That is deliberate: it is what the copies did, and
+ * wrapping them would silently change which statements a brace-less
+ * `if (cond) FF_CHECK_...(...);` guards. They are therefore not safe as the
+ * body of an unbraced `if` / `else`. Tightening that is a behaviour change
+ * and belongs in its own commit, measured against the CPU suite.
+ */
+
+#include <cstddef> // size_t
+#include <cstdint> // int32_t (the shape loops), int64_t
+#include <stdexcept> // std::invalid_argument
+#include <vector> // as_weights
+#include "fastfields/core/dlpack.h"
+#include "fastfields/core/defines.h" // FF_NAMESPACE_*
+#include "fastfields/core/autocast.h" // canUse32BitIndexMath
+
+/***********************************************************************
+ * POINTER MARSHALLING *
+ ***********************************************************************/
+
+// DLPack keeps the element offset out of `data`, so every call into the impl
+// layer has to fold it back in.
+#define FF_VOIDPTR(x) (static_cast<void*>(static_cast<char*>(x.data) + x.byte_offset))
+#define FF_CVOIDPTR(x) (static_cast<const void*>(static_cast<const char*>(x.data) + x.byte_offset))
+
+// PRESERVED DIVERGENCE 1/3. `posdef.cpp` -- and only posdef.cpp -- guarded
+// its CVOIDPTR against a null `data`: an absent optional operand is passed as
+// a descriptor with `data == nullptr`, and offsetting a null pointer is
+// undefined behaviour even when the result is never dereferenced. Given its
+// own name rather than folded into FF_CVOIDPTR, so the difference is visible
+// at the call site instead of hiding in one file's private prologue.
+#define FF_CVOIDPTR_OR_NULL(x) (x.data ? FF_CVOIDPTR(x) : nullptr)
+
+// Can this tensor's shape/stride arithmetic be narrowed to 32-bit offsets?
+// See core/autocast.h.
+#define FF_CANUSE32BITS(x) (canUse32BitIndexMath(x.ndim, x.shape, x.strides))
+
+/***********************************************************************
+ * CHECKS *
+ ***********************************************************************/
+
+#define FF_CHECK_NO_LANES(tensor) \
+ if (tensor.dtype.lanes > 1) \
+ throw std::invalid_argument("Only scalar data types are supported");
+
+#define FF_CHECK_SAME(X, Y, msg) \
+ if (X != Y) throw std::invalid_argument(msg);
+
+#define FF_CHECK_SAME_DTYPE(X, Y) \
+ if ((X.dtype.code != Y.dtype.code) || \
+ (X.dtype.bits != Y.dtype.bits) || \
+ (X.dtype.lanes != Y.dtype.lanes)) \
+ throw std::invalid_argument("Tensors do not have the same data type");
+
+// Agreement on the leading `D` (batch) dimensions.
+#define FF_CHECK_SAME_BATCH(X, Y, D) \
+ for (int32_t d=0; d < D; ++d) \
+ if (X.shape[d] != Y.shape[d]) \
+ throw std::invalid_argument("Tensors do not have the same batch shape");
+
+// PRESERVED DIVERGENCE 2/3. As above, but first rejects tensors with fewer
+// than `D` dimensions rather than reading past the end of `shape`.
+// `distance.cpp` and `posdef.cpp` are the two that need it -- they derive `D`
+// from an operand's own `ndim` -- and were the only two that had it.
+#define FF_CHECK_SAME_BATCH_ND(X, Y, D) \
+ if (X.ndim < D || Y.ndim < D) \
+ throw std::invalid_argument("Number of dimensions does not match"); \
+ FF_CHECK_SAME_BATCH(X, Y, D)
+
+// PRESERVED DIVERGENCE 3/3. `CHECK_SAME_SHAPE` was two *different macros
+// sharing one name*: a 3-argument leading-D check in the regularisers and
+// solve_field (this one), and a 2-argument whole-shape check in distance.cpp
+// (below). Merging them under one name would have silently changed one set of
+// call sites, which is the sharpest illustration of why 17 private copies of a
+// macro is a hazard rather than a tidiness complaint.
+#define FF_CHECK_SAME_SHAPE_N(X, Y, D) \
+ for (int32_t d=0; d < D; ++d) \
+ if (X.shape[d] != Y.shape[d]) \
+ throw std::invalid_argument("Tensors do not have the same shape");
+
+// Agreement on the WHOLE shape, rank included.
+#define FF_CHECK_SAME_SHAPE(X, Y) \
+ if (X.ndim != Y.ndim) \
+ throw std::invalid_argument("Tensors do not have the same number of dimensions"); \
+ FF_CHECK_SAME_BATCH_ND(X, Y, X.ndim)
+
+/***********************************************************************
+ * NON-TENSOR ARGUMENT MARSHALLING *
+ ***********************************************************************/
+
+FF_NAMESPACE_BEGIN(FF)
+
+/**
+ * Build a length-`nc` penalty-weight vector from the ABI's `const double *`.
+ *
+ * The regulariser entry points take each energy term's weight as a raw pointer
+ * that is either an `nc`-long array or null ("this term is off"); the impl
+ * layer takes a filled vector. Was copied verbatim into
+ * `src/lib-cpu/reg_field.cpp` and `src/lib-cuda/reg_field{,_rls}.cpp`.
+ *
+ * Returns `std::vector<double>` rather than the dispatch sources' local
+ * `reduce_t` typedef, which is `double` in every one of them. If a backend
+ * ever changes its accumulation type, the assignment at the call site stops
+ * compiling -- which is the failure you want, rather than a silent narrowing
+ * inside a shared header.
+ */
+inline std::vector<double> as_weights(const double * w, int64_t nc)
+{
+ std::vector<double> v(static_cast<size_t>(nc), 0.0);
+ if (w) for (int64_t c = 0; c < nc; ++c) v[static_cast<size_t>(c)] = w[c];
+ return v;
+}
+
+FF_NAMESPACE_END(FF)
+
+#endif // FF_CORE_DISPATCH