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

Conversation

@HabKaffee
Copy link

This patch enabling KernelAndProgram/level-zero-link-flags.cpp due to intel/llvm#5476

@HabKaffee HabKaffee requested a review from a team as a code owner February 7, 2022 08:14
@HabKaffee
Copy link
Author

/verify with intel/llvm#5476

@HabKaffee HabKaffee marked this pull request as draft February 7, 2022 08:31
@HabKaffee
Copy link
Author

/verify intel/llvm#5476

@HabKaffee
Copy link
Author

/verify with intel/llvm#5476

@HabKaffee HabKaffee changed the title [SYCL] Enable level-zero-link-flags.cpp [SYCL] Enable level-zero-link-flags and fix kernel_bundle_api Feb 7, 2022
@HabKaffee
Copy link
Author

/verify with intel/llvm#5476

@alexbatashev alexbatashev requested a review from s-kanaev February 7, 2022 10:18
// 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

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

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

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.

@HabKaffee
Copy link
Author

/verify with intel/llvm#5476

@HabKaffee
Copy link
Author

/verify with intel/llvm#5476

@HabKaffee HabKaffee marked this pull request as ready for review February 8, 2022 11:06
@@ -1,4 +1,3 @@
// Disable fallback assert here until online-support is fixed.
Copy link

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

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.

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.

@HabKaffee
Copy link
Author

/verify with intel/llvm#5476

@HabKaffee
Copy link
Author

/verify with intel/llvm#5476

1 similar comment
@HabKaffee
Copy link
Author

/verify with intel/llvm#5476

@HabKaffee
Copy link
Author

intel/llvm#5476

1 similar comment
@HabKaffee
Copy link
Author

intel/llvm#5476

@HabKaffee
Copy link
Author

/verify with intel/llvm#5476

@HabKaffee
Copy link
Author

/verify with intel/llvm#5476

@HabKaffee
Copy link
Author

@s-kanaev @alexbatashev Could you, please, take a look on this PR?

Copy link

@s-kanaev s-kanaev left a 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.

@HabKaffee
Copy link
Author

/verify with intel/llvm#5476

@vladimirlaz vladimirlaz merged commit 6f29028 into intel:intel Feb 28, 2022
myler pushed a commit to myler/llvm-test-suite that referenced this pull request Apr 12, 2022
myler pushed a commit to myler/llvm-test-suite that referenced this pull request Jun 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants