Skip to content

Conversation

@ahesham-arm
Copy link
Contributor

No description provided.

@ahesham-arm ahesham-arm force-pushed the enable_beta_extensions branch 2 times, most recently from 9555e62 to 3ccdbe0 Compare March 27, 2025 13:00
@bashbaug
Copy link
Contributor

Thanks for the PR! I started taking a look at this yesterday after I saw the CI was failing for #321, but I didn't get very far.

Instead of a cmake option, would it be better to enable beta extensions unconditionally for the tests?

add_definitions(-DCL_TARGET_OPENCL_VERSION=300)
add_definitions(-DCL_EXPERIMENTAL)

My rationale is:

  • I don't think the tests can possibly work without beta extensions due to the way the mock files are created.
  • We don't have cmake options for anything else like this, such as CL_TARGET_OPENCL_VERSION.

I'd be OK adding something like a CL_HPP_ENABLE_BETA_EXTENSIONS in addition, if we choose to. This would be similar to how we have CL_HPP_TARGET_OPENCL_VERSION to set CL_TARGET_OPENCL_VERSION. Or, we could just have clients define CL_ENABLE_BETA_EXTENSIONS directly before including opencl.hpp if they want to enable beta extensions.

@ahesham-arm ahesham-arm force-pushed the enable_beta_extensions branch from 3ccdbe0 to 45b1e4a Compare March 27, 2025 15:13
Signed-off-by: Ahmed Hesham <ahmed.hesham@arm.com>
@ahesham-arm ahesham-arm force-pushed the enable_beta_extensions branch from 45b1e4a to ab12900 Compare March 27, 2025 15:14
@ahesham-arm
Copy link
Contributor Author

Thanks for the PR! I started taking a look at this yesterday after I saw the CI was failing for #321, but I didn't get very far.

Instead of a cmake option, would it be better to enable beta extensions unconditionally for the tests?

Well, looks like we both reached the same conclusion at the same time :)

@bashbaug
Copy link
Contributor

Test-only fix, not waiting to merge.

@bashbaug bashbaug merged commit 9a852f4 into KhronosGroup:main Mar 31, 2025
52 checks passed
@ahesham-arm ahesham-arm deleted the enable_beta_extensions branch March 31, 2025 22:51
@ahesham-arm ahesham-arm mentioned this pull request Mar 31, 2025
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