Skip to content

Conversation

@PatKamin
Copy link
Contributor

@PatKamin PatKamin commented Sep 9, 2025

Filter out banned OpenCl platforms earlier, in the UR OpenCL adapter instead of the SYCL RT.

Filter out banned OpenCl platforms earlier, in the UR OpenCL adapter
instead of the SYCL RT.
@RossBrunton
Copy link
Contributor

Is there a reason this needs to be baked into the binary? Would having it be specified in a config file not mean it could be modified by vendors/sysadmins without having to rebuild DPCPP?

}
}

static bool isBannedOpenCLPlatform(cl_platform_id platform) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also ban opencl:fpga. If device name is not known for fpga, can we filter on the basis of device type? Perhaps by using clGetDeviceInfo (https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_API.html#platform-querying-devices)

Copy link
Contributor

@uditagarwal97 uditagarwal97 left a comment

Choose a reason for hiding this comment

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

We need to ban opencl:fpga as well

@PatKamin
Copy link
Contributor Author

Is there a reason this needs to be baked into the binary? Would having it be specified in a config file not mean it could be modified by vendors/sysadmins without having to rebuild DPCPP?

This is to avoid possible future issues like this one by simplifying SYCL RT logic. This could possibly be made with a config file set with SYCL_CONFIG_FILE_NAME but this could lead to hard to track bugs if this env var is not set or is set with an incomplete list of banned platforms. I think the current approach would be more reliable.

Copy link
Contributor

@uditagarwal97 uditagarwal97 left a comment

Choose a reason for hiding this comment

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

SYCL RT LGTM.

@PatKamin
Copy link
Contributor Author

@intel/llvm-gatekeepers, please consider merging. Jenkins failure seems to be unrelated as it fails on other PRs, too.

@aelovikov-intel aelovikov-intel merged commit 4dfc9b4 into intel:sycl Oct 22, 2025
54 of 56 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.

5 participants