Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cudawrappers on Windows #253

Open
loostrum opened this issue Feb 14, 2024 · 5 comments
Open

cudawrappers on Windows #253

loostrum opened this issue Feb 14, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@loostrum
Copy link
Member

For the ultrasound part of the RECRUIT project I'd like to see if we can use cudawrappers, but their production system runs windows. I see no reason why cudawrappers shouldn't work on windows, but I haven't tried it. @csbnw have you ever tried it?

I can set up a windows machine at home to test if needed, I should still have an NVIDIA GPU lying around somewhere.

@csbnw
Copy link
Contributor

csbnw commented Feb 14, 2024

We (at ASTRON) have never tried compiling and running cudawrappers on Windows. It is an interesting use-case, but to properly "support" this, we would also need to include Windows in the CI pipeline.

@csbnw csbnw added the enhancement New feature or request label Feb 14, 2024
@loostrum
Copy link
Member Author

Supporting it in the CI seems difficult. For my use case I'd be more than happy to have it as an experimental, no-promises-that-it-works feature, but I can also understand if you don't want such a non-supported feature.

In any case I'll see if I can get it to build on my Windows machine at home.

@loostrum
Copy link
Member Author

loostrum commented Feb 28, 2024

The current code doesn't work as-is on windows, I thought I'd share my findings here:

  • When CUDAWRAPPERS_BUILD_TESTING is enabled a few coverage flags and debug are set, these don't work with msvc.
  • The fancy target_embed_source macro uses ld to convert a source file directly into an object file. The windows linker can't do this, and there doesn't seem to be a windows tool that can. We could default to loading the source file at runtime on windows, leaving out this embedding entirely.
  • There is an issue in the cu::DeviceMemory constructor, see this error:
C:\Users\Leon\source\repos\cudawrappers\tests\test_cufft.cpp(71): error C2668: 'cu::DeviceMemory::DeviceMemory': ambiguous call to overloaded function
C:\Users\Leon\source\repos\cudawrappers\include\cudawrappers/cu.hpp(458): note: could be 'cu::DeviceMemory::DeviceMemory(CUdeviceptr)'
C:\Users\Leon\source\repos\cudawrappers\include\cudawrappers/cu.hpp(438): note: or       'cu::DeviceMemory::DeviceMemory(size_t,CUmemorytype,unsigned int)'
C:\Users\Leon\source\repos\cudawrappers\tests\test_cufft.cpp(71): note: while trying to match the argument list '(const size_t)'

The constructors are marked as explicit, but for some reason it doesn't pick the one with a size_t argument, presumably because CUdeviceptr and size_t decay to the same type. I'm not sure why this happens.

The coverage and source embedding seem easy enough to fix, but I'm not sure what to do about the DeviceMemory constructor.

@csbnw
Copy link
Contributor

csbnw commented Feb 28, 2024

The DeviceMemory constructor has the following signature:

explicit DeviceMemory(size_t size, CUmemorytype type = CU_MEMORYTYPE_DEVICE, unsigned int flags = 0)

How about removing the default value for the second parameter? It will however likely cause some existing code using this API to break.

@loostrum
Copy link
Member Author

That does fix the issue, but is rather unfortunate. I quite like being able to do call cu::DeviceMemory(size) without having to specify the memory type. Calling DeviceMemory with a CUdeviceptr seems like the less used variant? If we could somehow change that one that might be better, although I'm not sure how we'd do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants