Refactor separate can_access_peer methods into a single method

Author: ndgrigorian

dpctl/_sycl_device.pyx

@@ -1855,7 +1855,7 @@ cdef class SyclDevice(_SyclDevice):
             raise ValueError("Internal error: NULL device vector encountered")
         return _get_devices(cDVRef)
 
-    def can_access_peer_access_supported(self, peer):
+    def can_access_peer(self, peer, value="access_supported"):
         """ Returns ``True`` if this device (``self``) can enable peer access
         to USM device memory on ``peer``, ``False`` otherwise.
 
@@ -1869,60 +1869,50 @@ cdef class SyclDevice(_SyclDevice):
             peer (:class:`dpctl.SyclDevice`):
                 The :class:`dpctl.SyclDevice` instance to check for peer access
                 by this device.
+            value (str, optional):
+                Specifies the kind of peer access being queried
 
-        Returns:
-            bool:
-                ``True`` if this device may access USM device memory on
-                ``peer`` when peer access is enabled, otherwise ``False``.
+                - ``"access_supported"``
+                    Returns ``True`` if it is possible for this device to
+                    enable peer access to USM device memory on ``peer``.
 
-        Raises:
-            TypeError:
-                If ``peer`` is not :class:`dpctl.SyclDevice`.
-        """
-        cdef SyclDevice p_dev
+                - ``"atomics_supported"``
+                    Returns ``True`` if it is possible for this device to
+                    concurrently access and atomically modify USM device
+                    memory on ``peer`` when enabled.
 
-        if not isinstance(peer, SyclDevice):
-            raise TypeError(
-                "peer device must be a `dpctl.SyclDevice`, got "
-                f"{type(peer)}"
-            )
-        p_dev = <SyclDevice>peer
-        if _check_peer_access(self, p_dev):
-            return DPCTLDevice_CanAccessPeer(
-                self._device_ref,
-                p_dev.get_device_ref(),
-                _peer_access._access_supported
-            )
-        return False
+                    If ``False`` is returned, these operations result in
+                    undefined behavior.
 
-    def can_access_peer_atomics_supported(self, peer):
-        """ Returns ``True`` if this device (``self``) can concurrently access
-        and modify USM device memory on ``peer`` when peer access is enabled,
-        ``False`` otherwise.
+                    Note: atomics must have ``memory_scope::system`` when
+                    modifying memory on a peer device.
 
-        If peer access is supported, it may be enabled by calling
-        :meth:`.enable_peer_access`.
-
-        For details, see
-        :oneapi_peer_access:`DPC++ peer access SYCL extension <>`.
-
-        Args:
-            peer (:class:`dpctl.SyclDevice`):
-                The :class:`dpctl.SyclDevice` instance to check for concurrent
-                peer access and modification by this device.
+                Default: ``"access_supported"``
 
         Returns:
             bool:
-                ``True`` if this device may concurrently access and modify USM
-                device memory on ``peer`` when peer access is enabled,
-                otherwise ``False``.
+                ``True`` if this device may access USM device memory on
+                ``peer`` when peer access is enabled, otherwise ``False``.
 
         Raises:
             TypeError:
                 If ``peer`` is not :class:`dpctl.SyclDevice`.
         """
         cdef SyclDevice p_dev
 
+        if not isinstance(value, str):
+            raise TypeError(
+                f"Expected `value` to be of type str, got {type(value)}"
+            )
+        if value == "access_supported":
+            access_type = _peer_access._access_supported
+        elif value == "atomics_supported":
+            access_type = _peer_access._atomics_supported
+        else:
+            raise ValueError(
+                "`value` must be 'access_supported' or 'atomics_supported', "
+                f"got {value}"
+            )
         if not isinstance(peer, SyclDevice):
             raise TypeError(
                 "peer device must be a `dpctl.SyclDevice`, got "
@@ -1933,7 +1923,7 @@ cdef class SyclDevice(_SyclDevice):
             return DPCTLDevice_CanAccessPeer(
                 self._device_ref,
                 p_dev.get_device_ref(),
-                _peer_access._atomics_supported
+                access_type
             )
         return False
 
@@ -2002,11 +1992,10 @@ cdef class SyclDevice(_SyclDevice):
             )
         p_dev = <SyclDevice>peer
         _raise_invalid_peer_access(self, p_dev)
-        if _check_peer_access(self, p_dev):
-            DPCTLDevice_DisablePeerAccess(
-                self._device_ref,
-                p_dev.get_device_ref()
-            )
+        DPCTLDevice_DisablePeerAccess(
+            self._device_ref,
+            p_dev.get_device_ref()
+        )
         return
 
     @property

dpctl/tests/test_sycl_device.py

@@ -360,8 +360,10 @@ def test_can_access_peer(platform_name):
         )
     dev0 = devices[0]
     dev1 = devices[1]
-    assert isinstance(dev0.can_access_peer_access_supported(dev1), bool)
-    assert isinstance(dev0.can_access_peer_atomics_supported(dev1), bool)
+    assert isinstance(dev0.can_access_peer(dev1), bool)
+    assert isinstance(
+        dev0.can_access_peer(dev1, value="atomics_supported"), bool
+    )
 
 
 @pytest.mark.parametrize("platform_name", ["level_zero", "cuda", "hip"])
@@ -381,7 +383,7 @@ def test_enable_disable_peer_access(platform_name):
         )
     dev0 = devices[0]
     dev1 = devices[1]
-    if dev0.can_access_peer_access_supported(dev1):
+    if dev0.can_access_peer(dev1):
         dev0.enable_peer_access(dev1)
         dev0.disable_peer_access(dev1)
     else:
@@ -393,8 +395,7 @@ def test_enable_disable_peer_access(platform_name):
 @pytest.mark.parametrize(
     "method",
     [
-        "can_access_peer_access_supported",
-        "can_access_peer_atomics_supported",
+        "can_access_peer",
         "enable_peer_access",
         "disable_peer_access",
     ],
@@ -427,3 +428,33 @@ def test_peer_access_to_self(platform_name):
         dev.enable_peer_access(dev)
     with pytest.raises(ValueError):
         dev.disable_peer_access(dev)
+
+
+def test_peer_access_value_keyword_validation():
+    """
+    Validate behavior of `can_access_peer` for invalid `value` keyword.
+    """
+    # we pick an arbitrary platform that supports peer access
+    platforms = dpctl.get_platforms()
+    peer_access_backends = [
+        dpctl.backend_type.cuda,
+        dpctl.backend_type.hip,
+        dpctl.backend_type.hip,
+    ]
+    devs = None
+    for p in platforms:
+        if p.backend in peer_access_backends:
+            p_devs = p.get_devices()
+            if len(p_devs) >= 2:
+                devs = p_devs
+                break
+    if devs is None:
+        pytest.skip("No platform available with enough devices")
+    dev0 = devs[0]
+    dev1 = devs[1]
+    bad_type = 2
+    with pytest.raises(TypeError):
+        dev0.can_access_peer(dev1, value=bad_type)
+    bad_value = "wrong"
+    with pytest.raises(ValueError):
+        dev0.can_access_peer(dev1, value=bad_value)