[SYCL][clang-linker-wrapper] fix SYCL offload wrapping in dry-mode in clang-linker-wrapper. #17661
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,11 +12,11 @@ | |||||||
| // RUN: touch %t.devicelib.cpp | ||||||||
| // RUN: %clang %t.devicelib.cpp -fsycl -fsycl-targets=spir64-unknown-unknown -c --offload-new-driver -o %t.devicelib.o | ||||||||
| // | ||||||||
| // Check SYCL Offload Wrapper in non-dry-mode since we need to cover every it's function. | ||||||||
|
Contributor
There was a problem hiding this comment. My understanding is that offload wrapping functionality must work in dry-run mode as well.
Contributor
Author
There was a problem hiding this comment.
Yes and I added such testing below in this file. SYCL Offloading contains parts like this: llvm/llvm/lib/Frontend/Offloading/SYCLOffloadWrapper.cpp Lines 502 to 518 in fb58e23 The loop in the function is being covered by test only if properties are non-empty. As you already noticed it is not a good idea to add arbitrary stub values in // Arbitrary values are used for the testing of SYCL Offload Wrapping.
auto Properties = util::PropertySetRegistry();
Properties.add(util::PropertySetRegistry::SYCL_DEVICE_REQUIREMENTS, "key",
util::PropertyValue(uint32_t(0)));
std::vector Modules = {module_split::SplitModule(
*ImageFileOrErr, std::move(Properties), "entry1\nentry2")};In order to cover the whole SYCLOffloadWrapper it leaves me with a choice between:
If I do the testing in |
||||||||
| // | ||||||||
| // RUN: clang-linker-wrapper --print-wrapped-module --host-triple=x86_64-unknown-linux-gnu \ | ||||||||
| // RUN: -sycl-device-libraries=%t.devicelib.o \ | ||||||||
| // RUN: -sycl-post-link-options="-split=auto -symbols -properties" %t.o -o %t.out 2>&1 --linker-path="/usr/bin/ld" | FileCheck %s --check-prefix=CHECK-FULL | ||||||||
|
|
||||||||
| template <typename t, typename Func> | ||||||||
| __attribute__((sycl_kernel)) void kernel(const Func &func) { | ||||||||
|
|
@@ -35,42 +35,75 @@ int main() { | |||||||
|
|
||||||||
| //#endif | ||||||||
|
|
||||||||
| // CHECK-FULL: %_pi_device_binary_property_struct = type { ptr, ptr, i32, i64 } | ||||||||
| // CHECK-FULL-NEXT: %_pi_device_binary_property_set_struct = type { ptr, ptr, ptr } | ||||||||
| // CHECK-FULL-NEXT: %struct.__tgt_offload_entry = type { i64, i16, i16, i32, ptr, ptr, i64, i64, ptr } | ||||||||
| // CHECK-FULL-NEXT: %__sycl.tgt_device_image = type { i16, i8, i8, ptr, ptr, ptr, ptr, ptr, ptr, ptr, ptr, ptr, ptr, ptr } | ||||||||
| // CHECK-FULL-NEXT: %__sycl.tgt_bin_desc = type { i16, i16, ptr, ptr, ptr } | ||||||||
|
|
||||||||
| // CHECK-FULL: @.sycl_offloading.target.0 = internal unnamed_addr constant [7 x i8] c"spir64\00" | ||||||||
| // CHECK-FULL-NEXT: @.sycl_offloading.opts.compile.0 = internal unnamed_addr constant [1 x i8] zeroinitializer | ||||||||
| // CHECK-FULL-NEXT: @.sycl_offloading.opts.link.0 = internal unnamed_addr constant [1 x i8] zeroinitializer | ||||||||
| // CHECK-FULL-NEXT: @prop = internal unnamed_addr constant [17 x i8] c"DeviceLibReqMask\00" | ||||||||
| // CHECK-FULL-NEXT: @__sycl_offload_prop_sets_arr = internal constant [1 x %_pi_device_binary_property_struct] [%_pi_device_binary_property_struct { ptr @prop, ptr null, i32 1, i64 0 }] | ||||||||
| // CHECK-FULL-NEXT: @SYCL_PropSetName = internal unnamed_addr constant [24 x i8] c"SYCL/devicelib req mask\00" | ||||||||
| // CHECK-FULL-NEXT: @prop.1 = internal unnamed_addr constant [8 x i8] c"aspects\00" | ||||||||
| // CHECK-FULL-NEXT: @prop_val = internal unnamed_addr constant [8 x i8] zeroinitializer | ||||||||
| // CHECK-FULL-NEXT: @__sycl_offload_prop_sets_arr.2 = internal constant [1 x %_pi_device_binary_property_struct] [%_pi_device_binary_property_struct { ptr @prop.1, ptr @prop_val, i32 2, i64 8 }] | ||||||||
| // CHECK-FULL-NEXT: @SYCL_PropSetName.3 = internal unnamed_addr constant [25 x i8] c"SYCL/device requirements\00" | ||||||||
| // CHECK-FULL-NEXT: @SYCL_PropSetName.4 = internal unnamed_addr constant [22 x i8] c"SYCL/kernel param opt\00" | ||||||||
| // CHECK-FULL-NEXT: @__sycl_offload_prop_sets_arr.5 = internal constant [3 x %_pi_device_binary_property_set_struct] [%_pi_device_binary_property_set_struct { ptr @SYCL_PropSetName, ptr @__sycl_offload_prop_sets_arr, ptr getelementptr ([1 x %_pi_device_binary_property_struct], ptr @__sycl_offload_prop_sets_arr, i64 0, i64 1) }, %_pi_device_binary_property_set_struct { ptr @SYCL_PropSetName.3, ptr @__sycl_offload_prop_sets_arr.2, ptr getelementptr ([1 x %_pi_device_binary_property_struct], ptr @__sycl_offload_prop_sets_arr.2, i64 0, i64 1) }, %_pi_device_binary_property_set_struct { ptr @SYCL_PropSetName.4, ptr null, ptr null }] | ||||||||
| // CHECK-FULL-NEXT: @.sycl_offloading.0.data = internal unnamed_addr constant [912 x i8] | ||||||||
| // CHECK-FULL-NEXT: @__sycl_offload_entry_name = internal unnamed_addr constant [25 x i8] c"_ZTSZ4mainE11fake_kernel\00" | ||||||||
| // CHECK-FULL-NEXT: @__sycl_offload_entries_arr = internal constant [1 x %struct.__tgt_offload_entry] [%struct.__tgt_offload_entry { i64 0, i16 1, i16 4, i32 0, ptr null, ptr @__sycl_offload_entry_name, i64 0, i64 0, ptr null }] | ||||||||
| // CHECK-FULL-NEXT: @.sycl_offloading.0.info = internal local_unnamed_addr constant [2 x i64] [i64 ptrtoint (ptr @.sycl_offloading.0.data to i64), i64 912], section ".tgtimg", align 16 | ||||||||
| // CHECK-FULL-NEXT: @llvm.used = appending global [1 x ptr] [ptr @.sycl_offloading.0.info], section "llvm.metadata" | ||||||||
| // CHECK-FULL-NEXT: @.sycl_offloading.device_images = internal unnamed_addr constant [1 x %__sycl.tgt_device_image] [%__sycl.tgt_device_image { i16 2, i8 4, i8 0, ptr @.sycl_offloading.target.0, ptr @.sycl_offloading.opts.compile.0, ptr @.sycl_offloading.opts.link.0, ptr null, ptr null, ptr @.sycl_offloading.0.data, ptr getelementptr ([912 x i8], ptr @.sycl_offloading.0.data, i64 0, i64 912), ptr @__sycl_offload_entries_arr, ptr getelementptr ([1 x %struct.__tgt_offload_entry], ptr @__sycl_offload_entries_arr, i64 0, i64 1), ptr @__sycl_offload_prop_sets_arr.5, ptr getelementptr ([3 x %_pi_device_binary_property_set_struct], ptr @__sycl_offload_prop_sets_arr.5, i64 0, i64 3) }] | ||||||||
| // CHECK-FULL-NEXT: @.sycl_offloading.descriptor = internal constant %__sycl.tgt_bin_desc { i16 1, i16 1, ptr @.sycl_offloading.device_images, ptr null, ptr null } | ||||||||
| // CHECK-FULL-NEXT: @llvm.global_ctors = {{.*}} { i32 1, ptr @sycl.descriptor_reg, ptr null }] | ||||||||
| // CHECK-FULL-NEXT: @llvm.global_dtors = {{.*}} { i32 1, ptr @sycl.descriptor_unreg, ptr null }] | ||||||||
|
|
||||||||
| // CHECK-FULL: define internal void @sycl.descriptor_reg() section ".text.startup" { | ||||||||
| // CHECK-FULL-NEXT: entry: | ||||||||
| // CHECK-FULL-NEXT: call void @__sycl_register_lib(ptr @.sycl_offloading.descriptor) | ||||||||
| // CHECK-FULL-NEXT: ret void | ||||||||
| // CHECK-FULL-NEXT: } | ||||||||
|
|
||||||||
| // CHECK-FULL: define internal void @sycl.descriptor_unreg() section ".text.startup" { | ||||||||
| // CHECK-FULL-NEXT: entry: | ||||||||
| // CHECK-FULL-NEXT: call void @__sycl_unregister_lib(ptr @.sycl_offloading.descriptor) | ||||||||
| // CHECK-FULL-NEXT: ret void | ||||||||
| // CHECK-FULL-NEXT: } | ||||||||
|
|
||||||||
|
|
||||||||
| // Run Check SYCL Offload Wrapping in dry-run mode. | ||||||||
| // | ||||||||
| // RUN: clang-linker-wrapper --print-wrapped-module --dry-run --host-triple=x86_64-unknown-linux-gnu \ | ||||||||
| // RUN: -sycl-device-libraries=%t.devicelib.o \ | ||||||||
| // RUN: %t.o -o %t.out 2>&1 --linker-path="/usr/bin/ld" | FileCheck %s --check-prefix=CHECK-DRY | ||||||||
|
|
||||||||
| // CHECK-DRY: %__sycl.tgt_device_image = type { i16, i8, i8, ptr, ptr, ptr, ptr, ptr, ptr, ptr, ptr, ptr, ptr, ptr } | ||||||||
| // CHECK-DRY-NEXT: %__sycl.tgt_bin_desc = type { i16, i16, ptr, ptr, ptr } | ||||||||
|
|
||||||||
| // CHECK-DRY: @.sycl_offloading.target.0 = internal unnamed_addr constant [7 x i8] c"spir64\00" | ||||||||
| // CHECK-DRY-NEXT: @.sycl_offloading.opts.compile.0 = internal unnamed_addr constant [1 x i8] zeroinitializer | ||||||||
| // CHECK-DRY-NEXT: @.sycl_offloading.opts.link.0 = internal unnamed_addr constant [1 x i8] zeroinitializer | ||||||||
| // CHECK-DRY-NEXT: @.sycl_offloading.0.data = internal unnamed_addr constant [0 x i8] zeroinitializer, section "spir64" | ||||||||
| // CHECK-DRY-NEXT: @.sycl_offloading.0.info = internal local_unnamed_addr constant [2 x i64] [i64 ptrtoint (ptr @.sycl_offloading.0.data to i64), i64 0], section ".tgtimg", align 16 | ||||||||
| // CHECK-DRY-NEXT: @llvm.used = appending global [1 x ptr] [ptr @.sycl_offloading.0.info], section "llvm.metadata" | ||||||||
| // CHECK-DRY-NEXT: @.sycl_offloading.device_images = internal unnamed_addr constant [1 x %__sycl.tgt_device_image] [%__sycl.tgt_device_image { i16 2, i8 4, i8 0, ptr @.sycl_offloading.target.0, ptr @.sycl_offloading.opts.compile.0, ptr @.sycl_offloading.opts.link.0, ptr null, ptr null, ptr @.sycl_offloading.0.data, ptr @.sycl_offloading.0.data, ptr null, ptr null, ptr null, ptr null }] | ||||||||
| // CHECK-DRY-NEXT: @.sycl_offloading.descriptor = internal constant %__sycl.tgt_bin_desc { i16 1, i16 1, ptr @.sycl_offloading.device_images, ptr null, ptr null } | ||||||||
| // CHECK-DRY-NEXT: @llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 1, ptr @sycl.descriptor_reg, ptr null }] | ||||||||
| // CHECK-DRY-NEXT: @llvm.global_dtors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 1, ptr @sycl.descriptor_unreg, ptr null }] | ||||||||
|
|
||||||||
| // CHECK-DRY: define internal void @sycl.descriptor_reg() section ".text.startup" { | ||||||||
| // CHECK-DRY-NEXT: entry: | ||||||||
| // CHECK-DRY-NEXT: call void @__sycl_register_lib(ptr @.sycl_offloading.descriptor) | ||||||||
| // CHECK-DRY-NEXT: ret void | ||||||||
| // CHECK-DRY-NEXT: } | ||||||||
|
|
||||||||
| // CHECK-DRY: define internal void @sycl.descriptor_unreg() section ".text.startup" { | ||||||||
| // CHECK-DRY-NEXT: entry: | ||||||||
| // CHECK-DRY-NEXT: call void @__sycl_unregister_lib(ptr @.sycl_offloading.descriptor) | ||||||||
| // CHECK-DRY-NEXT: ret void | ||||||||
| // CHECK-DRY-NEXT: } | ||||||||
|
||||||||
| // CHECK-DRY-NEXT: } | |
| // CHECK-DRY-NEXT: } | |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that
clang-linker-wrapper --dry-runprints only external commands.Why do we print
offload-wrapperwhich not an external command?On the other hand. LLVM-SPIRV is not call in dry-run. Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be fair, the previous line checking that llvm-spirv tool is called, but the comment seems to be misleading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is right. 'offload-wrapper' printing was done that way to increase debuggability and transparency.
It should be adjusted to the initial approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the other hand, we have a complicated option parsing and it is not clear how to test without
verboseanddry-runmodes.llvm/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
Lines 638 to 706 in a8d7f08
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The work done by the tool be split into two areas:
Technically the tool calls host linker, but it's tested the same way as (1). (1) is tested by printing external commands in dry-run mode.
(2) is tested by printing the wrapper added by the tool. There is a special option for that.
Community version of the tool does not do the linking itself. It calls an external linker, we the test checks that external linker is invoked correctly. Assuming that external linker it tested, it's good enough check.
The main problem here is that clang-linker-wrapper is not supported call sycl-post-link directly.
sycl-post-linkshould be invoked byclang-sycl-linker.We can add an option to print the LLVM IR module, which SYCL code produces.
Eventually, we should use dry-run mode to validate that clang-linker-wrapper calls device linker in the SYCL mode.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sycl-post-linkshould be invoked byclang-sycl-linker.This could be tested by inspecting the output of
clang-sycl-linker.Removed the printing of
offload-wrapper.