Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.2 support #1

Open
cycle20 opened this issue Jan 13, 2023 · 0 comments
Open

PHP 8.2 support #1

cycle20 opened this issue Jan 13, 2023 · 0 comments

Comments

@cycle20
Copy link

cycle20 commented Jan 13, 2023

Hi,

Thank you for this accurate code base.
I would like to use it with PHP 8.2, but zend API has been changed years ago.
I was able to modify the code - however I am unsure about its correctness.

I had to change an include path:

$ git diff -- php_opencl.h 
diff --git a/php_opencl.h b/php_opencl.h
index 6ae48dd..f8dee17 100644
--- a/php_opencl.h
+++ b/php_opencl.h
@@ -19,7 +19,7 @@
 #include <SAPI.h>
 #include <ext/standard/info.h>
 #include <Zend/zend_extensions.h>
-#include <OpenCL/opencl.h>
+#include <CL/opencl.h>
 
 BEGIN_EXTERN_C()
 

Some macro calls - NOTE: I do not know what is the right replacement for ZEND_REGISTER_RESOURCE:

# git diff -- opencl.c
diff --git a/opencl.c b/opencl.c
index 6334f18..af7437f 100644
--- a/opencl.c
+++ b/opencl.c
@@ -372,7 +372,7 @@ phpcl_get_info(phpcl_get_info_func_t get_info,
                        size_t len = 0;
                        errcode = get_info(obj1, obj2, param->name, sizeof(buf), buf, &len);
                        if (errcode == CL_SUCCESS) {
-                               ZVAL_STRINGL(zinfo, buf, len, 1);
+                               ZVAL_STRINGL(zinfo, buf, len);
                        }
                }
                break;
@@ -381,7 +381,8 @@ phpcl_get_info(phpcl_get_info_func_t get_info,
                        cl_platform_id platform;
                        errcode = get_info(obj1, obj2, param->name, sizeof(cl_platform_id), &platform, NULL);
                        if (errcode == CL_SUCCESS) {
-                               ZEND_REGISTER_RESOURCE(zinfo, platform, le_platform);
+                               zend_register_resource(zinfo, le_platform);
+                               //ZEND_REGISTER_RESOURCE(zinfo, platform, le_platform);
                        }
                }
                break;

I adjusted the configuration by this command:
(Based on this doc https://wiki.php.net/phpng-upgrading)

./configure CFLAGS='-g -O2 -DCL_TARGET_OPENCL_VERSION=120 -DTSRMLS_DC= -DTSRMLS_CC= -DTSRMLS_C= -Dzend_rsrc_list_entry=zend_resource'

I also had to change a function call in params.c, however I have no clue at all:

$ git diff -- params.c
diff --git a/params.c b/params.c
index 177385c..861c642 100644
--- a/params.c
+++ b/params.c
@@ -37,7 +37,7 @@ phpcl_get_devicecs(zval *zv, int argno, cl_uint *num_devices_ret TSRMLS_DC)
 
        if (Z_TYPE_P(zv) == IS_RESOURCE) {
                cl_device_id device = NULL;
-               ZEND_FETCH_RESOURCE_NO_RETURN(device, cl_device_id, &zv, -1,
+               zend_fetch_resource2(device, cl_device_id, &zv, -1,
                                              "cl_device", phpcl_le_device());
                if (device) {
                        devices = emalloc(sizeof(cl_device_id));

I hope that the upgrade is merely syntactic procedure with no semantic consequences.
Can you provide some guide to make this code build-able?
(I have no zend extension experience at all. Probably it requires a long learning curve.)

In a worst case scenario I can write my own C OpenCL code and call it by FFI.
However I think this project is useful for a wider audience and it worth a face lift.
Maybe that is less effort to me and others than an in-house C library.

Best regards,
Csongor

NOTE:
So probably this is the last build error to overcome - I have no time to dig into Zend programming ☹️ :

/root/php-opencl/params.c: In function 'phpcl_get_devicecs':
/root/php-opencl/params.c:40:32: error: expected expression before 'cl_device_id'
   40 |   zend_fetch_resource2(device, cl_device_id, &zv, -1,
      |                                ^~~~~~~~~~~~
/root/php-opencl/params.c:40:24: warning: passing argument 1 of 'zend_fetch_resource2' from incompatible pointer type [-Wincompatible-pointer-types]
   40 |   zend_fetch_resource2(device, cl_device_id, &zv, -1,
      |                        ^~~~~~
      |                        |
      |                        cl_device_id {aka struct _cl_device_id *}
In file included from /usr/local/include/php/Zend/zend_API.h:26,
                 from /usr/local/include/php/main/php.h:35,
                 from /root/php-opencl/php_opencl.h:17,
                 from /root/php-opencl/params.h:10,
                 from /root/php-opencl/params.c:10:
/usr/local/include/php/Zend/zend_list.h:62:52: note: expected 'zend_resource *' {aka 'struct _zend_resource *'} but argument is of type 'cl_device_id' {aka 'struct _cl_device_id *'}
   62 | ZEND_API void *zend_fetch_resource2(zend_resource *res, const char *resource_type_name, int resource_type, int resource_type2);
      |                                     ~~~~~~~~~~~~~~~^~~
/root/php-opencl/params.c:40:3: error: too few arguments to function 'zend_fetch_resource2'
   40 |   zend_fetch_resource2(device, cl_device_id, &zv, -1,
      |   ^~~~~~~~~~~~~~~~~~~~
In file included from /usr/local/include/php/Zend/zend_API.h:26,
                 from /usr/local/include/php/main/php.h:35,
                 from /root/php-opencl/php_opencl.h:17,
                 from /root/php-opencl/params.h:10,
                 from /root/php-opencl/params.c:10:
/usr/local/include/php/Zend/zend_list.h:62:16: note: declared here
   62 | ZEND_API void *zend_fetch_resource2(zend_resource *res, const char *resource_type_name, int resource_type, int resource_type2);
      |                ^~~~~~~~~~~~~~~~~~~~
make: *** [Makefile:213: params.lo] Error 1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant