Skip to content

[NVVM] Expose nvvm version detection in cuda.bindings.utils.#1837

Open
abhilash1910 wants to merge 7 commits intoNVIDIA:mainfrom
abhilash1910:nvvm_fix
Open

[NVVM] Expose nvvm version detection in cuda.bindings.utils.#1837
abhilash1910 wants to merge 7 commits intoNVIDIA:mainfrom
abhilash1910:nvvm_fix

Conversation

@abhilash1910
Copy link
Copy Markdown
Contributor

Description

Fixes issue #1457 . Provides an api call to cuda.bindings.utils to check the version of nvvm.
@leofang @rwgk pinging for review.

@copy-pr-bot
Copy link
Copy Markdown
Contributor

copy-pr-bot bot commented Mar 31, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@abhilash1910
Copy link
Copy Markdown
Contributor Author

pre-commit.ci autofix

@abhilash1910 abhilash1910 changed the title [NVVM]Expose nvvm version detection in cuda.bindings.utils. [NVVM][Fix] Expose nvvm version detection in cuda.bindings.utils. Mar 31, 2026
@abhilash1910 abhilash1910 changed the title [NVVM][Fix] Expose nvvm version detection in cuda.bindings.utils. [NVVM] Expose nvvm version detection in cuda.bindings.utils. Mar 31, 2026
@rwgk rwgk added the cuda.bindings Everything related to the cuda.bindings module label Mar 31, 2026
@rwgk rwgk added this to the cuda.bindings backlog milestone Mar 31, 2026
@rwgk rwgk added enhancement Any code-related improvements P1 Medium priority - Should do labels Mar 31, 2026
options_list = [opt.decode("utf-8") if isinstance(opt, bytes) else opt for opt in options]
nvvm.verify_program(program, len(options_list), options_list)
nvvm.compile_program(program, len(options_list), options_list)
except Exception:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're compiling anyway, do you actually need the verify_program() call?

I'm thinking the try should just be around this one line:

        try:
            nvvm.compile_program(prog, len(options), options)
        except nvvm.nvvmError as e:
            # can we add something here to ensure we're not masking errors other than invalid options?

I believe it's really important to take great care that we're not masking actual errors; e.g. the hard-wired _PRECHECK_NVVM_IR might need tweaks for future GPU generations. If we're simply reporting any error as "invalid compiler option", it'll potentially take someone downstream a long time to drill down all the way back here.

Copy link
Copy Markdown
Contributor Author

@abhilash1910 abhilash1910 Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I do agree verify_program() is not needed, the location of proposed try could eventually throw as then the destroy_program() call will never get triggered.

What I was thinking is that :

program = nvvm.create_program()
try:
  <..rest of code>
  nvvm.compile_program()
except nvvm.nvvmError as e:
 <exception raising>
finally:
  destroy_program()

Comment on lines +53 to +54
>>> check_nvvm_options([b"-arch=compute_90", b"-numba-debug"])
True # if -numba-debug is supported by the installed libNVVM
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use a non-numba example here?

@rparolin rparolin removed this from the cuda.bindings backlog milestone Apr 2, 2026
@abhilash1910
Copy link
Copy Markdown
Contributor Author

pre-commit.ci autofix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.bindings Everything related to the cuda.bindings module enhancement Any code-related improvements P1 Medium priority - Should do

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants