Uh oh!
There was an error while loading. Please reload this page.
[NVVM] Support - Followup enhancements - #1218
Conversation
leofang
commented
Nov 17, 2025
Thanks, @abhilash1910! Any ETA to wrap this up? |
abhilash1910
commented
Nov 25, 2025
pre-commit.ci autofix |
abhilash1910
commented
Nov 25, 2025
pre-commit.ci autofix |
leofang
left a comment
There was a problem hiding this comment.
Thanks, Abhilash! Leaving a few early feedbacks.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
abhilash1910
commented
Dec 3, 2025
pre-commit.ci autofix |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| libdevice_len = len(libdevice_bytes) | ||
| # Use lazy_add_module | ||
| with nogil: | ||
| HANDLE_RETURN_NVVM(prog, cynvvm.nvvmLazyAddModuleToProgram( |
There was a problem hiding this comment.
Compiling the same Program instance multiple times re-adds libdevice here multiple times. Is that a concern? Could it make sense to add call-once logic?
There was a problem hiding this comment.
The below is Cursor-generated, based on my suspicion. I'll continue to work on Option 1 with Cursor and will post an update here (hopefully very quick).
I think there is still a potential race in the current call-once logic.
Current pattern:
ifself._use_libdeviceandnotself._libdevice_added:
...
withnogil:
nvvmLazyAddModuleToProgram(...)
self._libdevice_added=TrueWhy this can race:
with nogilreleases the GIL.- Another thread can enter
Program.compile()on the samePrograminstance while the first thread is inside thenogilblock. - If that second thread checks
_libdevice_addedbefore the first thread storesTrue, both threads can pass the guard and both callnvvmLazyAddModuleToProgram(...).
So:
- Single-threaded usage is fine.
- Concurrent compile on the same
Programcan still double-add libdevice.
Potential fixes:
Per-instance lock (strongest)
Guard the add/compile section with a lock to serialize concurrent compile calls perProgram.Set-before-call with rollback (minimal change)
Set_libdevice_added = Truebefore enteringwith nogil, and if add fails, reset it toFalseinexcept.
This removes the check/set gap that currently exists around the GIL release.
There was a problem hiding this comment.
Sorry, the order of options got shuffled, I meant Option 1. Corrected above.
rwgk
commented
Feb 18, 2026
/ok to test f7b9c4f |
rwgk
commented
Feb 18, 2026
@abhilash1910 I powered through all the pathfinder-related things I could think of. I think that part is in merge-ready shape. I left a few comments on the cuda_core program changes; filtered things Cursor found. |
abhilash1910
commented
Feb 18, 2026
pre-commit.ci autofix |
abhilash1910
commented
Feb 18, 2026
pre-commit.ci autofix |
abhilash1910
commented
Feb 18, 2026
/ok to test 51f6009 |
abhilash1910
commented
Feb 18, 2026
Thanks @rwgk for the additions, addressed the suggestions. |
Add a per-instance threading lock around the NVVM verify/libdevice-add/compile path to prevent concurrent compile races from double-adding libdevice. The lock is taken under the GIL and held across nogil sections, and we verified this ordering avoids introducing a lock/GIL deadlock cycle. Co-authored-by: Cursor <cursoragent@cursor.com>
rwgk
commented
Feb 18, 2026
I pushed commit 701f54b to take care of the race. I'm testing locally. As soon as I see that passing (5-10 minutes) I'll trigger the CI here. |
rwgk
commented
Feb 18, 2026
/ok to test 701f54b |
rwgk
left a comment
There was a problem hiding this comment.
Assuming the CI passes, this looks merge-ready to me.
rwgk
commented
Feb 18, 2026
The one build failure is this: We need to rerun after the first attempt is done. |
Uh oh!
There was an error while loading. Please reload this page.
Description
Issue Link - #981
Changes to be addressed in this WIP PR:
{If / when it is possible to add multiple modules, a test with code that uses something from libdevice is probably a good idea.
It's also useful to be able to lazily add a module}
cc @leofang