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.

Sign up for GitHub

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

Jump to bottom

[SYCL][Test] Track amount of inter-dependencies between headers #16202

Open
wants to merge 1 commit into
base: sycl
Choose a base branch
from
Open

[SYCL][Test] Track amount of inter-dependencies between headers #16202

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions sycl/test/sycl-headers-are-isolated.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// sycl/sycl.hpp is a very heavy header, because it includes all SYCL feature.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// sycl/sycl.hpp is a very heavy header, because it includes all SYCL feature.
// sycl/sycl.hpp is a very heavy header, because it includes all SYCL features.

//
// In order to be able to split it, we need to make sure that all
// inter-dependencies between headers are resolved. We already have a set of
// tests which make sure that every header can be compiled if included alone,
// but to achieve that, every header can simply include every other header.
//
// Considering that the main reason for sycl.hpp splitting is compilation time
// improvement, it is important that SYCL headers do not include each other
// unnecessary to make sure that when you include a single specific header, you
// only get specific functionality and nothing extra.
//
// Note that this test ignores "ext" subfolder for now and concentrates on
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Note that this test ignores "ext" subfolder for now and concentrates on
// NOTE: This test ignores "ext" subfolder for now and concentrates on

Following FIXME: or TODO: notation, I find it helpful to use NOTE: to highlight the importance of this section and simplify the search.

// core SYCL functionality for now.
//
// REQUIRES: linux
//
// RUN: grep -rl "#include <sycl/sub_group.hpp>" %sycl_include/sycl \
// RUN: --exclude-dir=*ext* | FileCheck %s --check-prefix=SUB-GROUP
// SUB-GROUP: sycl/sycl.hpp
// SUB-GROUP-EMPTY:
//
// RUN: grep -rl "#include <sycl/stream.hpp>" %sycl_include/sycl \
// RUN: --exclude-dir=*ext* | FileCheck %s --check-prefix=STREAM
// STREAM: sycl/sycl.hpp
// STREAM-EMPTY:
Comment on lines +18 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

If I get the idea right, we check that #include <sycl/sub_group.hpp> is used only by sycl/sycl.hpp header. I guess this applies to any header declaring SYCL core class, right? If so, do we need to have separate matchers for each header?

Suggested change
// RUN: grep -rl "#include <sycl/sub_group.hpp>" %sycl_include/sycl \
// RUN: --exclude-dir=*ext* | FileCheck %s --check-prefix=SUB-GROUP
// SUB-GROUP: sycl/sycl.hpp
// SUB-GROUP-EMPTY:
//
// RUN: grep -rl "#include <sycl/stream.hpp>" %sycl_include/sycl \
// RUN: --exclude-dir=*ext* | FileCheck %s --check-prefix=STREAM
// STREAM: sycl/sycl.hpp
// STREAM-EMPTY:
// RUN: grep -rl "#include <sycl/sub_group.hpp>" %sycl_include/sycl \
// RUN: --exclude-dir=*ext* | FileCheck %s
// RUN: grep -rl "#include <sycl/stream.hpp>" %sycl_include/sycl \
// RUN: --exclude-dir=*ext* | FileCheck %s
//
// CHECK: sycl/sycl.hpp
// CHECK-EMPTY:

I guess we can further simplify the command to match all include patterns at once rather than searching one include per grep command.


Loading