Found by:claude-jitfields-to-fastfields, during the shared-macro de-duplication (#90/#91). Deliberately left unfixed there so it could be judged on its own.
include/fastfields/core/cuda_switch.h, inside the #ifdef __CUDACC_RTC__ block that hand-defines the fixed-width integer types (NVRTC ships no standard library, so the block itself is legitimate):
#defineint64_t long // 32-bit on LLP64 (Win64)
#defineuint64_t unsigned long // ditto
long is 64-bit under LP64 (Linux, macOS) but 32-bit under LLP64, which is Win64. The portable spelling is long long / unsigned long long, which is 64-bit on both. The 8-, 16- and 32-bit definitions in the same block are fine.
Why this is worse here than a generic portability nit
offset_t is int32_t or int64_t, and the entire canUse32BitIndexMath / autocast.h machinery exists to decide when narrowing indices to 32 bits is safe — falling back to 64-bit when a tensor is too large.
If int64_t silently is 32 bits, the fallback is also 32 bits. The safety check still passes, the narrow path and the "safe" path become the same path, and large tensors overflow their index arithmetic. That is silently wrong results, not a compile error and not a crash — the failure mode this codebase is least equipped to detect, since there is no GPU in CI and the CPU suite would never exercise an NVRTC build.
Current exposure: latent
This block only compiles under __CUDACC_RTC__ (NVRTC / runtime JIT). The project builds ahead-of-time with nvcc, so nothing hits it today. It becomes live if:
- anything adopts NVRTC/JIT compilation (note the predecessor project
jitfields was JIT-based, so this is not far-fetched), and - that runs on Windows.
Windows support is a stated future goal for this repo, which is why this is worth fixing now while it is a two-token change rather than after someone spends a day on a large-tensor bug.
Fix
#defineint64_t long long
#defineuint64_t unsigned long long
Worth a brief comment recording whylong is wrong, so it does not get "simplified" back.
Related
Found by:
claude-jitfields-to-fastfields, during the shared-macro de-duplication (#90/#91). Deliberately left unfixed there so it could be judged on its own.include/fastfields/core/cuda_switch.h, inside the#ifdef __CUDACC_RTC__block that hand-defines the fixed-width integer types (NVRTC ships no standard library, so the block itself is legitimate):longis 64-bit under LP64 (Linux, macOS) but 32-bit under LLP64, which is Win64. The portable spelling islong long/unsigned long long, which is 64-bit on both. The 8-, 16- and 32-bit definitions in the same block are fine.Why this is worse here than a generic portability nit
offset_tisint32_torint64_t, and the entirecanUse32BitIndexMath/autocast.hmachinery exists to decide when narrowing indices to 32 bits is safe — falling back to 64-bit when a tensor is too large.If
int64_tsilently is 32 bits, the fallback is also 32 bits. The safety check still passes, the narrow path and the "safe" path become the same path, and large tensors overflow their index arithmetic. That is silently wrong results, not a compile error and not a crash — the failure mode this codebase is least equipped to detect, since there is no GPU in CI and the CPU suite would never exercise an NVRTC build.Current exposure: latent
This block only compiles under
__CUDACC_RTC__(NVRTC / runtime JIT). The project builds ahead-of-time withnvcc, so nothing hits it today. It becomes live if:jitfieldswas JIT-based, so this is not far-fetched), andWindows support is a stated future goal for this repo, which is why this is worth fixing now while it is a two-token change rather than after someone spends a day on a large-tensor bug.
Fix
Worth a brief comment recording why
longis wrong, so it does not get "simplified" back.Related
FF_-prefixing work that surfaced this. Neither changes this block.__device__/__host__fallbacks in the same header were checked at the same time and are correctly guarded (inside#ifndef __CUDACC__, each behind its own#ifndefso a pre-existing definition wins). One noted caveat, not a bug: they are never#undef'd, so a host TU carries them as empty macros for the rest of the TU — harmless, since#ifndef __CUDACC__implies no CUDA compilation.