Skip to content

[SYCL][ESIMD] Fix inline asm test #8994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 10, 2023
Merged

[SYCL][ESIMD] Fix inline asm test #8994

merged 1 commit into from
Apr 10, 2023

Conversation

sarnex
Copy link
Contributor

@sarnex sarnex commented Apr 7, 2023

It passes on linux ocl/l0 and windows ocl, but fails on windows l0 in postcommit.

I rewrote it to use a host accessor when checking results, based on the pi trace logs, we were missing a piEnqueueMemBufferRead

It passes on linux ocl/l0 and windows ocl, but fails on windows l0.

I rewrote it to use a host accessor when checking results,
I think we were missing a barrier.

Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>
@sarnex
Copy link
Contributor Author

sarnex commented Apr 7, 2023

/summary:run

@sarnex sarnex temporarily deployed to aws April 7, 2023 20:32 — with GitHub Actions Inactive
@sarnex
Copy link
Contributor Author

sarnex commented Apr 7, 2023

ESIMD emulator not related, all tests fail in every PR

Gen9 unrelated

Failed Tests (1):
  SYCL :: Basic/memory-consumption.cpp

@sarnex sarnex marked this pull request as ready for review April 7, 2023 21:01
@sarnex sarnex requested a review from a team as a code owner April 7, 2023 21:01
@sarnex sarnex temporarily deployed to aws April 7, 2023 21:09 — with GitHub Actions Inactive
@sarnex
Copy link
Contributor Author

sarnex commented Apr 10, 2023

@intel/llvm-gatekeepers Hi all, can we merge? This fixes a postcommit failure in gen12 windows. CI failures unrelated. Thanks

@againull
Copy link
Contributor

againull commented Apr 10, 2023

Basic/memory-consumption.cpp fail is known: Basic/memory-consumption.cpp
Device not found errors on SYCL Pre Commit / Linux / ESIMD Emu LLVM Test Suite are not related to this PR and issue is reported.

@againull againull merged commit 6c8feff into intel:sycl Apr 10, 2023
@v-klochkov
Copy link
Contributor

The original code of the test looked correct. @sarnex - can you please confirm?
If it was correct, then we a have a bug in implementation and this needs an internal tracker for the issue. Nick, can you create such?

@sarnex
Copy link
Contributor Author

sarnex commented Apr 10, 2023

@v-klochkov I checked the pi trace logs, it was missing a piEnqueueMemBufferRead in the bad case, we were reading memory written in the device from the host without an explicit sync (which using sycl::buffers does), so I think the original test just happened to work on some platforms. I can make a bug tracker for the runtime team to investigate if you thing there's a bug in the runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants