OpenCL-Docs icon indicating copy to clipboard operation
OpenCL-Docs copied to clipboard

XML File and cl_icd_dispatch

Open bashbaug opened this issue 3 months ago • 2 comments

#1444 updated the XML file to add cl_icd_dispatch as a requirement for the cl_loader_layers extension.

This seemed like the right thing to do - see https://github.com/KhronosGroup/OpenCL-Docs/pull/1444/files#r2283135496 - but it causes issues with the generated headers, because it causes another typedef to be generated after the structure definition, which is not valid for some C versions.

In file included from /__w/OpenCL-Headers/OpenCL-Headers/tests/test_cl_layer.h.c:25:
/__w/OpenCL-Headers/OpenCL-Headers/CL/cl_layer.h:59:33: error: redefinition of typedef 'cl_icd_dispatch' [-Werror=pedantic]
   59 | typedef struct _cl_icd_dispatch cl_icd_dispatch;
      |                                 ^~~~~~~~~~~~~~~
In file included from /__w/OpenCL-Headers/OpenCL-Headers/CL/cl_layer.h:24:
/__w/OpenCL-Headers/OpenCL-Headers/CL/cl_icd.h:332:3: note: previous declaration of 'cl_icd_dispatch' with type 'cl_icd_dispatch' {aka 'struct _cl_icd_dispatch'}
  332 | } cl_icd_dispatch;
      |   ^~~~~~~~~~~~~~~

How do we want to fix this? Some options to consider:

  1. Remove the requirement from the XML file, so the duplicate typedef will not be generated in the extension headers.
  2. Remove the typedef from the structure definition in cl_icd.h, so the generated typedef is not a duplicate. Note: This could break code that is including cl_icd.h but not cl_layer.h and expecting the typedef to be present.
  3. Do not include cl_icd.h from cl_layer.h. Note: While this would probably work for some usages, it'd still be a problem if both headers are included in the "wrong" order, and most layers will want to include both headers.
  4. Add a special case in the header generation script to skip generating the duplicate typedef.

I don't like any of these options, so I'm hoping there's another alternative to consider. Thanks!

CC: @Kerilk @RocketRide9

bashbaug avatar Sep 09 '25 22:09 bashbaug

My view: I think the require should be in a formal XML definition of cl_icd.h (yes, I know we did not want to specify this) that formally bring the cl_icd_dispatch definition. Since the inclusion is described in the xml, the cl_loader_layer would get it transitively.

            <require>
                <type name="CL/cl_icd.h"/>
            </require>

Kerilk avatar Sep 10 '25 20:09 Kerilk

Sorry about that PR. Maybe it should be marked that cl_icd_dispatch is provided by another header and generator would know that it doesn't need to emit a typedef in the current header? If cl.h was generated too, I think that constants and API data types that were added in #1444 into OpenCL 1.0 feature would be problematic too. Because most of the items from OpenCL 1.0 are defined in cl.h and mentioned constants and types are defined in a different header file - cl_platform.h.

vdualb avatar Sep 14 '25 10:09 vdualb