Skip to content

Avoid aggregate graph access descriptor initialization - #2688

Merged
rwgk merged 1 commit into
NVIDIA:mainfrom
rwgk:follow_up_to_pr2683
Aug 24, 2026
Merged

Avoid aggregate graph access descriptor initialization#2688
rwgk merged 1 commit into
NVIDIA:mainfrom
rwgk:follow_up_to_pr2683

Conversation

@rwgk

@rwgk rwgk commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to #2683.

PR #2683 centralizes CUmemLocation construction, but the graph allocation path still embeds the returned location in a CUmemAccessDesc aggregate constructor. With the CUDA 13.4 generated declaration, that nested aggregate causes Cython to emit an invalid conversion for the added localized union arm.

Declare the access descriptor separately and assign its location and flags fields before adding it to the descriptor vector. This keeps the helper introduced by #2683 and preserves the existing runtime behavior.

Please see ctk-next PR 539 for background on how this escaped attention while validating #2683.

Validation

  • pre-commit run --all-files
  • The equivalent change is validated against the CUDA 13.4 generated bindings in ctk-next PR 539.

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@rwgk rwgk added this to the cuda.bindings 13.4.0 & 12.9.8 milestone Aug 23, 2026
@rwgk rwgk added P0 High priority - Must do! cuda.core Everything related to the cuda.core module labels Aug 23, 2026
@rwgk rwgk self-assigned this Aug 23, 2026
@copy-pr-bot

copy-pr-bot Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@rwgk

rwgk commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@github-actions

This comment has been minimized.

@rwgk
rwgk marked this pull request as ready for review August 23, 2026 22:19
@rwgk
rwgk requested a review from juenglin August 23, 2026 22:19
@rwgk

rwgk commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Additional rationale for merging this PR

The need for this change appears to arise from a Cython code-generation issue exposed by cybind's representation of CUDA's anonymous nested struct:

  • The aggregate initializer causes Cython to emit an unused struct-to-Python conversion helper.
  • That helper attempts to convert the actual anonymous C++ type of localized to cybind's synthetic named struct type, which the generated C++ compiler correctly rejects as incompatible.
  • Field-by-field initialization compiles because it avoids generating that conversion helper.
  • C struct aggregate initialization is supported Cython syntax, so the original call site was not misusing Cython. See the Cython language basics documentation.

The field-by-field initialization in this PR is therefore a narrowly scoped workaround that preserves the intended descriptor contents and behavior.

@rwgk
rwgk merged commit 54ce994 into NVIDIA:main Aug 24, 2026
118 checks passed
@rwgk
rwgk deleted the follow_up_to_pr2683 branch August 25, 2026 00:00
@github-actions

This comment has been minimized.

1 similar comment
@github-actions

Copy link
Copy Markdown
Doc Preview CI
Preview removed because the pull request was closed or merged.

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.

2 participants