Skip to content

Conversation

Kerilk
Copy link
Contributor

@Kerilk Kerilk commented Jun 4, 2025

This addresses some aspects of KhronosGroup/OpenCL-Docs#1378
clGetExtensionFunctionForPlatform will always return NULL if called with function_name set as one of the functions defined in the cl_khr_icd extension.
clGetExtensionFunction would not return icd specific symbols (unless the platform extension suffix is "KHR") so I don't think we need to do anything there, bu I could be persuaded otherwise.

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 think this is fine. In theory, an intercept layer could want to intercept calls to these functions, and with this change it would need to jump through some extra hoops to do so. I've never had a need to do this with the OpenCL Intercept Layer, though, so I think the odds of this happening are small.

Noting for completeness, there are still mechanisms to get these functions, if needed for a sufficiently stubborn application, but it'd be a bit more work now.

@Kerilk
Copy link
Contributor Author

Kerilk commented Jun 4, 2025

This will go through the layers, but the return from the platform will always be null (or I did not code the behavior I wanted).
Inside the loader we always use the raw pointer obtained from the library, so layers would not be invoked, but if as intercept library was used as a driver then it would provide it's own version.

How would you gain access to these from an application if these can be obtained neither through clGetExtensionFunctionForPlatform or clGetExtensionFunction? You would need to load the driver's yourself and get the function pointer from clGetExtensionFunction, correct?

@bashbaug
Copy link
Contributor

bashbaug commented Jun 4, 2025

You would need to load the driver's yourself and get the function pointer from clGetExtensionFunction, correct?

Yes, this is one of the mechanisms I was thinking of.

The other would be to somehow extract the dispatch table and call into the ICD's clGetExtensionFunctionAddressForPlatform directly, bypassing the ICD loader's checks. I suppose once you have the dispatch table you could also call into ICD's clGetExtensionFunctionAddress via the dispatch table vs. loading the ICD and getting the function pointer via OS mechanisms.

@karolherbst
Copy link

I think this is fine. In theory, an intercept layer could want to intercept calls to these functions, and with this change it would need to jump through some extra hoops to do so. I've never had a need to do this with the OpenCL Intercept Layer, though, so I think the odds of this happening are small.

Noting for completeness, there are still mechanisms to get these functions, if needed for a sufficiently stubborn application, but it'd be a bit more work now.

I was a bit curious about that aspect. ocl-icd supports printing called CL functions via OCL_ICD_DEBUG=4 and an intercept layer could just print every CL function called. However, one could argue that from an application developers perspective you don't want to see the internal ICD loader <-> ICD calls anyway.

And if the ICD loader wants to use such a layer, then it needs to be wrapped around the ICD. So I'd just see those as two different uses of intercept layer.

The other would be to somehow extract the dispatch table and call into the ICD's clGetExtensionFunctionAddressForPlatform directly, bypassing the ICD loader's checks. I suppose once you have the dispatch table you could also call into ICD's clGetExtensionFunctionAddress via the dispatch table vs. loading the ICD and getting the function pointer via OS mechanisms.

I'd argue, that if an application makes use of the cl_khr_icd extension itself, then it's by definition an ICD loader and can just load the implementations directly instead anyway. The idea came up mostly just to prevent low effort applications to be sneaky and do weird stuff which is hard to debug.

Anyway, personally I don't think it's going to be a huge issue, I was just curious about some details when trying to implement the 2.0.0 version and what to expect from an ICD perspective.

@bashbaug
Copy link
Contributor

Merging as discussed in the August 19th teleconference.

@bashbaug bashbaug merged commit 02134b0 into KhronosGroup:main Aug 19, 2025
33 checks passed
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.

3 participants