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

Special memories only work with PyCUDA backend #210

Closed
isazi opened this issue Aug 30, 2023 · 9 comments
Closed

Special memories only work with PyCUDA backend #210

isazi opened this issue Aug 30, 2023 · 9 comments

Comments

@isazi
Copy link
Collaborator

isazi commented Aug 30, 2023

The functions:

  • copy_shared_memory_args
  • copy_constant_memory_args
  • copy_texture_memory_args
    In core.py check if self.lang == "CUDA", and not work with e.g. CuPy or other backends that implement the functionality.

Moreover, this is only checked if using run_kernel and not tune_kernel. I am waiting for some comments before fixing it, as I may have overlooked something.

@benvanwerkhoven
Copy link
Collaborator

I do know that constant memory works with the CuPy backend. But now that the number of backends is expanding, a table with feature completeness per backend would be nice to have.

In the meantime, I was also inspecting the code and I think you're onto something interesting here. There is some very old code in core.py that I've quoted below. This indeed includes the if's for lang == "CUDA".

The code in compile_and_benchmark is however not running into any issues since it skips these high-level variants and immediately calls the copy_constant_memory_args etc on self.dev instead of self. Basically, I think that the functions on these lines:

def copy_shared_memory_args(self, smem_args):
"""adds shared memory arguments to the most recently compiled module, if using CUDA"""
if self.lang == "CUDA":
self.dev.copy_shared_memory_args(smem_args)
else:
raise RuntimeError("Error cannot copy shared memory arguments when language is not CUDA")
def copy_constant_memory_args(self, cmem_args):
"""adds constant memory arguments to the most recently compiled module, if using CUDA"""
if self.lang == "CUDA":
self.dev.copy_constant_memory_args(cmem_args)
else:
raise RuntimeError("Error cannot copy constant memory arguments when language is not CUDA")
def copy_texture_memory_args(self, texmem_args):
"""adds texture memory arguments to the most recently compiled module, if using CUDA"""
if self.lang == "CUDA":
self.dev.copy_texture_memory_args(texmem_args)
else:
raise RuntimeError("Error cannot copy texture memory arguments when language is not CUDA")

are actually not used at all and could be removed.

@benvanwerkhoven
Copy link
Collaborator

I've just made some extensions to the documentation on backends see pull request #213

@benvanwerkhoven
Copy link
Collaborator

It would appear that run_kernel maybe does use these methods. Will look into this soon.

@isazi
Copy link
Collaborator Author

isazi commented Oct 2, 2023

I will fix this tomorrow. And base my "patch" on the mega pull-request from @fjwillemsen (unless that is merged into master already today or tomorrow).

@fjwillemsen
Copy link
Collaborator

The plan is indeed that #214 will be merged today or tomorrow, directly after Stein's pull request #189.
I'll notify you by posting in this thread once they have been merged.

@isazi
Copy link
Collaborator Author

isazi commented Oct 2, 2023

Perfect, then I will just fix this issue on master after your merge, and avoid the conflicts :)

@fjwillemsen
Copy link
Collaborator

fjwillemsen commented Oct 3, 2023

Both #189 and #214 have been merged into master!

@isazi
Copy link
Collaborator Author

isazi commented Oct 4, 2023

Great, I will do this today. Will reach to you if something is not clear about the new development environment.

@isazi isazi mentioned this issue Oct 4, 2023
isazi added a commit that referenced this issue Oct 4, 2023
Minor set of fixes, addressing mainly Issue #210
@isazi
Copy link
Collaborator Author

isazi commented Oct 4, 2023

This should be fixed for now.

@isazi isazi closed this as completed Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants