Skip to content

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!

@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 <[email protected]>

Co-authored-by: Egor Churaev <[email protected]>
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