yaksa: use unsigned GPU device counts - #7915
Conversation
Issue pmodels#6680 reports a GCC allocation-size warning in the CUDA backend because ndevices is stored as a signed int and then used as the element count for calloc and malloc. A negative device count is not meaningful for CUDA, HIP, or ZE, and leaving it signed lets GCC reason about an impossible negative-to-size_t conversion. Store GPU device counts as unsigned in the shared Yaksa GPU hook plumbing and the CUDA/HIP backend globals. CUDA and HIP still receive the count through the vendor API's int output parameter, then explicitly reject negative values before converting to unsigned. Using plain unsigned fixes the signed allocation-size issue without requiring an extra stdint include in the shared yaksuri.h header. ZE already uses uint32_t internally for the Level Zero API calls, so this only updates its shared hook signature to return the count through an unsigned pointer. Device indices passed back into CUDA/HIP APIs are cast to int; those values originate from the nonnegative vendor device count, so the casts do not broaden the representable range. The shared buffer-pool callback avoids computing device[ndevices - 1] when the backend has no devices, which prevents unsigned underflow while preserving the existing host-pool path. Verification: - Rebuilt the existing CUDA-enabled GCC build directory with make -j50 after the unsigned adjustment; this rebuilt yaksur_hooks.lo, yaksuri_cuda_init_hooks.lo, relinked libyaksa.la and libmpi.la, and completed successfully. - Compiled yaksuri_cuda_init_hooks.c with the system default gcc, -Wall, and -Werror=alloc-size-larger-than=9223372036854775807 using the CUDA-enabled Yaksa build headers; this passed. The system default gcc is GCC 16 trunk, which CUDA 12.9 nvcc does not accept for its own configure probe in this environment, so the CUDA-enabled full build directory was configured with GCC 11 as the nvcc host compiler while the targeted warning check uses the default gcc directly.
|
Unless you feel strongly about it, I personally prefer use signed int unless we need the extra range. The reason is, signed number is more robust in arithmetic. Typical small values lives at the edge of zero and can very easily become negative. The signed type preserves the value and have more stable behavior. It is more likely for some assertions to catch negative values, and it is easier to debug since we can see the value being negative. On the other hand, unsigned value underflows easily and there is no good way to defensively assert. Your comment:
Exactly the danger of using unsigned type. Signed type prevents that. The issue is really libc and many other library uses unsigned type, esp. |
Issue #6680 reports a GCC allocation-size warning in the CUDA backend because ndevices is stored as a signed int and then used as the element count for calloc and malloc. A negative device count is not meaningful for CUDA, HIP, or ZE, and leaving it signed lets GCC reason about an impossible negative-to-size_t conversion.
Store GPU device counts as unsigned in the shared Yaksa GPU hook plumbing and the CUDA/HIP backend globals. CUDA and HIP still receive the count through the vendor API's int output parameter, then explicitly reject negative values before converting to unsigned. Using plain unsigned fixes the signed allocation-size issue without requiring an extra stdint include in the shared yaksuri.h header.
ZE already uses uint32_t internally for the Level Zero API calls, so this only updates its shared hook signature to return the count through an unsigned pointer. Device indices passed back into CUDA/HIP APIs are cast to int; those values originate from the nonnegative vendor device count, so the casts do not broaden the representable range.
The shared buffer-pool callback avoids computing device[ndevices - 1] when the backend has no devices, which prevents unsigned underflow while preserving the existing host-pool path.
Verification:
The system default gcc is GCC 16 trunk, which CUDA 12.9 nvcc does not accept for its own configure probe in this environment, so the CUDA-enabled full build directory was configured with GCC 11 as the nvcc host compiler while the targeted warning check uses the default gcc directly.
Pull Request Description
Author Checklist
Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
Commits are self-contained and do not do two things at once.
Commit message is of the form:
module: short descriptionCommit message explains what's in the commit.
Whitespace checker. Warnings test. Additional tests via comments.
For non-Argonne authors, check contribution agreement.
If necessary, request an explicit comment from your companies PR approval manager.
Resolves #6680.