Skip to content

Isolate CUmemLocation construction in a helper - #2683

Open
juenglin wants to merge 2 commits into
NVIDIA:mainfrom
juenglin:custom-initializer
Open

Isolate CUmemLocation construction in a helper#2683
juenglin wants to merge 2 commits into
NVIDIA:mainfrom
juenglin:custom-initializer

Conversation

@juenglin

@juenglin juenglin commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Cython's aggregate struct-literal syntax (CUmemLocation(type=..., id=...)) warns "Not all members given for struct" whenever a call site doesn't name every member Cython knows about for that struct — and cuda.core promotes that warning to a build error. CUDA 13.4 adds a localized union member to CUmemLocation_st alongside the existing id, so once cydriver.pxd is regenerated from 13.4 headers, every existing CUmemLocation(type=..., id=...) call site in cuda.core fails to build, even though id is a complete initializer for the union arm actually being used.

This PR consolidates all CUmemLocation construction in cuda_core/cuda/core/_memory/_location.pxd into two small helpers that use field assignment internally (so the warning never fires there), while call sites keep compact constructor-style syntax:

  • to_cumemlocation(kind: str, loc_id: int) — for callers with a semantic kind ("device" / "host" / "host_numa" / "host_numa_current"), e.g. from _LocSpec.kind.
  • cumemlocation_from_type(loc_type: CUmemLocationType, loc_id: int) — for callers that already hold a CUmemLocationType enum value directly (pool configuration, peer-access, graph alloc nodes).

Every CUmemLocation-constructing call site across cuda_core now routes through one of these two helpers instead of duplicating field-assignment or aggregate-literal logic inline.

Relationship to #2673

Both PRs fix the same build failure. #2673 replaces every aggregate constructor with bare field assignment at each call site directly — minimal diff, already passed full CI, but it also removes the partial-initialization warning for this struct everywhere it touches. This PR keeps constructor-style call sites and centralizes the field-assignment logic in two helpers instead of duplicating it across call sites. The safety benefit is modest: Cython's aggregate-literal check isn't union-aware and can't verify "the correct arm was set" either way, so the real gain here is having one tested place for the mapping logic rather than six.

Test plan

  • Rebuild cuda_core against local CUDA 13.3 headers — no -Werror failures
  • cython-lint passes on all changed files
  • Memory, peer-access, managed-ops, pool, and graph-memory tests — 389 passed, 29 skipped

@copy-pr-bot

copy-pr-bot Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the cuda.core Everything related to the cuda.core module label Aug 21, 2026
@juenglin
juenglin force-pushed the custom-initializer branch from 03e7064 to a288474 Compare August 21, 2026 15:54
@juenglin juenglin self-assigned this Aug 21, 2026
@juenglin
juenglin requested a review from rwgk August 21, 2026 15:59
Build CUmemLocation via field assignment in to_cumemlocation() so cuda.core
compiles against both the 13.3 and 13.4 layouts. The localized arm is an
optional helper argument that exists only when CUDA_VERSION >= 13040.
@juenglin
juenglin force-pushed the custom-initializer branch from a288474 to d6c6c39 Compare August 21, 2026 16:36
@juenglin
juenglin force-pushed the custom-initializer branch from 61f4c66 to f9c4529 Compare August 21, 2026 18:55
@juenglin

Copy link
Copy Markdown
Contributor Author

/ok to test f9c4529

@github-actions

Copy link
Copy Markdown

@juenglin juenglin changed the title Isolate CUmemLocation construction in a versioned helper Isolate CUmemLocation construction in a helper Aug 21, 2026
@juenglin juenglin added the P0 High priority - Must do! label Aug 21, 2026
@juenglin
juenglin marked this pull request as ready for review August 21, 2026 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.core Everything related to the cuda.core module P0 High priority - Must do!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant