[SYCL] migrate sycl-module-split to llvm-split #14559
Conversation
Removes sycl-module-split tool and replaces all usages with llvm-split tool.
maksimsab
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
| @@ -6,7 +6,7 @@ | |||
| ; RUN: FileCheck %s -input-file=%t_0.sym --check-prefix CHECK-PER-SOURCE-SYM0 | |||
| ; RUN: FileCheck %s -input-file=%t_1.sym --check-prefix CHECK-PER-SOURCE-SYM1 | |||
| ; | |||
| ; RUN: sycl-module-split -split=source -S < %s -o %t1 | |||
| ; RUN: llvm-split -sycl-split=source -S < %s -o %t1 | |||
There was a problem hiding this comment.
sycl-post-link tests should not call sycl-module-split or llvm-split tools. This is very confusing.
@maksimsab, please, file an issue to refactor these tests.
Also, it's not clear why do we test pass functionality twice using different executables. Can we run just one tool to validate split logic?
There was a problem hiding this comment.
Migration of functionality of sycl-post-link to library hasn't been completed yet. For example, ESIMD splitting is going to be moved to the library during this sprint.
During the migration we keep testing of sycl-post-link since it is used in the default pipeline and we keep the testing of the library to be sure that there are no bugs.
In the end of the migration the library will be invoked from clang-linker-wrapper and sycl-post-link will be removed and these tests will move to the right directory. These tests weren't duplicated because it is easy to make a mistake when someone modifies one test and they forget to modify the equivalent duplicated test.
Proposed testing approach with llvm-split is the same as one that was used for AMD in llvm/llvm-project#89245 .
|
Functionality of this change LGTM. However, I think we will need the functions to be accompanied by detailed comments. Specifically, the 'SYCLSplitModule' function. This will be required if/when we try to upstream this to llorg. Thanks much Maksim. |
Removes sycl-module-split tool and replaces all usages with llvm-split tool.