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

[SYCL] Fix tests for assert #332

Merged
merged 19 commits into from
Jul 12, 2021
Merged

Conversation

s-kanaev
Copy link

@s-kanaev s-kanaev commented Jun 28, 2021

Fixes of regressions for intel/llvm#3767

Sergey Kanaev added 3 commits June 28, 2021 22:26
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
@s-kanaev
Copy link
Author

s-kanaev commented Jun 28, 2021

@AlexeySachkov , @Fznamznon , possibly some of the tests are for features which are owned by you, so please, pay extreme attention to changes of those tests in this patch.

@s-kanaev s-kanaev marked this pull request as draft July 1, 2021 18:30
@s-kanaev s-kanaev changed the title [SYCL] Fix tests for assert [WIP] [SYCL] Fix tests for assert Jul 1, 2021
@s-kanaev s-kanaev changed the title [WIP] [SYCL] Fix tests for assert [SYCL] Fix tests for assert Jul 2, 2021
@s-kanaev s-kanaev marked this pull request as ready for review July 2, 2021 13:20
@s-kanaev s-kanaev requested a review from vzakhari as a code owner July 2, 2021 13:20
@s-kanaev s-kanaev requested a review from kbobrovs as a code owner July 2, 2021 13:38
Comment on lines +21 to +22
// FIXME: Change the behavior when proper automation for assert support is
// introduced

Choose a reason for hiding this comment

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

Will it be fixed by intel/llvm#3767 ?

Copy link
Author

Choose a reason for hiding this comment

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

Per offline discussion, agreed to elaborate on the comment in distinct patch.

// RUN: %clangxx -fsycl-device-only -Xclang -fenable-sycl-dae -Xclang -fsycl-int-header=int_header.h %s -c -o device_code.bc -Wno-sycl-strict
// RUN: %clangxx %cxx_std_optionc++17 %include_option int_header.h %debug_option -c %s -o host_code.o %sycl_options -Wno-sycl-strict
// FIXME Disabled fallback assert as it'll require either online linking or
// explicit offline linking step here

Choose a reason for hiding this comment

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

Why not to add explicit offline linking like in previous test?

Copy link
Author

Choose a reason for hiding this comment

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

Per offline discussion, agreed to add offline linking to this test in distinct patch.

Comment on lines +14 to +15
// FIXME Disabled use of devicelib by assert feature until the 2-step build gets
// fixed.

Choose a reason for hiding this comment

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

Will it be fixed by intel/llvm#3767 ?

Copy link
Author

Choose a reason for hiding this comment

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

This one won't be fixed by intel/llvm#3767. There's an issue in driver which isn't connected to intel/llvm#3767. The issue can be encountered when using any devicelib function and compiling the same way it's done in this test. @mdtoguchi is aware of the issue.

@vladimirlaz vladimirlaz merged commit 25298dc into intel:intel Jul 12, 2021
s-kanaev pushed a commit to s-kanaev/llvm-test-suite that referenced this pull request Jul 13, 2021
@s-kanaev s-kanaev deleted the fix-tests-for-assert branch July 13, 2021 06:49
vladimirlaz pushed a commit that referenced this pull request Jul 14, 2021
smaslov-intel pushed a commit to smaslov-intel/llvm-test-suite that referenced this pull request Aug 12, 2021
smaslov-intel pushed a commit to smaslov-intel/llvm-test-suite that referenced this pull request Aug 12, 2021
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
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.