Skip to content

Commit 1774004

Browse files
[SYCL] Fix linking of compressed device images when dependencies are not compressed (#18906)
**Problem** When linking device images, we reject dependencies whose image format does not match the parent image. However, consider the case when parent image is compressed, while dependencies are not (demonstrated in the test case attached to this PR). In this case, we are incorrectly rejecting device images and thus causing `No device image found for external symbol` error. **Solution** If the format of the main and dependent device image differs and one of them is compressed, we decompress them and recheck the format of decompressed device images. One side-effect of this solution is that now we'll have to decompress device images, even if we are not using them. For example, when format of decompressed main and dependent images differs. Unfortunately, there's no way to find format of the compressed device image, without first decompressing it. However, I don't think this will incur a significant overhead as (1) we decompress device image only once and cache it for subsequent use, and (2) we decompress only if the dependent device image has an export symbol that main device image wants (when finding which images to link) and if it is compatible with the device.
1 parent a877430 commit 1774004

File tree

2 files changed

+53
-11
lines changed

2 files changed

+53
-11
lines changed

sycl/source/detail/program_manager/program_manager.cpp

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,15 @@ ProgramManager::collectDeviceImageDeps(const RTDeviceBinaryImage &Img,
687687
return DeviceImagesToLink;
688688
}
689689

690+
static inline void
691+
CheckAndDecompressImage([[maybe_unused]] RTDeviceBinaryImage *Img) {
692+
#ifndef SYCL_RT_ZSTD_NOT_AVAIABLE
693+
if (auto CompImg = dynamic_cast<CompressedRTDeviceBinaryImage *>(Img))
694+
if (CompImg->IsCompressed())
695+
CompImg->Decompress();
696+
#endif
697+
}
698+
690699
std::set<RTDeviceBinaryImage *>
691700
ProgramManager::collectDeviceImageDepsForImportedSymbols(
692701
const RTDeviceBinaryImage &MainImg, const device &Dev,
@@ -713,13 +722,30 @@ ProgramManager::collectDeviceImageDepsForImportedSymbols(
713722
bool Found = false;
714723
for (auto It = Range.first; It != Range.second; ++It) {
715724
RTDeviceBinaryImage *Img = It->second;
716-
if (Img->getFormat() != Format ||
717-
!doesDevSupportDeviceRequirements(Dev, *Img) ||
725+
726+
if (!doesDevSupportDeviceRequirements(Dev, *Img) ||
718727
!compatibleWithDevice(Img, *getSyclObjImpl(Dev).get()))
719728
continue;
729+
730+
// If the image is a special device image, we need to check if it
731+
// should be used for this device.
720732
if (isSpecialDeviceImage(Img) &&
721733
!isSpecialDeviceImageShouldBeUsed(Img, *getSyclObjImpl(Dev).get()))
722734
continue;
735+
736+
// If any of the images is compressed, we need to decompress it
737+
// and then check if the format matches.
738+
if (Format == SYCL_DEVICE_BINARY_TYPE_COMPRESSED_NONE ||
739+
Img->getFormat() == SYCL_DEVICE_BINARY_TYPE_COMPRESSED_NONE) {
740+
auto MainImgPtr = const_cast<RTDeviceBinaryImage *>(&MainImg);
741+
CheckAndDecompressImage(MainImgPtr);
742+
CheckAndDecompressImage(Img);
743+
Format = MainImg.getFormat();
744+
}
745+
// Skip this image if its format differs from the main image.
746+
if (Img->getFormat() != Format)
747+
continue;
748+
723749
DeviceImagesToLink.insert(Img);
724750
Found = true;
725751
for (const sycl_device_binary_property &ISProp :
@@ -836,15 +862,6 @@ setSpecializationConstants(const std::shared_ptr<device_image_impl> &InputImpl,
836862
}
837863
}
838864

839-
static inline void
840-
CheckAndDecompressImage([[maybe_unused]] RTDeviceBinaryImage *Img) {
841-
#ifndef SYCL_RT_ZSTD_NOT_AVAIABLE
842-
if (auto CompImg = dynamic_cast<CompressedRTDeviceBinaryImage *>(Img))
843-
if (CompImg->IsCompressed())
844-
CompImg->Decompress();
845-
#endif
846-
}
847-
848865
// When caching is enabled, the returned UrProgram will already have
849866
// its ref count incremented.
850867
ur_program_handle_t ProgramManager::getBuiltURProgram(
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Test device image linking when using dynamic libraries when one of
2+
// the device image is compressed and the other is not.
3+
4+
// UNSUPPORTED: target-nvidia || target-amd
5+
// UNSUPPORTED-INTENDED: Linking using dynamic libraries is not supported on AMD
6+
// and Nvidia.
7+
// REQUIRES: zstd
8+
9+
// DEFINE: %{dynamic_lib_options} = -fsycl %fPIC %shared_lib -fsycl-allow-device-image-dependencies -I %S/Inputs %if windows %{-DMAKE_DLL %}
10+
// DEFINE: %{dynamic_lib_suffix} = %if windows %{dll%} %else %{so%}
11+
12+
// RUN: %clangxx %{dynamic_lib_options} %S/Inputs/d.cpp -o %T/libdevice_d.%{dynamic_lib_suffix}
13+
// RUN: %clangxx %{dynamic_lib_options} %S/Inputs/c.cpp %if windows %{%T/libdevice_d.lib%} -o %T/libdevice_c.%{dynamic_lib_suffix}
14+
// RUN: %clangxx %{dynamic_lib_options} %S/Inputs/b.cpp %if windows %{%T/libdevice_c.lib%} -o %T/libdevice_b.%{dynamic_lib_suffix}
15+
// RUN: %clangxx %{dynamic_lib_options} %S/Inputs/a.cpp %if windows %{%T/libdevice_b.lib%} -o %T/libdevice_a.%{dynamic_lib_suffix}
16+
17+
// Compressed main executable, while dependencies are not compressed.
18+
19+
// RUN: %clangxx -fsycl --offload-compress %{sycl_target_opts} -fsycl-allow-device-image-dependencies -fsycl-device-code-split=per_kernel %S/Inputs/basic.cpp -o %t.out \
20+
// RUN: %if windows \
21+
// RUN: %{%T/libdevice_a.lib%} \
22+
// RUN: %else \
23+
// RUN: %{-L%T -ldevice_a -ldevice_b -ldevice_c -ldevice_d -Wl,-rpath=%T%}
24+
25+
// RUN: %{run} %t.out

0 commit comments

Comments
 (0)