ch4/shm/posix: avoiding device id lookup for ZE registered host memory

[colleeneb]

Pull Request Description

This is a change to fix #7927 . The assertion Assertion local_dev_id < local_ze_device_count was failing because the local dev id was -1 (since Level Zero registered host memory, unlike Level Zero device memory, does not have a specific device associated with it(zeMemGetAllocProperties returns a NULL device handle for it)), but local_ze_device_count is an unsigned int32, so in the comparison the -1 was converted to unsigned, and this was larger than the device count. When the asserts are off, it would return local_to_global_map[-1], which is out of bounds.

To fix this, this PR:

1. Adds a check in MPL_gpu_local_to_global_dev_id for if the local_dev_id is -1 and returns -1 in that case, avoiding the signed-to-unsigned conversion in the assert and the out-of-bounds local_to_global_map[-1] call when the assert is off.

2. Changes MPL_gpu_attr_is_dev to MPL_gpu_attr_is_strict_dev in src/mpid/ch4/shm/posix/posix_rma.h for whenever it tries to look up a device_id, since ZE host registered memory does not have a specific device id. (MPL_gpu_attr_is_strict_dev is only true for ZE device memory, and false for ZE host memory). This isn't necessary after the change in 1) but it seems clearer to me since if it's host registered memory we know there's no device id.

3. Changes MPL_gpu_attr_is_dev to MPL_gpu_attr_is_strict_dev in src/mpid/ch4/shm/posix/posix_rma.h as arguments to MPIDI_RMA_choose_engine. This is to avoid hitting another assert at

   mpich/src/mpl/src/gpu/mpl_gpu_ze.c

   Line 1594 in 6a79bf6

   assert(global_dev1 >= 0);

   if MPIDI_RMA_choose_engine is called with MPL_gpu_attr_is_dev and then calls MPL_gpu_query_is_same_dev. If instead MPL_gpu_attr_is_strict_dev is used, for Level Zero registered host memory, it will be considered "host memory" and it takes the host memory engine path (

   mpich/src/mpid/ch4/shm/posix/posix_rma.h

   Line 91 in 6a79bf6

   return MPL_GPU_ENGINE_TYPE_COPY_HIGH_BANDWIDTH;

   ).

I'm happy to make any adjustments in the PR! And thanks to Claude for help understanding the code.

Author Checklist

• Provide Description
  Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
• Commits Follow Good Practice
  Commits are self-contained and do not do two things at once.
  Commit message is of the form: module: short description
  Commit message explains what's in the commit.
• Passes All Tests
  Whitespace checker. Warnings test. Additional tests via comments.
• Contribution Agreement
  For non-Argonne authors, check contribution agreement.
  If necessary, request an explicit comment from your companies PR approval manager.

[hzhou]

but local_ze_device_count is an unsigned int32, so in the comparison the -1 was converted to unsigned, and this was larger than the device count.

This is root of the issue. I always advocate to use signed int for any variables that will do arithmetic or comparisons. This is a case in point.

[hzhou]

MPL_gpu_attr_is_dev and MPL_gpu_attr_is_strict_dev are so confusing to any one who is not familiar with ZE. I think I we should have MPL_gpu_attr_is_dev and MPL_gpu_attr_is_host. !MPL_gpu_attr_is_host() would be so much clearer than the current MPL_gpu_attr_is_dev.

[colleeneb]

Agreed! I can change it here or in another PR unless you want to.

For local_ze_device_count is an unsigned int32,, same I can change it here or in another PR.

[hzhou]

I think this PR is good as is. Let's make a separate PR for refactoring.

[hzhou]

LGTM