Avoid aggregate CUmemLocation initialization - #2673
Open
rwgk wants to merge 1 commit into
Open
Conversation
rwgk
added
P0
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Initialize
CUmemLocationstructures by declaring them and assigning the activetypeandidfields instead of using Cython aggregate constructors. Apply the same pattern when constructing the nestedCUmemLocationinsideCUmemAccessDesc.This preserves the existing behavior while keeping the source compatible with both the older two-member generated declaration and the newer CUDA 13.4 declaration.
Rationale
This issue surfaced while building #2641. Public main's
cuda.coresource used aggregate initializers such as:The older generated declaration exposed only
typeandid, making that initializer complete. CUDA 13.4 adds thelocalizedarm to the anonymous location union, so the generated Cython declaration exposestype,id, andlocalized. With that declaration, Cython reports:All affected paths currently construct location kinds whose active payload is
id:DEVICE,HOST,HOST_NUMA, orHOST_NUMA_CURRENT. They do not constructDEVICE_LOCALITY_DOMAIN, which uses thelocalizedarm. Declaring the structure and assigning only its active fields therefore expresses the intended union use directly and compiles against both generated layouts.This is a source-compatibility change only; it does not alter the selected location types, IDs, access flags, or runtime control flow.
Note for completeness
The aggregate initialization style entered through #2434 and was valid for the generated binding declaration available at that time. Public PR #2593 later centralized one of the location conversions in
_memory/_location.pxd, so this patch updates that shared helper as well as the original call sites. This is a narrow compatibility follow-up that preserves the broader refactoring in those PRs.