-
Notifications
You must be signed in to change notification settings - Fork 796
[SYCL][E2E] Add -Werror flag to sycl e2e tests
#14689
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the tests have -Wno-error=.... Can we try and fix any of these warnings? Maybe in separate PRs?
| @@ -1,5 +1,5 @@ | |||
| // REQUIRES: opencl | |||
| // RUN: %{build} -o %t.out | |||
| // RUN: %{build} -std=c++20 -o %t.out | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This warning is generated without it:
# .---command stderr------------
# | /localdisk2/dgarciao/llvm/sycl/test-e2e/Regression/check_vector_of_opencl_event.cpp:21:38: warning: use of function template name with no prior declaration in function call with explicit template arguments is a C++20 extension [-Wc++20-extensions]
# | 21 | std::vector<cl_event> ClEventVec = get_native<sycl::backend::opencl>(event);
# | | ^
# | 1 warning generated.
# | /localdisk2/dgarciao/llvm/sycl/test-e2e/Regression/check_vector_of_opencl_event.cpp:21:38: warning: use of function template name with no prior declaration in function call with explicit template arguments is a C++20 extension [-Wc++20-extensions]
# | 21 | std::vector<cl_event> ClEventVec = get_native<sycl::backend::opencl>(event);
# | | ^
# | 1 warning generated.
# \`-----------------------------
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Folks this is the wrong way to fix this warning. The test is not supposed to use C++20 features.
Please replace get_native<sycl::backend::opencl>(event) to sycl::get_native<sycl::backend::opencl>(event) at line 20.
@maarquitos14 |
It's definitely a good idea. On a side note, just wondering if having tests for deprecated functionalities makes sense or those should be dropped. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tools/esimd LGTM! I agree we should see if we can fix some of the warnings that are not expected as part of the test in a separate PR
Should fix the errors on ARC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM on asan part
|
@intel/unified-runtime-reviewers Friendly ping. |
This patch adds the
-Werrorflag to all SYCL e2e tests to stop the introduction of new warnings.Added
-Wno-error=to existing tests that have warnings (Or made changes to resolve the warnings).