Conversation
cpcloud
added
enhancement
|
cpcloud
force-pushed
the
linker-backend-classmethod-714
branch
2 times, most recently
from
dbe6176 to
215418a
Compare
leofang
left a comment
leofang
left a comment
There was a problem hiding this comment.
Thanks, Phillip! Implementation-wise LGTM. Left a few suggestions.
There was a problem hiding this comment.
I believe we are shooting for 1.0.0, unless @mdboom concludes that a 0.8.0 is a must for RAPIDS adoption.
There was a problem hiding this comment.
Renamed to 1.0.0-notes.rst.
There was a problem hiding this comment.
nit: let's move the tests to the existing test_linker.py. There is no need for separate test files.
There was a problem hiding this comment.
Moved into test_linker.py and deleted test_linker_backend.py.
| These live in a separate file from test_linker.py because that module calls | ||
| Device() at import time, which requires a GPU. These tests use monkeypatch | ||
| to set module-level flags and never touch CUDA devices. |
There was a problem hiding this comment.
Does this make sense? Would the driver cuLink* APIs be functional when there is no GPU present? I don't think this is ever tested.
NVRTC/nvJitLink work without GPU or CUDA, but it is not something we want to guarantee to users without the compiler team onboarding. This is not documented AFAIK.
There was a problem hiding this comment.
Good point — test_linker.py already calls Device() at module import (line 11), so the GPU-free framing was moot. Dropped the rationale and merged the tests in; they just sit alongside the existing ones now.
| - :meth:`Linker.backend` is now a classmethod instead of an instance property. | ||
| Call sites must use ``Linker.backend()`` (with parentheses) instead of | ||
| ``linker.backend``. This allows querying the linking backend without | ||
| constructing a ``Linker`` instance — for example, to choose between PTX and | ||
| LTOIR input before linking. |
Allows querying the linking backend without constructing a Linker instance — useful for dispatching on input format (PTX vs. LTOIR) before linking. Updates existing call sites (Program init, test_linker) to use the new invocation form Linker.backend().
Covers classmethod invocation (Linker.backend() without an instance), memoisation flag handling, probe-on-first-use, and non-property attribute semantics.
Breaking change: Linker.backend is now a classmethod, so call sites must use parens: Linker.backend().
cpcloud
force-pushed
the
linker-backend-classmethod-714
branch
from
215418a to
12d291b
Compare
2 participants
Summary
Linker.backendfrom an instance property to a classmethod so callers can query the linking backend without constructing aLinker_decide_nvjitlink_or_driver()(existing memoised probe) and maps the return value to"nvJitLink"or"driver"_program.pyxand existing test assertion to use parensDevice()/cuInitrequired)0.8.0-notes.rstdocumenting the breaking changeBreaking change:
linker.backend(attribute access) now returns a bound method, not a string. All call sites must useLinker.backend(). Only 2 in-repo call sites affected; numba-cuda (the requester) will adoptLinker.backend()from day one.Test plan
test_linker_initpasses with updated paren callCloses #714
🤖 Generated with Claude Code