Skip to content

Conversation

@rwgk
Copy link
Collaborator

@rwgk rwgk commented Feb 8, 2025

Resolve compatibility problems discovered while backporting PR #421 to the 11.8.x branch.

The NVVM IR major & debug_major version numbers in MINIMAL_NVVMIR_TXT in test_nvvm.py need to match the version numbers in the libnvvm.so being used.

For CTK 11.8 the libnvvm.so IR major & debug_major versions numbers are 1 and 3, respectively.

For CTK 12.x the libnvvm.so IR major & debug_major versions numbers are 2 and 3, respectively.

This is easily accommodated for MINIMAL_NVVMIR_TXT, but generating MINIMAL_NVVMIR_BITCODE requires a tool for converting NVVM IR text to bitcode.

If llvmlite is available the conversion is actually very simple:

            bitcode_dynamic = llvmlite_binding.parse_assembly(txt.decode()).as_bitcode()

However, we decided it is best to not make llvmlite a hard dependency for testing. To this end, a dictionary with bitcode inputs in binascii format is introduced, keyed by NVVM IR major & debug_major version numbers, e.g.:

MINIMAL_NVVMIR_BITCODE_STATIC = {
    (1, 3):  # (major, debug_major)
    "4243c0de3514000005000000620c30244a59be669dfbb4bf0b51804c01000000210c00007f010000"
    ...
    "00000000",
    (2, 3):  # (major, debug_major)
    "4243c0de3514000005000000620c30244a59be669dfbb4bf0b51804c01000000210c000080010000"
    ....
    "6e673e0000000000",
}

Now we have a total of four combinations of situations:

  • llvmlite available yes/no

  • MINIMAL_NVVMIR_BITCODE_STATIC entry available yes/no

  1. If both are not available (e.g. for a new CTK version) test_nvvm.py will fail:
                raise RuntimeError("Please `pip install llvmlite` to generate `bitcode_static` (see PR #443)")
  1. If only llvmlite is available, running test_nvvm.py will generate the missing entry for MINIMAL_NVVMIR_BITCODE_STATIC, correctly formatted for pasting into test_nvvm.py, and bitcode_static testing will simply be skipped.
                pytest.skip(f"UNAVAILABLE: {request.param}")
  1. If only the MINIMAL_NVVMIR_BITCODE_STATIC entry is available, bitcode_dynamic testing will simply be skipped. This is assumed to be the routine situation.

  2. If both llvmlite and the MINIMAL_NVVMIR_BITCODE_STATIC entry are available, testing will run with both independently.

With this setup it is expected to be very easy to keep up with new CTK versions. llvmlite is needed only when testing with a new CTK version for the first time, to generate the new MINIMAL_NVVMIR_BITCODE_STATIC entry.

@copy-pr-bot
Copy link
Contributor

copy-pr-bot bot commented Feb 8, 2025

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 mentioned this pull request Feb 8, 2025
@rwgk
Copy link
Collaborator Author

rwgk commented Feb 8, 2025

/ok to test

@github-actions

This comment has been minimized.

@rwgk
Copy link
Collaborator Author

rwgk commented Feb 11, 2025

/ok to test

@rwgk rwgk changed the title Forward port of test_nvvm.py changes under PR #442 test_nvvm.py changes for compatibility with CTK 11.8 Feb 11, 2025
rwgk added a commit to leofang/cuda-python that referenced this pull request Feb 11, 2025
@rwgk rwgk requested a review from leofang February 11, 2025 18:21
@rwgk
Copy link
Collaborator Author

rwgk commented Feb 11, 2025

Hi @leofang, this is ready for merging. I didn't re-run the tests after the last commit c8d0842 because that only changes comments. Testing before was successful for both PRs (this one and #442).

@leofang leofang marked this pull request as ready for review February 11, 2025 21:05
@copy-pr-bot
Copy link
Contributor

copy-pr-bot bot commented Feb 11, 2025

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.

@leofang
Copy link
Member

leofang commented Feb 11, 2025

/ok to test

@leofang leofang added P1 Medium priority - Should do test Improvements or additions to tests cuda.bindings Everything related to the cuda.bindings module labels Feb 11, 2025
@leofang leofang added this to the cuda-python 12-next, 11-next milestone Feb 11, 2025
@leofang leofang enabled auto-merge February 11, 2025 21:08
@leofang leofang merged commit acff0c5 into NVIDIA:main Feb 11, 2025
72 checks passed
@rwgk rwgk deleted the fwd-port-11.8.x-compatibility branch February 11, 2025 21:47
@github-actions
Copy link

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.bindings Everything related to the cuda.bindings module P1 Medium priority - Should do test Improvements or additions to tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants