Skip to content

Adapt intercept layer to be used with native OPENCL_LAYERS - #474

Open
bigmat18 wants to merge 2 commits into
intel:mainfrom
bigmat18:enable-native-layer
Open

Adapt intercept layer to be used with native OPENCL_LAYERS#474
bigmat18 wants to merge 2 commits into
intel:mainfrom
bigmat18:enable-native-layer

Conversation

@bigmat18

@bigmat18 bigmat18 commented Aug 10, 2026

Copy link
Copy Markdown

Fixes #473

Description of Changes

This PR adapts the opencl-intercept-layer to function as a native Khronos OpenCL Layer. By enabling support for the OPENCL_LAYERS environment variable, it allows the system's Khronos ICD Loader to load the intercept layer cleanly and officially, eliminating the need to use a drop-in replacement for the system's OpenCL.dll.

Specifically, this change:

  • Implements the required layer entry points (clInitLayer and clGetLayerInfo returning CL_LAYER_API_VERSION_100).
  • Allows the layer to intercept OpenCL API calls through dispatch table redirection without conflicting with other local ICD loaders.
  • Ensures the base data structures and headers remain unmodified.

Testing Done

  • Tested on Windows. Verified successful loading of the intercept layer via the OPENCL_LAYERS environment variable on Windows without utilizing the drop-in shim.
  • Verified that no existing functionality is broken when used as a standard library.

@bashbaug

Copy link
Copy Markdown
Contributor

Thanks for taking a look at this! I'm definitely supportive of getting this working. I have a question about the high-level approach, though. Specifically, do we need to choose whether to use the system library method or the layer method at build time (ENABLE_OPENCL_NATIVE_LAYER) or can we choose between the two methods at runtime, instead?

If we can switch between the two methods at runtime, then I think this has several advantages: there's only one build configuration to maintain, and a user wouldn't need to manage two binaries to switch between the two methods. Here's how I think this might work. The main difference between the layers method and the standard library replacement method is the dispatch table, as you have demonstrated. So, what if:

  1. We allow the dispatch table to be setup as it was previously, unconditionally, via initDispatch. If needed, we can bypass this when a "use the layer method" control is set, but I think it's fine to let this proceed as usual, and then no new control is needed.
  2. We provide a definition of clInitLayer similar to the one in this PR. clInitLayer will do two things: it will replace the previously setup dispatch table with functions in the passed-in target_dispatch, and it will return back the CLIntercept functions in layer_dispatch_ret. This can be done as a member function of the already-created CLIntercept object.

What do you think? Is this workable?

One more specific comment: Please implement both clInitLayerWithProperties and clInitLayer, so the layer will be loaded by newer loaders (see: KhronosGroup/OpenCL-ICD-Loader#250). It's fine if clInitLayer simply calls clInitLayerWithProperties with nullptr properties, for now. Thanks!

@bigmat18

Copy link
Copy Markdown
Author

Thanks for taking a look at this! I'm definitely supportive of getting this working. I have a question about the high-level approach, though. Specifically, do we need to choose whether to use the system library method or the layer method at build time (ENABLE_OPENCL_NATIVE_LAYER) or can we choose between the two methods at runtime, instead?

If we can switch between the two methods at runtime, then I think this has several advantages: there's only one build configuration to maintain, and a user wouldn't need to manage two binaries to switch between the two methods. Here's how I think this might work. The main difference between the layers method and the standard library replacement method is the dispatch table, as you have demonstrated. So, what if:

1. We allow the dispatch table to be setup as it was previously, unconditionally, via `initDispatch`.  If needed, we can bypass this when a "use the layer method" control is set, but I think it's fine to let this proceed as usual, and then no new control is needed.

2. We provide a definition of `clInitLayer` similar to the one in this PR.  `clInitLayer` will do two things: it will replace the previously setup dispatch table with functions in the passed-in `target_dispatch`, and it will return back the CLIntercept functions in `layer_dispatch_ret`.  This can be done as a member function of the already-created CLIntercept object.

What do you think? Is this workable?

Thank you for your interest in my implementation. Regarding this question, I actually think this approach is better. I'll try to implement it and verify that it doesn't introduce any issues. In theory, it should work, but it's better to validate it first.

@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.

This is coming along nicely - thanks! Can you fix the CI build failures? Some suggestions below.

///////////////////////////////////////////////////////////////////////////////
//
#define INIT_NATIVE_LAYER_FUNC(funcname) \
layer_dispatch . funcname = CLIRN(funcname)

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.

Might need to do something similar to CHECK_RETURN_ICD_LOADER_EXTENSION_FUNCTION here to avoid casting warnings. Something like (note: untested):

Suggested change
layer_dispatch . funcname = CLIRN(funcname)
{ \
void** pfunc = (void**)( &layer_dispatch . funcname ); \
*pfunc = CLIRN(funcname); \
}

Comment thread intercept/src/main.cpp
if (param_value_size < sizeof(version))
return CL_INVALID_VALUE;

std::memcpy(param_value, &version, sizeof(version));

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.

Suggest using CLI_MEMCPY here instead of std::memcpy.

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.

Support loading as a native Khronos OpenCL Layer via OPENCL_LAYERS

2 participants