Skip to content

[SYCL] Reset PiFunctionTable in PiMock::~PiMock #6128

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 12, 2022
Merged

[SYCL] Reset PiFunctionTable in PiMock::~PiMock #6128

merged 2 commits into from
May 12, 2022

Conversation

aelovikov-intel
Copy link
Contributor

So that tests wouldn't have unexpected cross-tests influence/dependency.

Re-enable RequiredWGSize.HasRequiredSize as it is fixed by that change.

@aelovikov-intel
[SYCL] Reset PiFunctionTable in PiMock::~PiMock
034d895 
So that tests wouldn't have unexpected cross-tests influence/dependency.

Re-enable RequiredWGSize.HasRequiredSize as it is fixed by that change.
@aelovikov-intel aelovikov-intel requested a review from a team as a code owner May 9, 2022 23:10
@aelovikov-intel aelovikov-intel requested a review from v-klochkov May 9, 2022 23:10
@v-klochkov v-klochkov removed their request for review May 10, 2022 01:45
@againull againull merged commit 256d2cc into intel:sycl May 12, 2022
@cperkinsintel
Copy link
Contributor

cperkinsintel commented May 26, 2022 •
edited
Loading

This PR is causing serious problems for me with the unit tests. I used git bisect and tracked it to this commit.

Presently there are 12 unit tests that fail whenever ninja check-sycl is run. Interestingly though, while their failures are logged in the test run, they are not counted at the end when the final results are reported.

For example, here is one of the 12 errors:

FAIL: SYCL-Unit :: program_manager/./ProgramManagerTests/7/8 (307 of 363)
******************** TEST 'SYCL-Unit :: program_manager/./ProgramManagerTests/7/8' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/export/iusers/cperkins/sycl_workspace/build/tools/sycl/unittests/program_manager/./ProgramManagerTests-SYCL-Unit-1501830-7-8.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=8 GTEST_SHARD_INDEX=7 /export/iusers/cperkins/sycl_workspace/build/tools/sycl/unittests/program_manager/./ProgramManagerTests
--

And there are 11 others like it.

And this is the test result summary:

  Unsupported:  14
  Passed     : 398

Is the summary only covering the lit tests and not the unit tests?

I'm not sure what the best course of action is. Can someone reproduce my work? Should we reverse this commit?

aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request May 26, 2022
@aelovikov-intel
[SYCL] Fix unittest crashes after PR intel#6128
b0dd8c1 
We need to destroy resources when the Ctx object is destructed. Otherwise the
destruction of internal RT objects would be delayed until global context
destruction that happens *after* ~PiMock.
againull pushed a commit that referenced this pull request May 26, 2022
@aelovikov-intel
[SYCL] Fix unittest crashes after PR #6128 (#6206)
7384e43 
We need to destroy resources when the Ctx object is destructed. Otherwise the
destruction of internal RT objects would be delayed until global context
destruction that happens *after* ~PiMock.
@aelovikov-intel
Copy link
Contributor Author

#6206 has been merged in to address the issues.

@aelovikov-intel aelovikov-intel deleted the mock branch November 8, 2022 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@againull againull againull approved these changes

@steffenlarsen steffenlarsen Awaiting requested review from steffenlarsen

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aelovikov-intel @cperkinsintel @againull