Skip to content

CUDA fixes for Windows - #8004

Merged
sklam merged 4 commits into
numba:mainfrom
gmarkall:cuda-windows-fixes-056
Apr 27, 2022
Merged

sklam merged 4 commits into
numba:mainfrom
gmarkall:cuda-windows-fixes-056

Conversation

@gmarkall

@gmarkall gmarkall commented Apr 25, 2022 •

Copy link
Copy Markdown
Member

This fixes a couple of issues on Windows:

  • IPC is not actually supported on Windows, there was just a small range of drivers where it appeared to work in some circumstances. Because it is not properly supported, we limit its support back to just Linux again - this includes skipping the tests, erroring if an attempt to use IPC on non-Linux is used, and updating the documentation.
  • Sometimes on Windows if nvdisasm is not found, the FileNotFoundError's filename is blank. There doesn't appear to be any value in checking for nvdisasm in the name, and when it's not found it resulted in incorrect logic, so I just removed the check for the filename.

One issue is still outstanding on Windows with recent drivers (tests of printing functionality, #8005), which will be fixed in a separate PR.

The filename may not be present on Windows, and it's not clear that
checking the filename adds anything, given that there is only a single
line of code in the try block with a fixed filename.

When the filename is empty, no exception is raised by
disassemble_cubin(), and it then attempts to access cp, which has not
been set. To avoid this issue, we now unconditionally raise the
RuntimeError that indicates that nvdisasm is not present.
IPC is unsupported on Windows - the fact that it worked for some driver
releases in some cases was an accident.
This mostly reverts commit 7ffa4cf.
@gmarkall gmarkall added 2 - In Progress CUDA CUDA related issue/PR Effort - short Short size effort needed labels Apr 25, 2022
@gmarkall

Copy link
Copy Markdown
Member Author

gpuci run tests

@stuartarchibald stuartarchibald left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the patch.

@stuartarchibald stuartarchibald added 4 - Waiting on CI Review etc done, waiting for CI to finish Pending BuildFarm For PRs that have been reviewed but pending a push through our buildfarm and removed 3 - Ready for Review labels Apr 27, 2022
@stuartarchibald stuartarchibald added this to the Numba 0.56 RC milestone Apr 27, 2022
@stuartarchibald

Copy link
Copy Markdown
Contributor

Buildfarm ID: numba_smoketest_cuda_yaml_123.

@stuartarchibald stuartarchibald added BuildFarm Passed For PRs that have been through the buildfarm and passed 5 - Ready to merge Review and testing done, is ready to merge and removed Pending BuildFarm For PRs that have been reviewed but pending a push through our buildfarm 4 - Waiting on CI Review etc done, waiting for CI to finish labels Apr 27, 2022
@stuartarchibald

Copy link
Copy Markdown
Contributor

Buildfarm ID: numba_smoketest_cuda_yaml_123.

Passed.

@sklam
sklam merged commit e5bbae7 into numba:main Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5 - Ready to merge Review and testing done, is ready to merge BuildFarm Passed For PRs that have been through the buildfarm and passed CUDA CUDA related issue/PR Effort - short Short size effort needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants