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

Move the dispatch table declaration to OpenCL-Headers? #72

Closed
kpet opened this issue May 15, 2019 · 3 comments · Fixed by #80
Closed

Move the dispatch table declaration to OpenCL-Headers? #72

kpet opened this issue May 15, 2019 · 3 comments · Fixed by #80

Comments

@kpet
Copy link
Contributor

kpet commented May 15, 2019

The various function pointer types as well as the declaration for the dispatch table in icd_dispatch.h would be easier to reuse directly in OpenCL implementations if they were part of OpenCL-Headers.

It seems to me that only the declaration for the structure types at the bottom of the file should be part of the ICD loader itself.

Is there a good reason to not do that?

@bashbaug
Copy link
Contributor

Interesting idea. This makes sense to me. I think it could be useful for some other projects as well, and not just for OpenCL implementations.

At some point I'd also like to investigate generating things like the function pointer types automatically from the API cl.xml file.

@mmha
Copy link
Contributor

mmha commented May 16, 2019

A while ago I wrote a small clang tool for a similar use case. It takes a header with (member) function declarations generates a cpp file with a table of function pointers that are either eagerly initialized after loading the shared object (emulating the PLT, so no runtime overhead) or lazily through static variables (example output). It's basically an ICD loader generator for arbitrary APIs.

I can clean up the source, make it suitable for the ICD and make it public if that's of any interest.

The headers should contain the declarations in my opinion, nothing else. I would prefer it to generate the tables as part of the ICD loader build, be it from cl.xml (probably saner) or a clang tool accepting the headers.

@kpet
Copy link
Contributor Author

kpet commented May 21, 2019

@bashbaug Thanks for the feedback. I'll create PRs when I find a bit of time to spend on this. Agree a lot of this can be generated.

@mmha That sounds cool but I'm wondering whether that's not a bit overkill for what we're doing here, especially given that we have a description of the API in machine-parseable form. Also, generating the tables comes with the interesting issue of guaranteeing that the order of members remain the same as that of the current structure. I think we have far too much code in the wild that relies on this order to change it now.

kpet added a commit to kpet/OpenCL-Headers that referenced this issue Jul 2, 2019
...so that implementations (and others) can use definitions
straight from the headers instead of mirroring them.

The definitions come from https://github.com/KhronosGroup/OpenCL-ICD-Loader
with the following name changes:

- function pointers are now prefixed with 'cl_pfn_'
- the dispatch table type is now 'cl_icd_vendor_dispatch_table'

Contributes to KhronosGroup/OpenCL-ICD-Loader#72

Signed-off-by: Kevin Petit <kevin.petit@arm.com>
kpet added a commit to kpet/OpenCL-ICD-Loader that referenced this issue Jul 2, 2019
…eaders

Depends on KhronosGroup/OpenCL-Headers#50.

Fixes KhronosGroup#72.

Signed-off-by: Kevin Petit <kevin.petit@arm.com>
kpet added a commit to kpet/OpenCL-Headers that referenced this issue Jul 11, 2019
...so that implementations (and others) can use definitions
straight from the headers instead of mirroring them.

The definitions come from https://github.com/KhronosGroup/OpenCL-ICD-Loader
with the following name changes:

- function pointers are now prefixed with 'cl_pfn_'
- the dispatch table type is now 'cl_icd_dispatch_table'

Contributes to KhronosGroup/OpenCL-ICD-Loader#72

Signed-off-by: Kevin Petit <kevin.petit@arm.com>
kpet added a commit to kpet/OpenCL-ICD-Loader that referenced this issue Jul 11, 2019
…eaders

Depends on KhronosGroup/OpenCL-Headers#50.

Fixes KhronosGroup#72.

Signed-off-by: Kevin Petit <kevin.petit@arm.com>
kpet added a commit to kpet/OpenCL-Headers that referenced this issue Aug 8, 2019
...so that implementations (and others) can use definitions
straight from the headers instead of mirroring them.

The definitions come from https://github.com/KhronosGroup/OpenCL-ICD-Loader
with the following name changes:

- function pointers are now prefixed with 'cl_pfn_'
- the dispatch table type is now 'cl_icd_dispatch_table'

Contributes to KhronosGroup/OpenCL-ICD-Loader#72

Signed-off-by: Kevin Petit <kevin.petit@arm.com>
kpet added a commit to kpet/OpenCL-Headers that referenced this issue Dec 13, 2019
...so that implementations (and others) can use definitions
straight from the headers instead of mirroring them.

The definitions come from https://github.com/KhronosGroup/OpenCL-ICD-Loader
with the following name changes:

- function pointers are now prefixed with 'cl_api_'
- the dispatch table type is now 'cl_icd_dispatch'

Contributes to KhronosGroup/OpenCL-ICD-Loader#72

Signed-off-by: Kevin Petit <kevin.petit@arm.com>
bashbaug pushed a commit to KhronosGroup/OpenCL-Headers that referenced this issue Dec 16, 2019
...so that implementations (and others) can use definitions
straight from the headers instead of mirroring them.

The definitions come from https://github.com/KhronosGroup/OpenCL-ICD-Loader
with the following name changes:

- function pointers are now prefixed with 'cl_api_'
- the dispatch table type is now 'cl_icd_dispatch'

Contributes to KhronosGroup/OpenCL-ICD-Loader#72

Signed-off-by: Kevin Petit <kevin.petit@arm.com>
kpet added a commit to kpet/OpenCL-ICD-Loader that referenced this issue Jan 8, 2020
…eaders

Depends on KhronosGroup/OpenCL-Headers#50.

Fixes KhronosGroup#72.

Signed-off-by: Kevin Petit <kevin.petit@arm.com>
@kpet kpet closed this as completed in #80 Feb 9, 2020
kpet added a commit that referenced this issue Feb 9, 2020
…eaders

Depends on KhronosGroup/OpenCL-Headers#50.

Fixes #72.

Signed-off-by: Kevin Petit <kevin.petit@arm.com>
joekilner pushed a commit to joekilner/OpenCL-ICD-Loader that referenced this issue Jun 20, 2020
…eaders

Depends on KhronosGroup/OpenCL-Headers#50.

Fixes KhronosGroup#72.

Signed-off-by: Kevin Petit <kevin.petit@arm.com>
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 a pull request may close this issue.

3 participants