Skip to content

Commit f9c4529

Browse files
committed
address review feedback
1 parent d6c6c39 commit f9c4529

8 files changed

Lines changed: 60 additions & 139 deletions

File tree

‎cuda_core/build_hooks.py‎

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -121,31 +121,6 @@ def _determine_cuda_major_version() -> str:
121121
)
122122

123123

124-
@functools.cache
125-
def _cuda_core_has_localized_location() -> bool:
126-
"""Whether CUmemLocation exposes the ``localized`` union arm (CUDA 13.4+).
127-
128-
CUDA 13.4 adds ``CUmemLocation.localized`` and
129-
``CU_MEM_LOCATION_TYPE_DEVICE_LOCALITY_DOMAIN``. This flag is independent of
130-
``CUDA_CORE_BUILD_MAJOR`` so 13.3 and 13.4 can share a major version while
131-
still compiling different ``to_cumemlocation`` signatures.
132-
"""
133-
override = os.environ.get("CUDA_CORE_HAS_LOCALIZED_LOCATION")
134-
if override is not None:
135-
return bool(int(override))
136-
cuda_path = _get_cuda_path()
137-
cuda_h = os.path.join(cuda_path, "include", "cuda.h")
138-
try:
139-
with open(cuda_h, encoding="utf-8") as f:
140-
for line in f:
141-
m = re.match(r"^#\s*define\s+CUDA_VERSION\s+(\d+)\s*$", line)
142-
if m:
143-
return int(m.group(1)) >= 13040
144-
except OSError:
145-
pass
146-
return False
147-
148-
149124
# used later by setup()
150125
_extensions = None
151126

@@ -245,10 +220,7 @@ def get_sources(mod_name):
245220
)
246221

247222
nthreads = int(os.environ.get("CUDA_PYTHON_PARALLEL_LEVEL", os.cpu_count() // 2))
248-
compile_time_env = {
249-
"CUDA_CORE_BUILD_MAJOR": int(_determine_cuda_major_version()),
250-
"CUDA_CORE_HAS_LOCALIZED_LOCATION": int(_cuda_core_has_localized_location()),
251-
}
223+
compile_time_env = {"CUDA_CORE_BUILD_MAJOR": int(_determine_cuda_major_version())}
252224
compiler_directives = {"embedsignature": True, "warn.deprecated.IF": False, "freethreading_compatible": True}
253225
_CythonOptions.warning_errors = True
254226
if COMPILE_FOR_COVERAGE:

‎cuda_core/cuda/core/_memory/_device_memory_resource.pyx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from __future__ import annotations
66

77
from cuda.bindings cimport cydriver
8-
from cuda.core._memory._location cimport to_cumemlocation
8+
from cuda.core._memory._location cimport cumemlocation_from_type
99
from cuda.core._memory._memory_pool cimport (
1010
_MemPool, MP_init_create_pool, MP_raise_release_threshold,
1111
)
@@ -322,7 +322,8 @@ cpdef str DMR_mempool_get_access(DeviceMemoryResource dmr, int device_id):
322322

323323
cdef int dev_id = Device(device_id).device_id
324324
cdef cydriver.CUmemAccess_flags flags
325-
cdef cydriver.CUmemLocation location = to_cumemlocation("device", dev_id)
325+
cdef cydriver.CUmemLocation location = cumemlocation_from_type(
326+
cydriver.CUmemLocationType.CU_MEM_LOCATION_TYPE_DEVICE, dev_id)
326327

327328
with nogil:
328329
HANDLE_RETURN(cydriver.cuMemPoolGetAccess(&flags, as_cu(dmr._h_pool), &location))

‎cuda_core/cuda/core/_memory/_location.pxd‎

Lines changed: 40 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2,69 +2,64 @@
22
#
33
# SPDX-License-Identifier: Apache-2.0
44

5-
# Conversion from the internal ``_LocSpec`` record produced by
6-
# ``_managed_location._coerce_location`` to the driver's ``CUmemLocation``.
5+
# Conversion helpers for the driver's ``CUmemLocation`` struct.
76
#
87
# Header-only so both the managed-memory ops and the batched copy path can
98
# cimport it without either module depending on the other. ``CUmemLocation``
109
# is only populated on a CUDA 13 build; the CUDA 12 stub exists so callers
1110
# compiled there still resolve the symbol.
1211
#
13-
# Construction uses field assignment rather than Cython struct literals so
14-
# the same source compiles against both the CUDA 13.3 two-member declaration
15-
# and the CUDA 13.4 declaration that adds the ``localized`` union arm.
16-
# ``CUDA_CORE_HAS_LOCALIZED_LOCATION`` selects a helper signature with an
17-
# optional ``localized`` argument (13.4+) or without it (13.3). Passing
18-
# ``localized=...`` is therefore a Cython compile-time error on 13.3.
12+
# Both helpers use field assignment rather than Cython struct literals
13+
# (``CUmemLocation(type=..., id=...)``) so this source keeps compiling if a
14+
# future generated ``cydriver.pxd`` adds a sibling member to the struct's
15+
# anonymous union (e.g. CUDA 13.4's ``localized`` arm): Cython's struct-literal
16+
# coercion warns "Not all members given for struct" whenever a call site does
17+
# not name every declared member, and cuda_core promotes that warning to a
18+
# build error.
1919

2020
from cuda.bindings cimport cydriver
2121

2222

2323
IF CUDA_CORE_BUILD_MAJOR >= 13:
24-
cdef inline void _fill_id_location(
25-
cydriver.CUmemLocation* cu_loc, str kind, int loc_id
26-
) except *:
24+
cdef inline cydriver.CUmemLocation cumemlocation_from_type(
25+
cydriver.CUmemLocationType loc_type, int loc_id
26+
):
27+
"""Build a ``CUmemLocation`` from an already-known ``CUmemLocationType``.
28+
29+
For call sites that already carry a ``CUmemLocationType`` value
30+
(e.g. from a pool-configuration parameter), rather than the
31+
``kind`` string used by :func:`to_cumemlocation`.
32+
"""
33+
cdef cydriver.CUmemLocation cu_loc
34+
cu_loc.type = loc_type
35+
cu_loc.id = loc_id
36+
return cu_loc
37+
38+
cdef inline cydriver.CUmemLocation to_cumemlocation(str kind, int loc_id):
2739
if kind == "device":
28-
cu_loc.type = cydriver.CUmemLocationType.CU_MEM_LOCATION_TYPE_DEVICE
29-
cu_loc.id = loc_id
40+
return cumemlocation_from_type(
41+
cydriver.CUmemLocationType.CU_MEM_LOCATION_TYPE_DEVICE, loc_id)
3042
elif kind == "host":
31-
cu_loc.type = cydriver.CUmemLocationType.CU_MEM_LOCATION_TYPE_HOST
32-
cu_loc.id = 0
43+
return cumemlocation_from_type(
44+
cydriver.CUmemLocationType.CU_MEM_LOCATION_TYPE_HOST, 0)
3345
elif kind == "host_numa":
34-
cu_loc.type = cydriver.CUmemLocationType.CU_MEM_LOCATION_TYPE_HOST_NUMA
35-
cu_loc.id = loc_id
46+
return cumemlocation_from_type(
47+
cydriver.CUmemLocationType.CU_MEM_LOCATION_TYPE_HOST_NUMA, loc_id)
3648
elif kind == "host_numa_current":
37-
cu_loc.type = cydriver.CUmemLocationType.CU_MEM_LOCATION_TYPE_HOST_NUMA_CURRENT
38-
cu_loc.id = 0
49+
return cumemlocation_from_type(
50+
cydriver.CUmemLocationType.CU_MEM_LOCATION_TYPE_HOST_NUMA_CURRENT, 0)
3951
else:
4052
raise ValueError(f"unknown location kind: {kind!r}")
41-
42-
IF CUDA_CORE_HAS_LOCALIZED_LOCATION:
43-
cdef inline cydriver.CUmemLocation to_cumemlocation(
44-
str kind, int loc_id=0, tuple localized=None
45-
):
46-
cdef cydriver.CUmemLocation cu_loc
47-
if kind == "device_locality_domain":
48-
if localized is None:
49-
raise ValueError(
50-
"kind='device_locality_domain' requires "
51-
"localized=(device_id, locality_domain_id)"
52-
)
53-
cu_loc.type = (
54-
cydriver.CUmemLocationType.CU_MEM_LOCATION_TYPE_DEVICE_LOCALITY_DOMAIN
55-
)
56-
cu_loc.localized.deviceId = localized[0]
57-
cu_loc.localized.localityDomainId = localized[1]
58-
return cu_loc
59-
_fill_id_location(&cu_loc, kind, loc_id)
60-
return cu_loc
61-
ELSE:
62-
cdef inline cydriver.CUmemLocation to_cumemlocation(str kind, int loc_id=0):
63-
cdef cydriver.CUmemLocation cu_loc
64-
_fill_id_location(&cu_loc, kind, loc_id)
65-
return cu_loc
6653
ELSE:
67-
cdef inline cydriver.CUmemLocation to_cumemlocation(str kind, int loc_id=0):
54+
cdef inline cydriver.CUmemLocation cumemlocation_from_type(
55+
cydriver.CUmemLocationType loc_type, int loc_id
56+
):
57+
cdef cydriver.CUmemLocation cu_loc
58+
cu_loc.type = loc_type
59+
cu_loc.id = loc_id
60+
return cu_loc
61+
62+
cdef inline cydriver.CUmemLocation to_cumemlocation(str kind, int loc_id):
6863
raise NotImplementedError(
6964
"CUmemLocation requires cuda.core built against CUDA 13 headers"
7065
)

‎cuda_core/cuda/core/_memory/_managed_memory_ops.pyx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@ IF CUDA_CORE_BUILD_MAJOR >= 13:
1313
from cuda.bindings cimport cydriver
1414
from cuda.core._memory._buffer cimport Buffer, Buffer_coerce_batch
1515

16-
# to_cumemlocation is referenced only from CUDA 13 branches. cython-lint does
17-
# not evaluate compile-time IF blocks, so it needs a pragma to be seen as used.
16+
# to_cumemlocation / cumemlocation_from_type are referenced only from CUDA 13
17+
# branches. cython-lint does not evaluate compile-time IF blocks, so they
18+
# need a pragma to be seen as used.
19+
from cuda.core._memory._location cimport cumemlocation_from_type # no-cython-lint
1820
from cuda.core._memory._location cimport to_cumemlocation # no-cython-lint
1921
from cuda.core._resource_handles cimport as_cu
2022
from cuda.core._stream cimport Stream, Stream_accept
@@ -193,7 +195,8 @@ cdef void _do_single_advise(Buffer buf, object advice_value, object loc, bint al
193195
# Driver ignores location for read_mostly / unset_preferred_location
194196
# advice values but still validates the CUmemLocation; pass a
195197
# host placeholder.
196-
cu_loc = to_cumemlocation("host", 0)
198+
cu_loc = cumemlocation_from_type(
199+
cydriver.CUmemLocationType.CU_MEM_LOCATION_TYPE_HOST, 0)
197200
else:
198201
cu_loc = to_cumemlocation(loc.kind, loc.id)
199202
with nogil:

‎cuda_core/cuda/core/_memory/_memory_pool.pyx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ from libc.string cimport memset
1212
from cuda.bindings cimport cydriver
1313
from cuda.core._memory._buffer cimport Buffer, Buffer_from_deviceptr_handle, MemoryResource
1414
from cuda.core._memory cimport _ipc
15+
# cumemlocation_from_type is referenced only from CUDA 13 branches. cython-lint
16+
# does not evaluate compile-time IF blocks, so it needs a pragma to be seen as used.
17+
from cuda.core._memory._location cimport cumemlocation_from_type # no-cython-lint
1518
from cuda.core._stream cimport Stream_accept, Stream
1619
from cuda.core._resource_handles cimport (
1720
MemoryPoolHandle,
@@ -279,8 +282,7 @@ cdef int MP_init_current_pool(
279282
"""
280283
IF CUDA_CORE_BUILD_MAJOR >= 13:
281284
cdef cydriver.CUmemoryPool pool
282-
cdef cydriver.CUmemLocation loc = cydriver.CUmemLocation(
283-
type=loc_type, id=loc_id)
285+
cdef cydriver.CUmemLocation loc = cumemlocation_from_type(loc_type, loc_id)
284286
with nogil:
285287
HANDLE_RETURN(cydriver.cuMemGetMemPool(&pool, &loc, alloc_type))
286288
self._h_pool = create_mempool_handle_ref(pool)

‎cuda_core/cuda/core/_memory/_peer_access_utils.pyx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ from typing import TYPE_CHECKING, Any
1010

1111
from cuda.bindings cimport cydriver
1212
from cuda.core._memory._device_memory_resource cimport DeviceMemoryResource
13-
from cuda.core._memory._location cimport to_cumemlocation
13+
from cuda.core._memory._location cimport cumemlocation_from_type
1414
from cuda.core._resource_handles cimport as_cu
1515
from cuda.core._utils.cuda_utils cimport HANDLE_RETURN
1616
from cpython.mem cimport PyMem_Malloc, PyMem_Free
@@ -114,7 +114,8 @@ cdef inline tuple _query_peer_access_ids(DeviceMemoryResource mr):
114114
cdef inline bint _peer_access_includes(DeviceMemoryResource mr, int dev_id):
115115
"""Return True if peer access from ``dev_id`` is currently granted."""
116116
cdef cydriver.CUmemAccess_flags flags
117-
cdef cydriver.CUmemLocation location = to_cumemlocation("device", dev_id)
117+
cdef cydriver.CUmemLocation location = cumemlocation_from_type(
118+
cydriver.CUmemLocationType.CU_MEM_LOCATION_TYPE_DEVICE, dev_id)
118119
with nogil:
119120
HANDLE_RETURN(cydriver.cuMemPoolGetAccess(&flags, as_cu(mr._h_pool), &location))
120121
return flags == cydriver.CUmemAccess_flags.CU_MEM_ACCESS_FLAGS_PROT_READWRITE

‎cuda_core/cuda/core/graph/_graph_node.pyx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ from cuda.core._event cimport Event
2121
from cuda.core._kernel_arg_handler cimport ParamHolder
2222
from cuda.core._launch_config cimport LaunchConfig
2323
from cuda.core._memory._buffer cimport Buffer
24-
from cuda.core._memory._location cimport to_cumemlocation
24+
from cuda.core._memory._location cimport cumemlocation_from_type
2525
from cuda.core._module cimport Kernel
2626
from cuda.core.graph._graph_definition cimport GraphCondition, GraphDefinition
2727
from cuda.core.graph._subclasses cimport (
@@ -836,7 +836,8 @@ cdef inline AllocNode GN_alloc(GraphNode self, size_t size, object device,
836836
peer_id = getattr(peer_dev, 'device_id', peer_dev)
837837
peer_ids.append(peer_id)
838838
access_descs.push_back(cydriver.CUmemAccessDesc_st(
839-
to_cumemlocation("device", peer_id),
839+
cumemlocation_from_type(
840+
cydriver.CUmemLocationType.CU_MEM_LOCATION_TYPE_DEVICE, peer_id),
840841
cydriver.CUmemAccess_flags.CU_MEM_ACCESS_FLAGS_PROT_READWRITE,
841842
))
842843

‎cuda_core/tests/test_build_hooks.py‎

Lines changed: 0 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ def _check_version_detection(
100100

101101
build_hooks._get_cuda_path.cache_clear()
102102
build_hooks._determine_cuda_major_version.cache_clear()
103-
build_hooks._cuda_core_has_localized_location.cache_clear()
104103
get_cuda_path_or_home.cache_clear()
105104

106105
mock_env = {
@@ -126,7 +125,6 @@ def test_env_var_override(self, version):
126125
"""CUDA_CORE_BUILD_MAJOR env var override works with various versions."""
127126
build_hooks._get_cuda_path.cache_clear()
128127
build_hooks._determine_cuda_major_version.cache_clear()
129-
build_hooks._cuda_core_has_localized_location.cache_clear()
130128
get_cuda_path_or_home.cache_clear()
131129
with mock.patch.dict(os.environ, {"CUDA_CORE_BUILD_MAJOR": version}, clear=False):
132130
result = build_hooks._determine_cuda_major_version()
@@ -161,61 +159,9 @@ def test_missing_cuda_path_raises_error(self):
161159
"""RuntimeError is raised when CUDA_PATH/CUDA_HOME not set and no env var override."""
162160
build_hooks._get_cuda_path.cache_clear()
163161
build_hooks._determine_cuda_major_version.cache_clear()
164-
build_hooks._cuda_core_has_localized_location.cache_clear()
165162
get_cuda_path_or_home.cache_clear()
166163
with (
167164
mock.patch.dict(os.environ, {}, clear=True),
168165
pytest.raises(RuntimeError, match="CUDA_PATH or CUDA_HOME"),
169166
):
170167
build_hooks._determine_cuda_major_version()
171-
172-
173-
def _check_localized_location_detection(cuda_version, expected, *, env_override=None):
174-
"""Test localized-arm detection with a mock cuda.h."""
175-
with tempfile.TemporaryDirectory() as tmpdir:
176-
include_dir = Path(tmpdir) / "include"
177-
include_dir.mkdir()
178-
(include_dir / "cuda.h").write_text(f"#define CUDA_VERSION {cuda_version}\n")
179-
180-
build_hooks._get_cuda_path.cache_clear()
181-
build_hooks._cuda_core_has_localized_location.cache_clear()
182-
get_cuda_path_or_home.cache_clear()
183-
184-
mock_env = {"CUDA_PATH": tmpdir}
185-
if env_override is not None:
186-
mock_env["CUDA_CORE_HAS_LOCALIZED_LOCATION"] = env_override
187-
188-
with mock.patch.dict(os.environ, mock_env, clear=True):
189-
assert build_hooks._cuda_core_has_localized_location() is expected
190-
191-
192-
class TestHasLocalizedLocation:
193-
"""Tests for _cuda_core_has_localized_location()."""
194-
195-
@pytest.mark.agent_authored(model="grok-4.6")
196-
@pytest.mark.parametrize(
197-
("cuda_version", "expected"),
198-
[
199-
(12080, False),
200-
(13000, False),
201-
(13030, False),
202-
(13040, True),
203-
(14000, True),
204-
],
205-
ids=["12.8", "13.0", "13.3", "13.4", "14.0"],
206-
)
207-
def test_cuda_headers_parsing(self, cuda_version, expected):
208-
"""CUDA_VERSION 13040+ enables the localized CUmemLocation arm."""
209-
_check_localized_location_detection(cuda_version, expected)
210-
211-
@pytest.mark.agent_authored(model="grok-4.6")
212-
@pytest.mark.parametrize(
213-
("override", "expected"),
214-
[
215-
("0", False),
216-
("1", True),
217-
],
218-
)
219-
def test_env_var_override(self, override, expected):
220-
"""CUDA_CORE_HAS_LOCALIZED_LOCATION overrides the header-derived value."""
221-
_check_localized_location_detection(13030, expected, env_override=override)

0 commit comments

Comments
 (0)