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

[XML] Consistently group <enum>-s under their <type> #1145

Open
SunSerega opened this issue Apr 4, 2024 · 0 comments
Open

[XML] Consistently group <enum>-s under their <type> #1145

SunSerega opened this issue Apr 4, 2024 · 0 comments

Comments

@SunSerega
Copy link
Contributor

The problem

High-level bindings benefit from grouping enum values under a common type name.
This allows verbose IntelliSense suggestions and avoids oversaturating the namespace with all the enum values in one place.

The cl.xml file in this repo already defined its own enum type for every enum group:

        <type category="define">typedef <type>cl_uint</type>          <name>cl_device_local_mem_type</name>;</type>
...
    <enums name="cl_device_local_mem_type" vendor="Khronos">
        <enum value="0x1"           name="CL_LOCAL"/>
        <enum value="0x2"           name="CL_GLOBAL"/>
            <unused start="0x3" end="9999"/>
    </enums>
...
        <require comment="cl_device_local_mem_type">
            <enum name="CL_LOCAL"/>
            <enum name="CL_GLOBAL"/>
        </require>

However, there is currently no consistent way in which XML links the enum type (cl_device_local_mem_type above) and its valid values.


Old solutions

The <enums> blocks seem to have been initially intended for this purpose.

However, this linkage was not maintained. At some point people just started adding all their new enums to:

    <enums start="0x1000" end="0x107F" name="cl_device_info" vendor="Khronos">

#779 has tried to rectify this, but was abandoned.

Initially, I also tried to continue the work in #779, because I was already using (and un-messing in other ways) the <enums> tags.

But then realized that the moderate abuse of comment attributes on <require> tags, of all things, was the most maintained form of such linkage between enums and their types.
I guess it's just a more natural form for it (in this repo at least).


My solution

With that, I made #926, which currently replaces all the comment attributes referring to an enum type with a group attribute.

A few odd-balls of the comment attributes currently follow one of these 3 patterns:

        # The "- bitfield" and the end (missing on some bitfields, I think I caught all of those)
        <require comment="cl_device_type - bitfield">
...
        # 2 enum types sharing <require> tag
        <require comment="cl_mem_flags and cl_svm_mem_flags - bitfield">
...
        # Human-only text describing things like deprecation, also containing the actual enum type name
        <require comment="OpenCL 1.2 cl_image_info enums that were deprecated in OpenCL 2.0">

I've converted them into:

        # Add etype (enum type/kind/usage), attribute
        <require group="cl_device_type" etype="bitfield">
...
        # Duplicate <require> tag with all of its contents
        <require group="cl_mem_flags" etype="bitfield">
        <require group="cl_svm_mem_flags" etype="bitfield">
...
        # Leave the comment, because the deprecatedby attribute is not supported by <require> inside <feature>
        <require group="cl_image_info" comment="OpenCL 1.2 cl_image_info enums that were deprecated in OpenCL 2.0">

And a few remaining <require> blocks with <enum>-s inside I've converted to either of these 2 etype-s:

        # Mark non-grouped defines, denoting separate constants rather than enumerated values
        <require etype="constants" comment="Misc API enums">
...
            # Mark enums, not used on the HOST side
            <require etype="OpenCL-C only">

My bindings' CI validates that no <require> block with enums avoids this conversion.


Future PRs

The etype attribute was made with an idea for another 2 kinds of enum-to-type linking metadata, which I wanted to transfer to this repo from my bindings because I think it would be useful for other XML consumers.

  1. Object info enums:

    • Enums in cl_device_info are each linked to a type of value the clGetDeviceInfo function returns;
    • Enums in cl_kernel_exec_info are each linked to a type of value the clSetKernelExecInfo function accepts;
    • Enums in cl_kernel_sub_group_info do both in the clGetKernelSubGroupInfo function.
  2. Property list enums:
    Enums in cl_device_partition_property are used to make lists, where values of other types are cast to cl_device_partition_property and then passed to the clCreateSubDevices function.

    In particular:

    • CL_DEVICE_PARTITION_EQUALLY is followed by a cl_uint;
    • CL_DEVICE_PARTITION_BY_COUNTS is followed by a list of cl_uint-s;
    • CL_DEVICE_PARTITION_BY_COUNTS_LIST_END terminates this list.

This metadata allows the creation of more strongly typed overloads in bindings.

I've already added and CI-validated (and then also maintained since the creation of #926) all this metadata to XML in the custom branch of my fork.
I'm not entirely happy with how I'm representing it in XML right now, and I'm ready to rewrite all of it.
But I think I'd rather make another issue after putting #926 to rest, on the off chance the extra questions about this metadata might stall the discussion about #926.
Well, I'll still answer here if anyone wants to talk about this metadata in this thread.


Any suggestions on how this could be done better?
Seriously, I'd take any scrutiny over having #926 just sit for even longer...

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