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 <extension>: Replace remaining condition with depends #1088

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

SunSerega
Copy link
Contributor

Asked at the end of #950. This in particular wasn't answered yet, but after getting a bit more comfortable with new XML - I'm pretty sure this does need to be fixed.

Note: There are also a bunch of <require> tags with condition instead of the depends attribute.
The depends attribute is being used for these too, but only in one place:

            <require depends="CL_VERSION_2_0" comment="From version 0.9.4 of the extension">

But half of the condition attributes here use negation:

            <require condition="!defined(CL_VERSION_1_2)" comment="cl_device_info - defined in CL.h for OpenCL 1.2 and newer">

And there is also one like this:

            <require condition="defined(_WIN32)">

Which is neither a version nor an extension name, so I guess this one cannot be converted to depends?

@bashbaug
Copy link
Contributor

The "conditions" are currently used by the extension header generation scripts:

https://github.com/KhronosGroup/OpenCL-Headers/blob/59452533d2afa817bc2dc0da4f783097f4cdbcb0/scripts/cl_ext.h.mako#L307

I'm OK doing something like this PR to remove redundancy, but we'll need to update the header generation scripts as well if we make this change.

@SunSerega
Copy link
Contributor Author

Ah, I was somehow sure there were more condition attributes before => most of them were replaced by depends => in-house scripts already work with depends to do the same thing.

@oddhack
Copy link
Contributor

oddhack commented Mar 29, 2024

The 'condition' is something OpenCL-specific. The 'depends' was introduced when syncing with the Vulkan scripts and has the same restrictions, it only speaks to extension and core version dependencies.

@SunSerega
Copy link
Contributor Author

and has the same restrictions

Well, it can represent more cases. For instance in this case:

        <extension name="cl_khr_subgroups" supported="opencl" promotedto="CL_VERSION_2_1" ratified="opencl">
            <require>
                <type name="CL/cl.h"/>
            </require>
            <require condition="!defined(CL_VERSION_2_1)" comment="defined in CL.h for OpenCL 2.1 and newer">
                <type name="cl_kernel_sub_group_info"/>
            </require>
            <require comment="cl_kernel_sub_group_info">
                <enum name="CL_KERNEL_MAX_SUB_GROUP_SIZE_FOR_NDRANGE_KHR"/>
                <enum name="CL_KERNEL_SUB_GROUP_COUNT_FOR_NDRANGE_KHR"/>
            </require>
            <require>
                <command name="clGetKernelSubGroupInfoKHR"/>
            </require>
        </extension>

Here, I don't know how to convert this condition to depends, because of the NOT operator.
But then again, maybe this one doesn't add any new info. Presumably, if an extension is promotedto="CL_VERSION_2_1", then all types it adds should only be defined when CL_VERSION_2_1 is not defined.

But what about all the extensions with depends but not condition - are there cases where generating a macro condition in the header based on depends would be wrong?

@oddhack oddhack mentioned this pull request Apr 1, 2024
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

Successfully merging this pull request may close these issues.

3 participants