Skip to content

yaksa: use unsigned GPU device counts - #7915

Open
jeffhammond wants to merge 1 commit into
pmodels:mainfrom
jeffhammond:fix-issue-6680
Open

yaksa: use unsigned GPU device counts#7915
jeffhammond wants to merge 1 commit into
pmodels:mainfrom
jeffhammond:fix-issue-6680

Conversation

@jeffhammond

Copy link
Copy Markdown
Member

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:

  • 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.

Pull Request Description

Author Checklist

  • Provide Description
    Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
  • Commits Follow Good Practice
    Commits are self-contained and do not do two things at once.
    Commit message is of the form: module: short description
    Commit message explains what's in the commit.
  • Passes All Tests
    Whitespace checker. Warnings test. Additional tests via comments.
  • Contribution Agreement
    For non-Argonne authors, check contribution agreement.
    If necessary, request an explicit comment from your companies PR approval manager.

Resolves #6680.

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.
@hzhou

hzhou commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

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:

If ndevices is ever negative, a number of loops are going to take a rather long time.

Exactly the danger of using unsigned type. Signed type prevents that.

The issue is really libc and many other library uses unsigned type, esp. size_t, causing dumb compiler warnings. I would simply use a cast and explicit conversion to suppress such warnings.

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.

Yaksa ndevices should probably be unsigned

2 participants