Fix #789: Remove cycle between c.b.cyruntime and c.b._lib.cyruntime.cyruntime

[mdboom]

This now textually includes cuda.bindings._lib.cyruntime.cyruntime in cuda.bindings._bindings.cyruntime to remove the cycle.

Since this is only /adding/ symbols to cuda.bindings._bindings.cyruntime, I believe it isn't breaking ABI compatibility (but I admittedly am only now getting used to "Cython ABI compatibility"). The old cuda.bindings._lib.cyruntime.cyruntime and cuda.bindings._lib.cyruntime.utils are just removed, but since those were private, I think we ok for ABI there, too.

I also discovered a bug in the test for cycles that was finding erroneous cycles when ImportErrors were raised and caught.

Description

closes

Checklist

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

[copy-pr-bot]

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.

[mdboom]

/ok to test

[leofang]

LGTM overall! Left a few questions. Is this still WIP (since it's in the draft state)?

[copy-pr-bot]

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

Contributors can view more details about this message here.

[mdboom]

LGTM overall! Left a few questions. Is this still WIP (since it's in the draft state)?

No, it's ready for review. I didn't notice I created a draft...

[mdboom]

/ok to test

[kkraus14]

The old cuda.bindings._lib.cyruntime.cyruntime and cuda.bindings._lib.cyruntime.utils are just removed, but since those were private, I think we ok for ABI there, too.

I poked around the generated .cpp code from the Cython to get a better sense as to what's happening when using cimport and the potential ABI issues.

It looks like when you cimport something it adds a declaration, i.e. for cuda.bindings._lib.cyruntime.utils.getDriverEglFrame the generated .cpp code from something that cimports it contains the following:

• A declaration statement that looks like:

staticcudaError_t (*__pyx_f_4cuda_8bindings_4_lib_9cyruntime_5utils_getDriverEglFrame)(__pyx_t_4cuda_8bindings_8cydriver_CUeglFrame *, __pyx_t_4cuda_8bindings_9cyruntime_cudaEglFrame); /*proto*/

• Within a static int __Pyx_modinit_function_import_code function, a call that looks like:

if (__Pyx_ImportFunction_3_0_12(__pyx_t_1, "getDriverEglFrame", (void (**)(void))&__pyx_f_4cuda_8bindings_4_lib_9cyruntime_5utils_getDriverEglFrame, "cudaError_t (__pyx_t_4cuda_8bindings_8cydriver_CUeglFrame *, __pyx_t_4cuda_8bindings_9cyruntime_cudaEglFrame)") < 0) __PYX_ERR(0, 1, __pyx_L1_error)

So there's zero linkage happening and everything is declared as static. The symbol resolution is happening via the __Pyx_ImportFunction_3_0_12 function and unless the cimport is redeclared in a .pxd file, it doesn't yield a corresponding __Pyx_ExportFunction call for it.

I think this effectively means we can treat Cython the same way we treat Python where we only need to worry about API compatibility, except for externed vs non-externed declarations in .pxd files in public modules where we have shown in the past that it causes ABI breakage.

[mdboom]

/ok to test

[mdboom]

/ok to test

[mdboom]

/ok to test

[mdboom]

/ok to test

[leofang]

Thanks a lot @mdboom!

[github-actions]

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