Skip to content

Commit baf0366

Browse files
[SYCL] Drop optimization level when merging properties (#19432)
Image properties are merged when images are linked. This merging currently fails if some of the properties conflict with eachother. One of the properties is the optimization level used when compiling the binaries, which are used when compiling the images. However, since linking uses compiled binaries, the optimization level has no effect on the merged image. As such, this commit drops the property. --------- Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
1 parent 860ebba commit baf0366

File tree

3 files changed

+77
-6
lines changed

3 files changed

+77
-6
lines changed

sycl/source/detail/device_binary_image.cpp

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -280,16 +280,21 @@ naiveMergeBinaryProperties(const std::vector<const RTDeviceBinaryImage *> &Imgs,
280280

281281
// Exclusive property merge logic. If IgnoreDuplicates is false it assumes there
282282
// are no cases where properties have different values and throws otherwise.
283-
template <typename RangeGetterT>
283+
template <typename RangeGetterT, typename DropPropertyT>
284284
static std::unordered_map<std::string_view, const sycl_device_binary_property>
285285
exclusiveMergeBinaryProperties(
286286
const std::vector<const RTDeviceBinaryImage *> &Imgs,
287-
const RangeGetterT &RangeGetter, bool IgnoreDuplicates = false) {
287+
const RangeGetterT &RangeGetter, bool IgnoreDuplicates,
288+
const DropPropertyT &DropProperty) {
288289
std::unordered_map<std::string_view, const sycl_device_binary_property>
289290
MergeMap;
290291
for (const RTDeviceBinaryImage *Img : Imgs) {
291292
const RTDeviceBinaryImage::PropertyRange &Range = RangeGetter(*Img);
292293
for (const sycl_device_binary_property Prop : Range) {
294+
// Skip adding the property if we can drop it.
295+
if (DropProperty(std::string_view{Prop->Name}))
296+
continue;
297+
293298
const auto [It, Inserted] =
294299
MergeMap.try_emplace(std::string_view{Prop->Name}, Prop);
295300
if (IgnoreDuplicates || Inserted)
@@ -308,6 +313,17 @@ exclusiveMergeBinaryProperties(
308313
return MergeMap;
309314
}
310315

316+
template <typename RangeGetterT,
317+
typename DropPropertyT = std::function<bool(std::string_view)>>
318+
static std::unordered_map<std::string_view, const sycl_device_binary_property>
319+
exclusiveMergeBinaryProperties(
320+
const std::vector<const RTDeviceBinaryImage *> &Imgs,
321+
const RangeGetterT &RangeGetter, bool IgnoreDuplicates = false) {
322+
return exclusiveMergeBinaryProperties(
323+
Imgs, RangeGetter, IgnoreDuplicates,
324+
/*DropProperty=*/[](std::string_view) { return false; });
325+
}
326+
311327
// Device requirements needs the ability to produce new properties. The
312328
// information for these are kept in this struct.
313329
struct MergedDeviceRequirements {
@@ -549,10 +565,11 @@ DynRTDeviceBinaryImage::DynRTDeviceBinaryImage(
549565
Imgs,
550566
[](const RTDeviceBinaryImage &Img) { return Img.getImportedSymbols(); },
551567
/*IgnoreDuplicates=*/true);
552-
auto MergedMisc =
553-
exclusiveMergeBinaryProperties(Imgs, [](const RTDeviceBinaryImage &Img) {
554-
return Img.getMiscProperties();
555-
});
568+
auto MergedMisc = exclusiveMergeBinaryProperties(
569+
Imgs,
570+
[](const RTDeviceBinaryImage &Img) { return Img.getMiscProperties(); },
571+
/*IgnoreDuplicates=*/true, /*DropProperty=*/
572+
[](std::string_view PropertyName) { return PropertyName == "optLevel"; });
556573

557574
std::array<const std::unordered_map<std::string_view,
558575
const sycl_device_binary_property> *,
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
//==--------- link_mixed_opt_input.cpp --- SYCLBIN extension tests ---------==//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
// REQUIRES: aspect-usm_shared_allocations
10+
11+
// -- Test for linking two SYCLBIN kernel_bundles with different optimization
12+
// -- levels.
13+
14+
// ptxas currently fails to compile images with unresolved symbols. Disable for
15+
// other targets than SPIR-V until this has been resolved. (CMPLRLLVM-68810)
16+
// Note: %{sycl_target_opts} should be added to the SYCLBIN compilation lines
17+
// once fixed.
18+
// REQUIRES: target-spir
19+
20+
// RUN: %clangxx --offload-new-driver -fsyclbin=input -fsycl-allow-device-image-dependencies -O0 %S/Inputs/exporting_function.cpp -o %t.export.syclbin
21+
// RUN: %clangxx --offload-new-driver -fsyclbin=input -fsycl-allow-device-image-dependencies -O1 %S/Inputs/importing_kernel.cpp -o %t.import.syclbin
22+
// RUN: %{build} -o %t.out
23+
// RUN: %{l0_leak_check} %{run} %t.out %t.export.syclbin %t.import.syclbin
24+
25+
#define SYCLBIN_INPUT_STATE
26+
27+
#include "Inputs/link.hpp"
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
//==--------- link_mixed_opt_input.cpp --- SYCLBIN extension tests ---------==//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
// REQUIRES: aspect-usm_shared_allocations
10+
11+
// -- Test for linking two SYCLBIN kernel_bundle with different optimization
12+
// -- levels.
13+
14+
// ptxas currently fails to compile images with unresolved symbols. Disable for
15+
// other targets than SPIR-V until this has been resolved. (CMPLRLLVM-68810)
16+
// Note: %{sycl_target_opts} should be added to the SYCLBIN compilation lines
17+
// once fixed.
18+
// REQUIRES: target-spir
19+
20+
// RUN: %clangxx --offload-new-driver -fsyclbin=object -fsycl-allow-device-image-dependencies -O0 %S/Inputs/exporting_function.cpp -o %t.export.syclbin
21+
// RUN: %clangxx --offload-new-driver -fsyclbin=object -fsycl-allow-device-image-dependencies -O1 %S/Inputs/importing_kernel.cpp -o %t.import.syclbin
22+
// RUN: %{build} -o %t.out
23+
// RUN: %{l0_leak_check} %{run} %t.out %t.export.syclbin %t.import.syclbin
24+
25+
#define SYCLBIN_OBJECT_STATE
26+
27+
#include "Inputs/link.hpp"

0 commit comments

Comments
 (0)