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

OpenCL tests fail gracefully if the OpenCL library can't be loaded #116

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

michel-steuwer
Copy link
Member

This PR makes all tests ineracting with the OpenCL executor fail gracefully if the executor library can not be loaded from disk (e.g. when it is not available on a specific development platform).

This PR guards against all code that failes do to a UnsatisfiedLinkError that is called from within a test function.

Copy link
Member

@Bastacyclop Bastacyclop left a comment

Choose a reason for hiding this comment

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

@michel-steuwer That looks fine for TestsWithExecutor, but we also use util.withExecutor sometimes (e.g. for shine.DPIA.Primitives.Scatter), as well as util.ExecuteOpenCL when using host code generation.

@michel-steuwer
Copy link
Member Author

@michel-steuwer That looks fine for TestsWithExecutor, but we also use util.withExecutor sometimes (e.g. for shine.DPIA.Primitives.Scatter), as well as util.ExecuteOpenCL when using host code generation.

I wasn't aware of this, but I have added support for this now.

@Bastacyclop
Copy link
Member

For util.ExecuteOpenCL maybe we can try running something like clang -lOpenCL trivial.c. If it fails, OpenCL is not installed? Maybe we actually then put the isOpenCLAvailable in a global variable and use that for all three cases? (TestsWithExecutor, util.withExecutor, and util.ExecuteOpenCL)

@umazalakain
Copy link
Contributor

What about exposing this opt out switch to the user? That way we keep things explicit and not engage in guess work.

@michel-steuwer
Copy link
Member Author

What about exposing this opt out switch to the user? That way we keep things explicit and not engage in guess work.

We could have an environment variable to opt-in or opt-out. That would also be good going forward with CUDA-specific tests coming.

@umazalakain, @Bastacyclop what should the defaults be? I.e. do we opt-in into OpenCL and CUDA tests, or opt-out?

@Bastacyclop
Copy link
Member

@umazalakain @michel-steuwer I like automatic support detection, maybe we can detect support by default while exposing override switches to bypass detection?

@Bastacyclop
Copy link
Member

So if clang -lOpenCL test.c works on my machine, OpenCL tests will be enabled by default. But I can still set RISE_NO_OPENCL=1 or whatever variable to disable them.

@umazalakain
Copy link
Contributor

Sorry, somehow I missed these notifications. Automatic support detection + override switch sounds great!

@umazalakain
Copy link
Contributor

Maybe the only risk of having it be automatic is that the user might not be aware of whether they are running or not, mostly because currently the tests output a lot of information (I've been thinking of getting rid of all those print statements left in the tests, they make it more difficult to scroll through the output)

@michel-steuwer
Copy link
Member Author

We should aim for having a configuration with three states:
RISE_OPENCL=ENABLE, RISE_OPENCL=TRY, RISE_OPENCL=DISABLE

We should also make this work for CUDA.

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