[SYCL] Fix compilation of joint_matrix_apply_two_matrices.cpp #13242
Conversation
intel@7fb3b20 replaces `#include <sycl/sycl.hpp>` to `#include <sycl/detail/core.hpp>` in common.hpp. core.hpp doesn't include usm.hpp, used in the header file modified by this patch.
|
@intel/sycl-matrix-reviewers, please, review ASAP. The test this patch fixes fails in pre-commit check! |
| @@ -5,6 +5,9 @@ | |||
| // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | |||
| // | |||
| //===----------------------------------------------------------------------===// | |||
| #include <sycl/detail/core.hpp> | |||
| #include <sycl/usm.hpp> | |||
There was a problem hiding this comment.
Matrix/joint_matrix_apply_two_matrices.cpp includes common.hpp that already has these definitions:
#include <sycl/detail/core.hpp>
#include <sycl/ext/oneapi/matrix/matrix.hpp>
I thought core.hpp includes usm.hpp
If not, please add `#include <sycl/usm.hpp> in common.hpp instead of this file.
There was a problem hiding this comment.
I don't think it would be a good fix. The whole idea behind using fine-grained includes is not to spend time compiling code that isn't being used in the test. The fact that only a few tests fail without this include means that all the other tests don't need it and shouldn't spend time processing code in them.
There was a problem hiding this comment.
I agree, usm.hpp include can be left in the test.
So you just need to remove the core.hpp include.
There was a problem hiding this comment.
I'll remove core.hpp from this header, but it must include core.hpp as it uses "core" SYCL API as well.
The way matrix tests use common.hpp creates implicit dependency on "common.hpp", which is considered a bad practice. Please, fix this issue in a separate PR.
Co-authored-by: aelovikov-intel <[email protected]>
7fb3b20 replaces
#include <sycl/sycl.hpp>to#include <sycl/detail/core.hpp>in common.hpp. core.hpp doesn't include usm.hpp, used in the header file modified by this patch.