[SYCL][ROCm] Fix freeing USM managed pointer with NVIDIA #4123
Conversation
|
@smaslov-intel, ping. |
| type = hipPointerAttributeType.memoryType; | ||
| #ifdef __HIP_PLATFORM_NVIDIA__ | ||
| // The NVIDIA hipPointerGetAttributes implementation doesn't know about |
There was a problem hiding this comment.
What about other calls to hipPointerGetAttributes which will continue to return error?
There was a problem hiding this comment.
That's a good point, I only ran into this call failing, but yeah it will happen with the others too, I'll update them
There was a problem hiding this comment.
please consider doing this adjustment in the function itself, if feasible
There was a problem hiding this comment.
Just a quick update on this patch, I'm currently looking at this and some of the other HIP workarounds in the ROCm plugin and figuring out if I can fix them in the HIP headers directly and submit the fixes to HIP upstream.
However I do think we'll probably still need the workarounds in the ROCm plugin, at least until the next HIP release and if my fixes get approved. So I'll come back to this patch in a bit and make it a proper workaround for all the other uses of hipPointerGetAttributes as well.
There was a problem hiding this comment.
@FMarno did we end up submitting a patch upstream for this?
There was a problem hiding this comment.
Not yet, but it's one of the things we are looking at in the next couple weeks. I'll write a note to notify you when we do something about it.
There was a problem hiding this comment.
@npmiller looks like AMD put a patch in for this last week ROCm/hipamd@88f1622. You should be able to do a proper fix now.
This patch works around a bug in the HIP API for NVidia where
hipPointerGetAttributesdoesn't know about managed pointers.