Skip to content

mpl: make MPL_gpu_attr_is_dev mean device memory; add MPL_gpu_attr_is_host - #8049

Merged
hzhou merged 6 commits into
pmodels:mainfrom
hzhou-muse:gpu-attr-is-host
Oct 25, 2026
Merged

hzhou merged 6 commits into
pmodels:mainfrom
hzhou-muse:gpu-attr-is-host

Conversation

@hzhou-muse

@hzhou-muse hzhou-muse commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Pull Request Description

We had both MPL_gpu_attr_is_dev and MPL_gpu_attr_is_strict_dev. They are identical on CUDA and HIP, but on ZE is_dev returned true for any ZE allocation (host-registered, shared, and mmap'd device memory included), because host-registered memory is a device object in the ZE driver and cannot be used with XPMEM. That made is_dev mean "not plain host memory" on ZE, and the real device check had to be is_strict_dev. The pair is confusing.

This PR makes MPL_gpu_attr_is_dev mean device memory on every backend and adds MPL_gpu_attr_is_host for "can be handled as ordinary host memory" (ZE: unregistered host only; CUDA/HIP: unregistered host, registered host, managed). A buffer may be neither. Every former non-strict is_dev caller becomes !is_host, and every is_strict_dev caller becomes is_dev, so there is no behavior change on any backend.

  • Add MPIR_GPU_query_pointer_is_host; remove MPL_gpu_attr_is_strict_dev and the unused MPIR_GPU_query_pointer_is_strict_dev.
  • Replace direct attr.type == MPL_GPU_POINTER_DEV comparisons in ccl, GPU IPC local mmap and MPIR_Localcopy_gpu with MPL_gpu_attr_is_dev.
  • Fix test/mpi/rma/testlist.gpu, which still listed getfence1, putfence1 and lock_dt after they became fence -get/-put and lock_x_dt, and fix -flushlocal/-flushlocalall in lock_x_dt setting the wrong flush variable.

[warnings:normal]

Author Checklist

  • Provide Description
  • Commits Follow Good Practice
  • Passes All Tests
  • Contribution Agreement

🤖 Generated with Claude Code

MPL_gpu_attr_is_host returns true when a buffer can be handled as
ordinary host memory. On CUDA and HIP that is unregistered host,
registered host and managed memory; on ZE it is only unregistered host
memory, since host-registered memory is a device object in the driver.
MPIR_GPU_query_pointer_is_host wraps it for a pointer.

Previously callers used the non-strict MPL_gpu_attr_is_dev, whose
meaning differs on ZE, to express this.

model: Opus 5.5
Callers that need to know whether a buffer can be handled as host
memory now check MPL_gpu_attr_is_host or
MPIR_GPU_query_pointer_is_host. This has the same result on every
backend.

Previously they used the non-strict MPL_gpu_attr_is_dev, which on ZE
also returned true for host-registered and shared memory.

model: Opus 5.5
MPL_gpu_attr_is_dev now returns true only for device memory on every
backend, and MPL_gpu_attr_is_strict_dev and
MPIR_GPU_query_pointer_is_strict_dev are removed. Callers that need
host memory check MPL_gpu_attr_is_host; a buffer can be neither, e.g.
ZE host-registered or shared memory.

Previously MPL_gpu_attr_is_dev on ZE returned true for any ZE
allocation, and the device-only check was a separate is_strict_dev.

model: Opus 5.5
Checks for device memory in ccl, the GPU IPC local mmap and
MPIR_Localcopy_gpu now call MPL_gpu_attr_is_dev. This has the same
result.

Previously they compared attr.type with MPL_GPU_POINTER_DEV directly.

model: Opus 5.5
@hzhou-muse

Copy link
Copy Markdown
Contributor Author

test:mpich/ch4/gpu/ofi


/* FIXME: do we allocate buffer for non-strict dev buffer? */
if (!MPL_gpu_attr_is_strict_dev(&attr)) {
/* FIXME: do we allocate buffer for memory that is neither host nor device? */

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The question is whether non-strict dev buffer can operated like a host buf, e.g. memcpy, a+=b. Run a test to check


MPIR_GPU_query_pointer_attr(buf, &queue->attr);
queue->need_pack = (!dt_contig || MPL_gpu_attr_is_dev(&queue->attr));
queue->need_pack = (!dt_contig || !MPL_gpu_attr_is_host(&queue->attr));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depend on the same question as prev.

MPL_pointer_attr_t * outattr)
{
if (!MPL_gpu_attr_is_dev(inattr) && !MPL_gpu_attr_is_dev(outattr)) {
if (MPL_gpu_attr_is_host(inattr) && MPL_gpu_attr_is_host(outattr)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question

@hzhou hzhou left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR poses no behavior changes. We'll merge it as is. There is policy decision that whether in the collective and typerep path, should we perform staging if the buffer is non-strict buffer (registered host or managed). I am leaning toward not-to staging. Registered host should directly perform memcpy and load/store without loss of performance. Managed buffer depends on first touch but should work without error. Using managed buffer is a user choice. If they worry about performance, they should avoid managed buffer in the first place. We'll leave that to a separate PR

Commit 3dddca4 converted getfence1, putfence1 and lock_dt into the
fence (-get/-put) and lock_x_dt test functions but left the old names in
testlist.gpu, so the GPU tests failed to build.

model: Opus 5.5
Commit 3dddca4 set flush_type instead of flush_local_type for the
-flushlocal and -flushlocalall options. The enum values coincide, so
those tests ran MPI_Win_flush and MPI_Win_flush_all instead of the
flush_local variants.

model: Opus 5.5
@hzhou

hzhou commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator

test:mpich/ch4/gpu

@hzhou-muse

Copy link
Copy Markdown
Contributor Author

CI failure analysis for df1b283 (mpich-review-ch4-gpu-ofi#459, mpich-review-ch4-gpu-ucx#272). None of the failures come from this PR.

gpu-ofi: 2 tests fail in all 3 cells (am-only, debug, default):

  • coll/allred 4 -counts=10,100 -memtype=all
  • coll/allred 7 -counts=10 -memtype=all

Both segfault inside MPI_Allreduce. The ucx backtrace shows "invalid permissions for mapped object" from PMPI_Allreduce into libmpi, which means the reduction is running on the host against device buffers.

These failures are on main. They appear in every gpu-ofi build since #455 (Oct 13), including builds of other PRs, and the last clean builds were #450 and #454. The cause is 8d2e84d ("coll/binding: collective bindings call MPIR_Xxx_impl", part of #8016). The bindings used to call the generated MPIR_Allreduce, which did the host-buffer swap: MPIR_Coll_host_buffer_alloc when MPIR_Typerep_reduce_is_supported() is false, and the swap back afterwards. The bindings now call MPIR_Allreduce_impl directly, so the swap is skipped. 843c7eb then deleted the swap code from gen_coll.py, which left MPIR_Coll_host_buffer_* with no callers. I'll fix this in a separate PR that restores the device-buffer swap.

gpu-ucx, other failures (also not from this PR):

  • cuda/stream 2 ... RESERVE_VCIS=1 and the -progress-thread ... GPU_HAS_WAIT_KERNEL=1 timeout: known regression from stream: use MPL_malloc for pt2pt enqueue staging buffers and add impls GPU tests to testlist.gpu #8044.
  • coll/allred2 12 and coll/reduce 7: UCX could not allocate its 512 MB cuda buffer (cuMemAlloc out of memory). Most likely the GPU's memory was in use by other jobs.
  • rma/fence ... -memtype=random: a UCX check failed (mem_type == memh->mem_type, got cuda vs host). I'll look into this one separately.

@hzhou
hzhou merged commit 1cbb526 into pmodels:main Oct 25, 2026
7 of 9 checks passed
@hzhou-muse
hzhou-muse deleted the gpu-attr-is-host branch November 8, 2026 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants