-
Notifications
You must be signed in to change notification settings - Fork 130
[SYCL] Enable level-zero-link-flags and fix kernel_bundle_api #806
[SYCL] Enable level-zero-link-flags and fix kernel_bundle_api #806
Conversation
/verify with intel/llvm#5476 |
/verify intel/llvm#5476 |
/verify with intel/llvm#5476 |
/verify with intel/llvm#5476 |
// 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 |
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.
I'm curious, how is this related to the bug that was fixed in intel/llvm#5476?
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.
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.
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.
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.
/verify with intel/llvm#5476 |
/verify with intel/llvm#5476 |
@@ -1,4 +1,3 @@ | |||
// Disable fallback assert here until online-support is fixed. |
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.
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.
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.
I'll ask about your concern.
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.
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.
/verify with intel/llvm#5476 |
/verify with intel/llvm#5476 |
1 similar comment
/verify with intel/llvm#5476 |
1 similar comment
/verify with intel/llvm#5476 |
/verify with intel/llvm#5476 |
@s-kanaev @alexbatashev Could you, please, take a look on this PR? |
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 change seems legit.
/verify with intel/llvm#5476 |
This patch enabling KernelAndProgram/level-zero-link-flags.cpp due to intel/llvm#5476