Skip to content

remove memory type check #4767

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mayuyuace
Copy link

When I used virtual memory pointer from zeVirtualMemReserve, I found that zeMemGetAllocProperties cannot know if it is from device.
So remove memory type check to avoid runtime error.

@chengjunlu
Copy link
Contributor

chengjunlu commented Jul 23, 2025

In what kind of use case the address was passed to Triton with zeVirtualMemReserve but unknown it is to the device?

The check is to protect the kernel from accessing in-accessible memory address. We need use other L0 API to make sure it is safe to access the memory address if the zeMemGetAllocProperties doesn't work for some use cases rather than just remove the ckeck.

@mayuyuace
Copy link
Author

@chengjunlu
I want to pass virtual memory address to triton kernel.
But failed at the memory device type check.
By removing the check, triton kernel can run successfully.
If this check cannot be removed, can you help add another check for virtual memory pointer?
Thanks!
image

@chengjunlu
Copy link
Contributor

chengjunlu commented Jul 23, 2025

@mayuyuace
Can you explain how did you get the virtual memory address?
In the typical use case of Triton, the memory management is under torch.xpu. The torch.xpu uses the SYCL malloc to get the device memory and the Triton can use the L0 to check the memory.

We want to protect the Triton kernel from using wild pointer of the un-awareness which may cause bug.
There is a test case test_pointer_arguments[cpu] for this protection. Just removing the check would cause the test case failure.

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

Successfully merging this pull request may close these issues.

2 participants