[Kerilk]

Adds an optional 'group' attribute to definitions in cl.xml, following the gl.xml convention: each token names the C value-set type(s) it belongs to (comma-separated for multi-role tokens). The RNC schema is extended with 'attribute group { text } ?' on enums.

Attributions were derived from the OpenCL core and extension specifications (cross-checked against the Khronos registry chapters and cl.xml's own containers), not from container names alone.

Changes xml/cl.xml and xml/registry.rnc only; generated headers are byte-identical (the attribute is metadata no current generator reads).

Chose not to implement the group property at the container level to avoid breaking the existing tooling.

[CLAassistant]

All committers have signed the CLA.

[bashbaug]

Thanks for doing this! Mostly LGTM, but here are a few things I would check.

The biggest question IMHO is what to do when a type / "group" starts as an extension and get promoted to a core feature, or moves from one extension type to another (e.g. vendor -> ext -> khr).

[bashbaug]

It's a little strange that we don't name a type for this. Should we? Regardless, clCommandExecutionStatus looks a lot more like an API function name than a type name, so we might want to consider something different for it. Maybe just CommandExecutionStatus?

[Kerilk]

Do we want to add a typedef cl_int cl_command_execution_status?

I agree with the above suggestion in the meantime.

[Kerilk]

When I changed the name of the group to CommandExecutionStatus I did not notice that the enum carries a name of clCommandExecutionStatus.

<enums name="clCommandExecutionStatus" vendor="Khronos">

Also we need to update CL_ECC_RECOVERED_IMG which for now carries clCommandExecutionStatus for a group.

@bashbaug what is your take on this? I am worried that changing the name field above might lead to problems for project that were relying on it.

[bashbaug]

I don't think I have a perfect answer, but here are a few options to consider:

1. Rename the enum name and group to cl_command_execution_status. This seems like the right thing to do with the benefit of hindsight. If we can convince ourselves that there is no problem actually adding the type (as you suggested above), then we can do so, but even if not I think this is the right name for the group.

2. Rename the enum name and group to CommandExecutionStatus so it doesn't look like an API name or an official type name. We wouldn't add a typedef if we did this.

3. Don't consider these enums part of any group (note that CL_COMMAND_EXECUTION_STATUS just returns a cl_int). I suspect this was probably done because the "command execution status" can also be an error code if the command is abnormally terminated.

   • Note, at some point in the future, maybe we should name error codes and address the command execution status at the same time - I think one of these PRs proposed doing something like this - but this would be a much bigger change.

4. Just stick with clCommandExecutionStatus for now. It's a little weird, but we've been living with it, and we may as well stick with it if we aren't sure what a better name is.

[Kerilk]

4 seems reasonable to me, We'd need to revert one of the suggestions I merged, but that seems rather straightforward.

[bashbaug]

Likewise, these should probably be cl_svm_mem_flags_arm.

[Kerilk]

Thanks for catching this one. Other SVM flags that were accepted by the clSVMAllocARM should maby get cl_svm_mem_flags_arm a second tag, what do you think?

[bashbaug]

It's a little confusing to me to list both cl_queue_properties and cl_queue_properties_khr here, but I do think this is correct. As a consideration for the future, if we ever do another extension like cl_khr_create_command_queue, are there things we should do differently that would be less confusing? Could we reuse the cl_queue_properties type, for example?

[Kerilk]

In this instance, the WithProperties API appeared first in an extension, and the type with it, so I am unsure how to get around this particular sequencing.

[Kerilk]

Thanks for doing this! Mostly LGTM, but here are a few things I would check.

Thanks for looking through it, and catching all those things I missed.

The biggest question IMHO is what to do when a type / "group" starts as an extension and get promoted to a core feature, or moves from one extension type to another (e.g. vendor -> ext -> khr).

Good question. I would prioritize the approach that would help type safety when creating bindings from the xml file, since I believe this is the main benefit from these changes.

[Kerilk]

Suggested change

	<enum value="0x1112" name="CL_IMAGE_ROW_PITCH" group="cl_image_info"/>
	<enum value="0x1113" name="CL_IMAGE_SLICE_PITCH" group="cl_image_info"/>
	<enum value="0x1112" name="CL_IMAGE_ROW_PITCH" group="cl_image_info,cl_image_pitch_info_qcom"/>
	<enum value="0x1113" name="CL_IMAGE_SLICE_PITCH" group="cl_image_info,cl_image_pitch_info_qcom"/>

@bashbaug I think this is also correct, and was proposed by earlier attempts at doing this.