Skip to content

[SYCL] Enhance the sycl device libraries filename check when constructing ll… #2944

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

Merged
merged 6 commits into from
Jan 18, 2021
Merged

[SYCL] Enhance the sycl device libraries filename check when constructing ll… #2944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9956e5d
Enhance the sycl device libraries filename check when constructing ll…
jinge90 Dec 23, 2020
ca73d2e
If all input files to llvm-link are sycl device libraries except the …
jinge90 Dec 23, 2020
62d6126
support clang-cl on Windows
jinge90 Jan 7, 2021
6648c4b
Merge remote-tracking branch 'origin/sycl' into enhance_sycl_deviceli…
jinge90 Jan 12, 2021
3535089
Add comments
jinge90 Jan 12, 2021
afe4210
Add test
jinge90 Jan 12, 2021
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
51 changes: 41 additions & 10 deletions clang/lib/Driver/ToolChains/SYCL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,21 @@ void SYCL::constructLLVMForeachCommand(Compilation &C, const JobAction &JA,
Foreach, ForeachArgs, None));
}

// The list should match pre-built SYCL device library files located in
// compiler package. Once we add or remove any SYCL device library files,
// the list should be updated accordingly.
static llvm::SmallVector<StringRef, 10> SYCLDeviceLibList{
"crt",
"cmath",
"cmath-fp64",
"complex",
"complex-fp64",
"fallback-cassert",
"fallback-cmath",
"fallback-cmath-fp64",
"fallback-complex",
"fallback-complex-fp64"};
Comment on lines +102 to +111
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that if we were to add a new device library, we'd have to put its base name here as well? Or is it that no new library is likely to be added?

Copy link
Contributor Author

@jinge90 jinge90 Jan 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Currently, those device libraries can meet our requirements but we may add new ones in the future and we need to put the basename here as well then.
Thanks very much.

Copy link
Contributor

Choose a reason for hiding this comment

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

The explanatory comment you've added should help, I think. I wonder if, in the future, we could make some sort of a configuration file with the device library base names, parsing that both in the relevant CMakeLists and in the driver code (via #include for compile-time?). I understand this is somewhat a vague approach - for example, where should we put the configuration file so that it could be conveniently accessed from all relevant LLVM subprojects? But just as something to consider for future improvements.


const char *SYCL::Linker::constructLLVMLinkCommand(
Compilation &C, const JobAction &JA, const InputInfo &Output,
const ArgList &Args, StringRef SubArchName, StringRef OutputFilePrefix,
Expand All @@ -116,16 +131,32 @@ const char *SYCL::Linker::constructLLVMLinkCommand(
// an actual object/archive. Take that list and pass those to the linker
// instead of the original object.
if (JA.isDeviceOffloading(Action::OFK_SYCL)) {
auto SYCLDeviceLibIter =
std::find_if(InputFiles.begin(), InputFiles.end(), [](const auto &II) {
StringRef InputFilename =
llvm::sys::path::filename(StringRef(II.getFilename()));
if (InputFilename.startswith("libsycl-") &&
InputFilename.endswith(".o"))
return true;
return false;
});
bool LinkSYCLDeviceLibs = (SYCLDeviceLibIter != InputFiles.end());
auto isSYCLDeviceLib = [&C](const InputInfo &II) {
const ToolChain *HostTC = C.getSingleOffloadToolChain<Action::OFK_Host>();
StringRef LibPostfix = ".o";
if (HostTC->getTriple().isWindowsMSVCEnvironment() &&
C.getDriver().IsCLMode())
LibPostfix = ".obj";
StringRef InputFilename =
llvm::sys::path::filename(StringRef(II.getFilename()));
if (!InputFilename.startswith("libsycl-") ||
!InputFilename.endswith(LibPostfix) || (InputFilename.count('-') < 2))
return false;
size_t PureLibNameLen = InputFilename.find_last_of('-');
// Skip the prefix "libsycl-"
StringRef PureLibName = InputFilename.substr(8, PureLibNameLen - 8);
for (const auto &L : SYCLDeviceLibList) {
if (PureLibName.compare(L) == 0)
return true;
}
return false;
};
size_t InputFileNum = InputFiles.size();
bool LinkSYCLDeviceLibs = (InputFileNum >= 2);
LinkSYCLDeviceLibs = LinkSYCLDeviceLibs && !isSYCLDeviceLib(InputFiles[0]);
for (size_t Idx = 1; Idx < InputFileNum; ++Idx)
LinkSYCLDeviceLibs =
LinkSYCLDeviceLibs && isSYCLDeviceLib(InputFiles[Idx]);
// Go through the Inputs to the link. When a listfile is encountered, we
// know it is an unbundled generated list.
if (LinkSYCLDeviceLibs)
Expand Down
8 changes: 8 additions & 0 deletions clang/test/Driver/sycl-device-lib.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,11 @@
// SYCL_LLVM_LINK_NO_DEVICE_LIB: clang{{.*}} "-cc1" {{.*}} "-fsycl-is-device"
// SYCL_LLVM_LINK_NO_DEVICE_LIB-NOT: llvm-link{{.*}} "-only-needed"
// SYCL_LLVM_LINK_NO_DEVICE_LIB: sycl-post-link{{.*}} "-symbols" "-spec-const=rt" "-o" "{{.*}}.table" "{{.*}}.bc"

/// ###########################################################################
/// test llvm-link behavior for special user input whose filename resembles SYCL device library
// RUN: touch libsycl-crt.o
// RUN: %clangxx -fsycl libsycl-crt.o -### 2>&1 \
// RUN: | FileCheck %s -check-prefix=SYCL_LLVM_LINK_USER_ONLY_NEEDED
// SYCL_LLVM_LINK_USER_ONLY_NEEDED: llvm-link{{.*}} "{{.*}}.o" "-o" "{{.*}}.bc" "--suppress-warnings"
// SYCL_LLVM_LINK_USER_ONLY_NEEDED: llvm-link{{.*}} "-only-needed" "{{.*}}" "-o" "{{.*}}.bc" "--suppress-warnings"