feat(cuda.core): support CU_LAUNCH_ATTRIBUTE_PRIORITY in LaunchConfig - #2706
feat(cuda.core): support CU_LAUNCH_ATTRIBUTE_PRIORITY in LaunchConfig#2706ichensun wants to merge 2 commits into
Conversation
|
/ok to test b963404 |
|
| Whether to allow programmatic stream serialization (PDL). When True, | ||
| the kernel may overlap with a previous kernel in the same stream that | ||
| signals completion via programmatic means. | ||
| priority : int, optional |
There was a problem hiding this comment.
Do we have a range for the value we can set? It cannot be any number.
CUDA doc should provide guidance on what value to set. Let's include the guidance in docstring.
There was a problem hiding this comment.
The range depends on the device, so there's no single fixed number to document. It's queried at runtime via cuCtxGetStreamPriorityRange(), which reports the valid bounds for the currently active context. On a device that doesn't support multiple stream priorities, both bounds come back as 0.
| public int shmem_size | ||
| public bint is_cooperative | ||
| public bint programmatic_stream_serialization | ||
| public object priority |
There was a problem hiding this comment.
Let's use int instead of obejct
c71a56d to
070353d
Compare
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
There is still a semantic gap in the current head around priority=0.
StreamOptions uses None as the sentinel and treats 0 as a real priority. This PR now stores LaunchConfig.priority as an int, but both construction and native conversion use truthiness (if priority: / if self.priority:). That makes an explicit priority=0 indistinguishable from not specifying the launch attribute at all.
That matters when the launch runs on a nonzero-priority stream. For example, if the stream priority is -1 and the caller passes LaunchConfig(priority=0), the expected behavior is to emit CU_LAUNCH_ATTRIBUTE_PRIORITY=0 and override the stream priority. The current code emits no priority attribute, so the launch inherits -1 instead. This also conflicts with the PR description's original distinction that 0 is meaningful rather than an unset sentinel.
Could we keep the public/int storage requested in the earlier review but track whether the option was explicitly supplied separately (for example with a _priority_is_set flag), and add a regression that distinguishes None from 0?
Description
issues #2631
Adds a
priorityattribute toLaunchConfigthat maps toCU_LAUNCH_ATTRIBUTE_PRIORITY, following the same pattern used forprogrammatic_stream_serialization(#1334).priority: int | None = None(defaultNone) — when omitted, the launch uses the stream's priority, matching existingLaunchConfigattribute conventions.__init__against the device's stream priority range: a nonzeropriorityoutside[greatestPriority, leastPriority], as returned bycuCtxGetStreamPriorityRange, raisesValueError.intwith0meaning "unset", soconfig.priorityreads back0(notNone) when omitted, and both native-config conversion paths (LaunchConfig._to_native_launch_configand the module-level_to_native_launch_config) use a truthiness check, emitting no launch attribute for0. This makes an explicitpriority=0indistinguishable from an unset priority, which is behaviorally equivalent in practice: on devices without multiple stream priorities both bounds are0, and elsewhere0isleastPriority, so omitting the attribute and inheriting the stream's priority yields the same scheduling.0/nonzero priorities, and an updatedLaunchConfigrepr pattern intest_object_protocols.py.