Skip to content

Added cl_icdl.h header file.#214

Closed
Kerilk wants to merge 4 commits into
KhronosGroup:mainfrom
Kerilk:cl_icdl
Closed

Added cl_icdl.h header file.#214
Kerilk wants to merge 4 commits into
KhronosGroup:mainfrom
Kerilk:cl_icdl

Conversation

@Kerilk

@Kerilk Kerilk commented Nov 10, 2022

Copy link
Copy Markdown
Contributor

This is a companion PR for:
KhronosGroup/OpenCL-ICD-Loader#188

Comment thread CL/cl_icdl.h Outdated
bashbaug
bashbaug previously approved these changes Feb 25, 2023

@bashbaug bashbaug left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with as-is, but just to ask the question: should we consider putting these additions into the existing cl_ext.h file vs. adding a new file?

I guess these extensions are little different than the other extensions in cl_ext.h: at least currently they won't go into any queryable extension strings, for example. Perhaps that's a good enough reason to keep them separate?

Regardless, I think I'll try to update my header generation scripts to generate the defines and typedefs for this extension - seems like that should be pretty straightforward.

@Kerilk

Kerilk commented Feb 25, 2023

Copy link
Copy Markdown
Contributor Author

Thanks for the insightful feedback.

I'm fine with as-is, but just to ask the question: should we consider putting these additions into the existing cl_ext.h file vs. adding a new file?

Very good question. I discarded putting the extension in the cl_icd.h file as its usage is very specific and not application facing. I hadn't thought about cl_ext.h. Given users could want to invoke it from their applications it would make sense I guess. Tell me if you want me to go this route.

bashbaug
bashbaug previously approved these changes Mar 10, 2023
Comment thread CL/cl_ext.h
Co-authored-by: Ben Ashbaugh <ben.ashbaugh@intel.com>
@Kerilk

Kerilk commented Apr 11, 2023

Copy link
Copy Markdown
Contributor Author

Closing as merged by #161

@Kerilk Kerilk closed this Apr 11, 2023
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.

2 participants