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

Proposed tidy up of CL_{API,EXT}_{PREFIX,SUFFIX}* definitions #153

Open
1 of 3 tasks
kpet opened this issue Feb 9, 2021 · 6 comments
Open
1 of 3 tasks

Proposed tidy up of CL_{API,EXT}_{PREFIX,SUFFIX}* definitions #153

kpet opened this issue Feb 9, 2021 · 6 comments

Comments

@kpet
Copy link
Contributor

kpet commented Feb 9, 2021

These definitions are used to prefix/suffix function declarations, for example to handle deprecation warnings. They currently fall into two categories:

  • Per-version definitions (CL_{API,EXT}_SUFFIX__{EXPERIMENTAL,VERSION*}). These are always defined to an empty string.
  • Per-version deprecation definitions (CL_EXT_SUFFIX__VERSION_*_DEPRECATED). These are defined to compiler-specific values that result in warnings being produced when deprecated functions are used. Note that there are no CL_API_SUFFIX__VERSION_*_DEPRECATED definitions.

Problems

  • CL_EXT and CL_API prefixes are used inconsistently and core API function declarations are marked as deprecated with CL_EXT definitions
  • The distinction doesn't seem to serve any purpose and is confusing

As another data point, extension functions use CL_API_{CALL,ENTRY} in their declarations.

Proposed resolution

  • Use CL_API_{PREFIX,SUFFIX} for core declarations as well as extensions and remove all CL_EXT_{PREFIX,SUFFIX} definitions.

OpenCL-Docs and OpenCL-CLHPP would need changing as well.

Agreed actions

  • Replace all uses of CL_EXT_{PREFIX,SUFFIX}* with CL_API_{PREFIX,SUFFIX}*
  • Reserve the CL_API prefix for internal use
  • Figure out what to do with suffixes on extension functions declarations (minimum required version? what about deprecation?)
@bashbaug
Copy link
Contributor

Switching to the CL_API_{PREFIX,SUFFIX} vs. CL_EXT_{PREFIX,SUFFIX} makes sense to me, +1.

Is there a usage model for the CL_API_SUFFIX__{EXPERIMENTAL,VERSION*} definitions or can they be removed?

@kpet
Copy link
Contributor Author

kpet commented Feb 11, 2021

Ok, I'll do that.

Is there a usage model for the CL_API_SUFFIX__{EXPERIMENTAL,VERSION*} definitions or can they be removed?

I actually started using them in #154 (which I'd like to get in before the tidy up as they would conflict).

@bashbaug
Copy link
Contributor

I was actually looking at #154 which prompted my question. Right now it looks like we have a possible use for prefix and suffix defiitions generally (for things like #154), and for prefix and suffix defiitions for deprecations for specific versions, but I don't currently see a need for prefix and suffix definitions for specific versions that are not tied to deprecations.

If so, would be fine with:

  • General prefix and suffix definitions that are not version-specific and are used for most APIs. These would replace the current version-specific prefix and suffix definitions for non-deprecated APIs.
  • Version-specific prefix and suffix definitions for APIs that were deprecated in a specific version.

@kpet
Copy link
Contributor Author

kpet commented Feb 12, 2021

Even if we don't have a use for them at the moment I think I'd rather we keep the version-specific definitions as they enable to centrally manage this should we need to in the future. All the annotation work has been done and I think the burden to keep them is minimal.

If someone strongly disagrees though I'm open to making the change as a follow-up PR.

@bashbaug
Copy link
Contributor

I think I'd rather we keep the version-specific definitions as they enable to centrally manage this should we need to in the future. All the annotation work has been done and I think the burden to keep them is minimal.

OK, no problem, makes sense.

The version-specific prefixes and suffixes that bother me the most are the ones on extension APIs in the extension headers: Some APIs have them, some APIs don't, and for the APIs that do have them the version appears to be chosen fairly arbitrarily. Would it make sense to have a non-versioned extension prefix and suffix definition? I don't think the answer is to drop the extension prefixes and suffixes entirely if we're going to support user-supplied prefix and suffix definitions, although extension APIs are a little strange so perhaps this needs a bit more thought.

@kpet
Copy link
Contributor Author

kpet commented Feb 15, 2021

I agree extension function declarations will need some special care. The first idea that comes to mind is to use the minimum required version (which could at least be 1.0 in all cases). Deprecation of extension function declarations is probably worth a discussion as well. I've added an item to the checklist I added at the end of the description.

kpet added a commit to kpet/OpenCL-Docs that referenced this issue Feb 16, 2021
See KhronosGroup/OpenCL-Headers#153 for the
discussion.

Signed-off-by: Kévin Petit <kpet@free.fr>
kpet added a commit to kpet/OpenCL-Headers that referenced this issue Feb 16, 2021
Contributes to KhronosGroup#153

Signed-off-by: Kévin Petit <kevin.petit@arm.com>
kpet added a commit to kpet/OpenCL-CLHPP that referenced this issue Feb 16, 2021
See KhronosGroup/OpenCL-Headers#153 for the
discussion.

Signed-off-by: Kévin Petit <kpet@free.fr>
kpet added a commit to kpet/OpenCL-Headers that referenced this issue Feb 16, 2021
Contributes to KhronosGroup#153

Signed-off-by: Kévin Petit <kevin.petit@arm.com>
kpet added a commit that referenced this issue Feb 19, 2021
Contributes to #153

Signed-off-by: Kévin Petit <kevin.petit@arm.com>
bashbaug pushed a commit to KhronosGroup/OpenCL-Docs that referenced this issue Feb 22, 2021
See KhronosGroup/OpenCL-Headers#153 for the
discussion.

Signed-off-by: Kévin Petit <kpet@free.fr>
kpet added a commit to KhronosGroup/OpenCL-CLHPP that referenced this issue Feb 24, 2021
See KhronosGroup/OpenCL-Headers#153 for the
discussion.

Signed-off-by: Kévin Petit <kpet@free.fr>
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

2 participants