[SYCL][Graph] Fix In-order queue unitests single context bug #249
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changes In-order queue unitests to use the same context for the recorded Queue and the Graph.
EwanC
approved these changes
Jul 6, 2023
There was a problem hiding this comment.
LGTM.
For Julian & Pablos reference this is intended to fail the windows fail found by PR3/4 CI
******************** TEST 'SYCL-Unit :: Extensions/./ExtensionsTests.exe/2/10' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:D:\github\_work\llvm\llvm\build\tools\sycl\unittests\Extensions\.\ExtensionsTests.exe-SYCL-Unit-5916-2-10.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=10 GTEST_SHARD_INDEX=2 D:\github\_work\llvm\llvm\build\tools\sycl\unittests\Extensions\.\ExtensionsTests.exe
--
Script:
--
D:\github\_work\llvm\llvm\build\tools\sycl\unittests\Extensions\.\ExtensionsTests.exe --gtest_filter=CommandGraphTest.InOrderQueue
--
D:\github\_work\llvm\llvm\src\sycl\unittests\Extensions\CommandGraph.cpp(416): error: Expected equality of these values:
InOrderQueue.get_context()
Which is: 16-byte object <50-2A 01-EF 56-02 00-00 40-2A 01-EF 56-02 00-00>
GraphExecImpl->getContext()
Which is: 16-byte object <10-01 01-EF 56-02 00-00 00-01 01-EF 56-02 00-00>
D:\github\_work\llvm\llvm\src\sycl\unittests\Extensions\CommandGraph.cpp:416
Expected equality of these values:
InOrderQueue.get_context()
Which is: 16-byte object <50-2A 01-EF 56-02 00-00 40-2A 01-EF 56-02 00-00>
GraphExecImpl->getContext()
Which is: 16-byte object <10-01 01-EF 56-02 00-00 00-01 01-EF 56-02 00-00>
********************
Testing: 0.. 10.. 20.. 30.. 40.. 50.. 60.. 70
FAIL: SYCL-Unit :: Extensions/./ExtensionsTests.exe/4/10 (353 of 485)
******************** TEST 'SYCL-Unit :: Extensions/./ExtensionsTests.exe/4/10' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:D:\github\_work\llvm\llvm\build\tools\sycl\unittests\Extensions\.\ExtensionsTests.exe-SYCL-Unit-5916-4-10.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=10 GTEST_SHARD_INDEX=4 D:\github\_work\llvm\llvm\build\tools\sycl\unittests\Extensions\.\ExtensionsTests.exe
--
Script:
--
D:\github\_work\llvm\llvm\build\tools\sycl\unittests\Extensions\.\ExtensionsTests.exe --gtest_filter=CommandGraphTest.InOrderQueueWithEmptyFirst
--
D:\github\_work\llvm\llvm\src\sycl\unittests\Extensions\CommandGraph.cpp(521): error: Expected equality of these values:
InOrderQueue.get_context()
Which is: 16-byte object <20-19 84-A4 02-02 00-00 10-19 84-A4 02-02 00-00>
GraphExecImpl->getContext()
Which is: 16-byte object <00-12 84-A4 02-02 00-00 F0-11 84-A4 02-02 00-00>
D:\github\_work\llvm\llvm\src\sycl\unittests\Extensions\CommandGraph.cpp:521
Expected equality of these values:
InOrderQueue.get_context()
Which is: 16-byte object <20-19 84-A4 02-02 00-00 10-19 84-A4 02-02 00-00>
GraphExecImpl->getContext()
Which is: 16-byte object <00-12 84-A4 02-02 00-00 F0-11 84-A4 02-02 00-00>
********************
Testing: 0.. 10.. 20.. 30.. 40.. 50.. 60.. 70
FAIL: SYCL-Unit :: Extensions/./ExtensionsTests.exe/3/10 (354 of 485)
******************** TEST 'SYCL-Unit :: Extensions/./ExtensionsTests.exe/3/10' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:D:\github\_work\llvm\llvm\build\tools\sycl\unittests\Extensions\.\ExtensionsTests.exe-SYCL-Unit-5916-3-10.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=10 GTEST_SHARD_INDEX=3 D:\github\_work\llvm\llvm\build\tools\sycl\unittests\Extensions\.\ExtensionsTests.exe
--
Script:
--
D:\github\_work\llvm\llvm\build\tools\sycl\unittests\Extensions\.\ExtensionsTests.exe --gtest_filter=CommandGraphTest.InOrderQueueWithEmpty
--
D:\github\_work\llvm\llvm\src\sycl\unittests\Extensions\CommandGraph.cpp(469): error: Expected equality of these values:
InOrderQueue.get_context()
Which is: 16-byte object <80-28 86-CD 1A-02 00-00 70-28 86-CD 1A-02 00-00>
GraphExecImpl->getContext()
Which is: 16-byte object <B0-1E 86-CD 1A-02 00-00 A0-1E 86-CD 1A-02 00-00>
D:\github\_work\llvm\llvm\src\sycl\unittests\Extensions\CommandGraph.cpp:469
Expected equality of these values:
InOrderQueue.get_context()
Which is: 16-byte object <80-28 86-CD 1A-02 00-00 70-28 86-CD 1A-02 00-00>
GraphExecImpl->getContext()
Which is: 16-byte object <B0-1E 86-CD 1A-02 00-00 A0-1E 86-CD 1A-02 00-00>
********************
Testing: 0.. 10.. 20.. 30.. 40.. 50.. 60.. 70
FAIL: SYCL-Unit :: Extensions/./ExtensionsTests.exe/5/10 (355 of 485)
******************** TEST 'SYCL-Unit :: Extensions/./ExtensionsTests.exe/5/10' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:D:\github\_work\llvm\llvm\build\tools\sycl\unittests\Extensions\.\ExtensionsTests.exe-SYCL-Unit-5916-5-10.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=10 GTEST_SHARD_INDEX=5 D:\github\_work\llvm\llvm\build\tools\sycl\unittests\Extensions\.\ExtensionsTests.exe
--
Script:
--
D:\github\_work\llvm\llvm\build\tools\sycl\unittests\Extensions\.\ExtensionsTests.exe --gtest_filter=CommandGraphTest.InOrderQueueWithEmptyLast
--
D:\github\_work\llvm\llvm\src\sycl\unittests\Extensions\CommandGraph.cpp(573): error: Expected equality of these values:
InOrderQueue.get_context()
Which is: 16-byte object <10-2E CC-53 A7-01 00-00 00-2E CC-53 A7-01 00-00>
GraphExecImpl->getContext()
Which is: 16-byte object <70-15 CC-53 A7-01 00-00 60-15 CC-53 A7-01 00-00>
D:\github\_work\llvm\llvm\src\sycl\unittests\Extensions\CommandGraph.cpp:573
Expected equality of these values:
InOrderQueue.get_context()
Which is: 16-byte object <10-2E CC-53 A7-01 00-00 00-2E CC-53 A7-01 00-00>
GraphExecImpl->getContext()
Which is: 16-byte object <70-15 CC-53 A7-01 00-00 60-15 CC-53 A7-01 00-00>
********************
Testing: 0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
********************
Failed Tests (4):
SYCL-Unit :: Extensions/./ExtensionsTests.exe/CommandGraphTest/InOrderQueue
SYCL-Unit :: Extensions/./ExtensionsTests.exe/CommandGraphTest/InOrderQueueWithEmpty
SYCL-Unit :: Extensions/./ExtensionsTests.exe/CommandGraphTest/InOrderQueueWithEmptyFirst
SYCL-Unit :: Extensions/./ExtensionsTests.exe/CommandGraphTest/InOrderQueueWithEmptyLast
|
My local windows build finished. I could reproduce the CI issue without this patch, and can verify this fixes it. |
Bensuo
approved these changes
Jul 6, 2023
EwanC
added a commit
that referenced
this pull request
Jul 19, 2023
The `RecordReplay/concurrent_graph.cpp` test is not valid as it is intending to check that an exception is thrown when `begin_recording()` is called on a different queue while a graph is already recording. However, this is allowed by the specification, hence why the test was XFAIL. The BeginEndRecording unittest already comprehensively tests this behaviour, so this test can be removed rather than modified for correctness https://github.com/reble/llvm/blob/sycl-graph-develop/sycl/unittests/Extensions/CommandGraph.cpp#L156 This was passing unexpectedly on Windows because we were throwing an exception. I suspect because `Queue2` was getting created with a different context than `Queue`, which we've seen before can happen on Windows #249 closes #268
EwanC
added a commit
that referenced
this pull request
Jul 19, 2023
The `RecordReplay/concurrent_graph.cpp` test is not valid as it is intending to check that an exception is thrown when `begin_recording()` is called on a different queue while a graph is already recording. However, this is allowed by the specification, hence why the test was XFAIL. The BeginEndRecording unittest already comprehensively tests this behaviour, so this test can be removed rather than modified for correctness https://github.com/reble/llvm/blob/sycl-graph-develop/sycl/unittests/Extensions/CommandGraph.cpp#L156 This was passing unexpectedly on Windows because we were throwing an exception. I suspect because `Queue2` was getting created with a different context than `Queue`, which we've seen before can happen on Windows #249 closes #268
EwanC
added a commit
that referenced
this pull request
Jul 19, 2023
The `RecordReplay/concurrent_graph.cpp` test is not valid as it is intending to check that an exception is thrown when `begin_recording()` is called on a different queue while a graph is already recording. However, this is allowed by the specification, hence why the test was XFAIL. The BeginEndRecording unittest already comprehensively tests this behaviour, so this test can be removed rather than modified for correctness https://github.com/reble/llvm/blob/sycl-graph-develop/sycl/unittests/Extensions/CommandGraph.cpp#L156 This was passing unexpectedly on Windows because we were throwing an exception. I suspect because `Queue2` was getting created with a different context than `Queue`, which we've seen before can happen on Windows #249 closes #268
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes In-order queue unitests to use the same context for the recorded Queue and the Graph.