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

[CMake][OpenCL] Remove warning for OpenCL wrapper #13683

Merged
merged 3 commits into from
Jan 10, 2023

Conversation

Lunderberg
Copy link
Contributor

Previously, setting set(USE_OPENCL ON) would result in a warning, stating that the runtime wrapper for OpenCL would be used. Since this is the desired behavior when OpenCL support is enabled, and is not something that a user should fix, this commit removes the warning.

Previously, setting `set(USE_OPENCL ON)` would result in a warning,
stating that the runtime wrapper for OpenCL would be used.  Since this
is the desired behavior when OpenCL support is enabled, and is not
something that a user should fix, this commit removes the warning.
@Lunderberg Lunderberg requested a review from echuraev January 3, 2023 16:34
@tvm-bot
Copy link
Collaborator

tvm-bot commented Jan 3, 2023

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

1 similar comment
@tvm-bot
Copy link
Collaborator

tvm-bot commented Jan 3, 2023

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

@Lunderberg
Copy link
Contributor Author

@tvm-bot rerun

Copy link
Contributor

@echuraev echuraev left a comment

Choose a reason for hiding this comment

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

I have added this warning message especially to notify the user that the OpenCL wrapper will be used, and it might lead to a runtime error if the OpenCL library doesn't exist on a target device.
If warning message might embarrass, let's keep just status message?

@Lunderberg
Copy link
Contributor Author

I have added this warning message especially to notify the user that the OpenCL wrapper will be used, and it might lead to a runtime error if the OpenCL library doesn't exist on a target device.

Good point, as that is a runtime error that would occur on first usage, whereas all the other library dependencies would cause a failure when importing libtvm_runtime.so.

If warning message might embarrass, let's keep just status message?

I like this idea, and have the message back in, but with message(STATUS ...) instead of message(WARNING ...). I also adjusted the wording of the message to make it clear what the wrapper does (runtime search for library) and not just that a wrapper exists. Could you check that the updated wording is accurate?

Copy link
Contributor

@echuraev echuraev left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

cmake/modules/OpenCL.cmake Outdated Show resolved Hide resolved
Co-authored-by: Egor Churaev <egor.churaev@gmail.com>
@masahi masahi merged commit d979949 into apache:main Jan 10, 2023
@Lunderberg Lunderberg deleted the disable_opencl_wrapper_warning branch January 11, 2023 19:34
fzi-peccia pushed a commit to fzi-peccia/tvm that referenced this pull request Mar 27, 2023
* [CMake][OpenCL] Remove warning for OpenCL wrapper

Previously, setting `set(USE_OPENCL ON)` would result in a warning,
stating that the runtime wrapper for OpenCL would be used.  Since this
is the desired behavior when OpenCL support is enabled, and is not
something that a user should fix, this commit removes the warning.

* Added "STATUS" message instead of "WARNING", with more detail

* "installation location" -> "library location"

Co-authored-by: Egor Churaev <egor.churaev@gmail.com>

Co-authored-by: Egor Churaev <egor.churaev@gmail.com>
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.

4 participants