Skip to content
This repository was archived by the owner on Mar 28, 2023. It is now read-only.

[SYCL] Enable level-zero-link-flags and fix kernel_bundle_api #806

Merged
vladimirlaz merged 11 commits into from
Feb 28, 2022
Merged

[SYCL] Enable level-zero-link-flags and fix kernel_bundle_api #806

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
87d2718
[SYCL] Enable level-zero-link-flags.cpp
HabKaffee Feb 7, 2022
e0efa74
Fix test, linked with online link options
HabKaffee Feb 7, 2022
9181b14
fix
HabKaffee Feb 7, 2022
3cdf85a
Merge branch 'intel:intel' into enable_level-zero-link-flags
HabKaffee Feb 8, 2022
28b7904
Fix affected test
HabKaffee Feb 8, 2022
3fd62ad
Merge branch 'intel' into enable_level-zero-link-flags
HabKaffee Feb 9, 2022
3a119ff
Merge branch 'intel' into enable_level-zero-link-flags
HabKaffee Feb 14, 2022
baf9db9
Merge branch 'intel:intel' into enable_level-zero-link-flags
HabKaffee Feb 15, 2022
100b33f
Merge branch 'intel' into enable_level-zero-link-flags
HabKaffee Feb 16, 2022
4a9a4ee
Merge branch 'intel:intel' into enable_level-zero-link-flags
HabKaffee Feb 21, 2022
27f1f68
Merge branch 'intel:intel' into enable_level-zero-link-flags
HabKaffee Feb 22, 2022
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
5 changes: 2 additions & 3 deletions SYCL/Basic/kernel_bundle/kernel_bundle_api.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
// Disable fallback assert here until online-support is fixed.
Copy link

@gmlueck gmlueck Feb 8, 2022

Choose a reason for hiding this comment

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

It might be worth asking @s-kanaev if this comment should be removed. It looks to me like the test still has fallback assert disabled, so maybe the comment should remain until it is enabled.

Copy link
Author

@HabKaffee HabKaffee Feb 8, 2022

Choose a reason for hiding this comment

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

I'll ask about your concern.

Copy link

@s-kanaev s-kanaev Feb 16, 2022

Choose a reason for hiding this comment

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

Fallback assert is now disabled by default. See intel/llvm#4694.
Hence, this comment might be removed along with -DSYCL_DISABLE_FALLBACK_ASSERT=1 in build line.
It'll be responsibility of the one who enables the fallback assert to verify it won't impact other tests.

P.S.: Sorry for late response.

// Use of per-kernel device code split and linking the bundle with all images
// involved leads to multiple definition of AssertHappened structure due each
// device image is statically linked against fallback libdevice.
// RUN: %clangxx -DSYCL_DISABLE_FALLBACK_ASSERT=1 -fsycl -fsycl-device-code-split=per_kernel -fsycl-targets=%sycl_triple %s -o %t.out
// RUN: %clangxx -fsycl -fsycl-device-code-split=per_kernel -fsycl-targets=%sycl_triple %s -o %t.out
Copy link

@gmlueck gmlueck Feb 7, 2022

Choose a reason for hiding this comment

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

I'm curious, how is this related to the bug that was fixed in intel/llvm#5476?

Copy link
Author

@HabKaffee HabKaffee Feb 8, 2022

Choose a reason for hiding this comment

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

This test started failing after I started check intel/llvm#5476 . Moreover, there was a comment line //Disable fallback assert here until online-support is fixed. I can guess, that it is about fixed bug.

Copy link
Author

@HabKaffee HabKaffee Feb 8, 2022

Choose a reason for hiding this comment

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

I resolved the problem. Basic/kernel_bundle/kernel_bundle_api was affected indirectly. Before intel/llvm#5476 it received a nullptr as a link option, but now here is passed an empty string.

// RUN: env SYCL_PI_TRACE=2 %CPU_RUN_PLACEHOLDER %t.out %CPU_CHECK_PLACEHOLDER
//
// -fsycl-device-code-split is not supported for cuda
Expand Down Expand Up @@ -185,7 +184,7 @@ int main() {
// CHECK-NEXT: <unknown> : {{.*}}
// CHECK-NEXT: <unknown> : {{.*}}
// CHECK-NEXT: <unknown> : {{.*}}
// CHECK-NEXT: <nullptr>
// CHECK-NEXT: <const char *>:
// CHECK-NEXT: <unknown> : {{.*}}
// CHECK-NEXT: <unknown> : {{.*}}
// CHECK-NEXT: <nullptr>
Expand Down
9 changes: 0 additions & 9 deletions SYCL/KernelAndProgram/level-zero-link-flags.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
// RUN: %clangxx -fsycl -Xsycl-target-linker=spir64 -foo %s -o %t.out
// RUN: %GPU_RUN_PLACEHOLDER %t.out
// REQUIRES: level_zero
//
// This test is disabled because the runtime does not currently pass linker
// flags from "-Xsycl-target-linker=spir64 <opts>" when the program calls
// "sycl::link()". This seems like a bug. Note that the runtime does pass
// those "<opts>" to the online linker in other cases when it links device
// code.
//
// XFAIL: level_zero
//
//==--- level-zero-link-flags.cpp - Error handling for link flags --==//
//
// The Level Zero backend does not accept any online linker options.
Expand Down