Keep the interior offset in hipHostGetDevicePointer and range-check registered host memory correctly - #1517
Open
pvelesko wants to merge 4 commits into
Open
Conversation
…or offset hipHostGetDevicePointer() on a pointer inside a hipHostMalloc(Mapped) or hipHostRegister(Mapped) buffer returns the device base of the allocation instead of base + offset. Trilinos STK queries one interior pointer per field of a per-bucket hipHostMalloc and reads the wrong bytes on device. The hipHostRegister half also queries an interior pointer before the base, which currently fails the lookup outright because the range check uses the not yet allocated DevPtr.
…evPtr getAllocInfoCheckPtrRanges() finds the record whose map key is the greatest address <= the queried pointer, but then measured the record's extent from DevPtr regardless of whether the key was the host or the device start. For a hipHostRegister'ed record the host range lives at a different address than the device range, and DevPtr is null until the backing is created, so an interior host pointer never matched and hipHostGetDevicePointer rejected it. Measure the extent from the key that matched.
hipHostGetDevicePointer() resolved an interior host pointer to its allocation by range and then returned the allocation's device base, dropping the offset of the queried pointer. ROCm returns the device address plus the offset of the pointer within its memory object. Trilinos STK does one hipHostMalloc per bucket and asks for the device pointer of each field's interior pointer, so every field but the first read the wrong bytes on device. In the lazy branch that creates the device backing for a hipHostRegister'ed range, register the host base of the range rather than the queried pointer, so that later queries at other offsets (and hipHostUnregister on the base) resolve to the same record.
RegisteredVarCopy() copies a hipHostRegister'ed range to and from its device backing around a kernel launch. For modules without indirect global buffer accesses it only does so when the backing is passed as a kernel argument, and that test compared the argument against the device base exactly, so a device pointer obtained for an interior host pointer skipped the copy and the kernel read stale device memory. Match any argument that falls inside the backing's range.
Collaborator
Author
|
/run-aurora-ci |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
hipHostGetDevicePointer()on a pointer inside ahipHostMalloc(Mapped)orhipHostRegister(Mapped)buffer returned the device base of the allocation instead of base plus offset, and for a registered range an interior pointer queried before the base failed the lookup outright becausegetAllocInfoCheckPtrRanges()measured the record's extent from the still nullDevPtrrather than from the key it matched. This keeps the interior offset in the returned device pointer (as ROCm does), range-checks against the matched key, registers the host base rather than the queried pointer when the device backing is created lazily, and makesRegisteredVarCopy()sync a registered range when a kernel argument points anywhere inside its backing rather than only at its base. Found with Trilinos STK on Aurora PVC, which does onehipHostMallocper bucket and asks for the device pointer of each field's interior pointer.TestFixHostGetDevicePointerOffsetcovers both thehipHostMallocand thehipHostRegisterpaths.Fixes #1505