Skip to content

Comments

Added cl_icdl.h header file.#214

Closed
Kerilk wants to merge 4 commits intoKhronosGroup:mainfrom
Kerilk:cl_icdl
Closed

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

Conversation

@Kerilk
Copy link
Contributor

@Kerilk Kerilk commented Nov 10, 2022

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

bashbaug
bashbaug previously approved these changes Feb 25, 2023
Copy link
Contributor

@bashbaug bashbaug left a comment

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
Copy link
Contributor Author

Kerilk commented Feb 25, 2023

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
Co-authored-by: Ben Ashbaugh <ben.ashbaugh@intel.com>
@Kerilk
Copy link
Contributor Author

Kerilk commented Apr 11, 2023

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