Make SVM practical

[inducer]

Draft because it's missing:

• Tests
  • Ensure that SVM allocators can't be passed to buffer mem pool and vice versa.
• A memory pool for SVM
  • Quite subtle: Memory pools and multiple queues: missing synchronization #447
• Documentation, especially about synchronization of deallocation with pending work.
• demo_array_svm crashes for the moment, the queue-associated SVM allocation is one potential solution for that, at least for in-order queues. See also SVM deallocation should wait for completion of operations on them #449.
• via @matthiasdiener Check for needed virtual destructors via clang.
• Pass gitlab CI: https://gitlab.tiker.net/inducer/pyopencl/-/tree/array-svm
• Do a full review

[matthiasdiener]

With 3abf89c, examples/svm.py is failing with the following message:

  File "/shared/home/mdiener/Work/pyopencl/examples/svm.py", line 35, in <module>
    svm_ary = cl.SVM(cl.csvm_empty(ctx, 10, np.float32))
  File "/shared/home/mdiener/Work/pyopencl/pyopencl/__init__.py", line 2104, in csvm_empty
    return svm_empty(ctx, svm_mem_flags.READ_WRITE, shape, dtype, order, alignment)
  File "/shared/home/mdiener/Work/pyopencl/pyopencl/__init__.py", line 2062, in svm_empty
    svm_alloc = _ArrayInterfaceSVMAllocation(
NameError: name '_ArrayInterfaceSVMAllocation' is not defined

The following patch works around it (it also adds the missing self.__array_interface__):

diff --git a/pyopencl/__init__.py b/pyopencl/__init__.py
index 22562d1..9c62cea 100644
--- a/pyopencl/__init__.py
+++ b/pyopencl/__init__.py
@@ -1141,21 +1141,6 @@ def _add_functionality():
             .. automethod:: enqueue_release
             """

-    if get_cl_header_version() >= (2, 0):
-        class _ArrayInterfaceSVMAllocation(SVMAllocation):
-            def __init__(self, ctx, size, alignment, flags, _interface=None,
-                    queue=None):
-                """
-                :arg ctx: a :class:`Context`
-                :arg flags: some of :class:`svm_mem_flags`.
-                """
-                super().__init__(ctx, size, alignment, flags, queue)
-
-                # mem_flags.READ_ONLY applies to kernels, not the host
-                read_write = True
-                _interface["data"] = (
-                        int(self._ptr_as_int()), not read_write)
-
     # }}}

     # {{{ SVM
@@ -1396,6 +1381,23 @@ def _add_functionality():

 _add_functionality()

+if get_cl_header_version() >= (2, 0):
+    class _ArrayInterfaceSVMAllocation(SVMAllocation):
+        def __init__(self, ctx, size, alignment, flags, _interface=None,
+                queue=None):
+            """
+            :arg ctx: a :class:`Context`
+            :arg flags: some of :class:`svm_mem_flags`.
+            """
+            super().__init__(ctx, size, alignment, flags, queue)
+
+            # mem_flags.READ_ONLY applies to kernels, not the host
+            read_write = True
+            _interface["data"] = (
+                    int(self._ptr_as_int()), not read_write)
+
+            self.__array_interface__ = _interface
+
 # }}}

(Do you prefer me to send these as PRs on top of this PR?)

[inducer]

7f20c75 is a WIP commit that doesn't even compile. It's intended to prevent further duplication of work with @matthiasdiener (cf. #587). Sorry about not getting this pushed sooner.

[inducer]

0cf016f has broken compatibility with examples/svm.py. I'll look into it.

[nchristensen]

Attemping to run svm.py on a Crusher MI250X with Rocm 5.2.0 results in a segmentation fault.

Device 'gfx90a:sramecc-:xnack-' on platform 'AMD Accelerated Parallel Processing (OpenCL 2.1 AMD-APP (3452.0))' has the following SVM features:
  Coarse-grained buffer SVM: True
  Fine-grained buffer SVM:   True
  Fine-grained system SVM:   False
/autofs/nccs-svm1_home1/njchris/Workspace/pyopencl/pyopencl/__init__.py:272: CompilerWarning: Non-empty compiler output encountered. Set the environment variable PYOPENCL_COMPILER_OUTPUT=1 to see more.
  warn("Non-empty compiler output encountered. Set the "
Segmentation fault

[inducer]

@nchristensen

Segmentation fault

Are you able to get a backtrace (e.g. with gdb?)

[inducer]

It appears that a good part of the slower execution of SVM comes from the ordering of the argument type checks in Kernel.set_arg:

pyopencl/src/wrap_cl.hpp

Lines 4504 to 4518 in 8bce4f6

	try
	{
	set_arg_mem(arg_index, arg.cast<memory_object_holder &>());
	return;
	}
	catch (py::cast_error &) { }
	#if PYOPENCL_CL_VERSION >= 0x2000
	try
	{
	set_arg_svm(arg_index, arg.cast<svm_arg_wrapper const &>());
	return;
	}
	catch (py::cast_error &) { }
	#endif

We check for memory objects (images, buffers) first, and for SVM second. FWIW, reversing that order seems to help a lot, and the exception handling appears to play a big part in the time spent. Unfortunately, according to pybind/pybind11#649, the try/except solution appears to be the only way to see whether a cast will succeed in pybind.

[nchristensen]

After pulling in the lastest changes and recompiling I am no longer seeing segmentation faults on Rocm 5.2.0 for svm.py or demo_array_svm.py.

[inducer]

With the changes in https://github.com/inducer/pyopencl/compare/7ef5ce5ec280038559295d45986c96302c193d6d..87420b1334806b7fa0fb0104e80a09366869b9f0, this performs about equivalently to buffers (with pocl) in my unscientific benchmarking.

[inducer]

OK, I've done my final review, made some final fixes. If it passes CI here and on Gitlab, then it'll go in. 🎉

[inducer]

I'll call that passing. 🙂

[inducer]

https://pypi.org/project/pyopencl/2022.2/