-
Notifications
You must be signed in to change notification settings - Fork 738
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
[SYCL] Throw an exception when usm_shared_allocations aspect not supported #12520
Changes from all commits
bee45c3
d11d72a
4045d77
8c9d3d2
3451e03
d48af00
b04b160
eb25955
1091fe9
1c2f31a
3303a96
ffcac5f
2326311
5e41f65
0d50453
d0f3828
aa87774
803fb59
cd07943
b3ad06d
310132b
bd8d54e
eae932b
b0d3bdc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
// REQUIRES: usm_shared_allocations | ||
// RUN: %{build} -o %t.out | ||
// RUN: %{run} %t.out | ||
// | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
// REQUIRES: usm_shared_allocations | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changes like this effectively mean that we are potentially reducing validation coverage for a feature, simply because another feature doesn't work. That doesn't look right, especially considering that using USM for kernel-device communication is not the only way and using regular buffer + accessor is actually guaranteed to always work. I think that instead of doing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Your comment about rewriting usm shared allocations with buffers convinced me that my PR is probably not the best way to move forward long term. That said though, this now means this JIRA and the PR will have to be blocked until all those tests are rewritten. I personally think since there are many such tests that it is better to just create JIRAs for these tests by grouping tests in the same directory together and we can probably split these JIRAs among several people to speed it up or if its not time sensitive I can just take them all in my backlog and start working one by one. |
||
// RUN: %{build} -o %t.out | ||
// RUN: %{run} %t.out | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
// REQUIRES: usm_shared_allocations | ||
// RUN: %{build} -o %t.out | ||
// RUN: %{run} %t.out | ||
// | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
// REQUIRES: usm_shared_allocations | ||
// RUN: %{build} -o %t.out | ||
// RUN: %{run} %t.out | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
// REQUIRES: usm_shared_allocations | ||
// RUN: %{build} -o %t.out | ||
// RUN: %{run} %t.out | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
// REQUIRES: usm_shared_allocations | ||
// RUN: %{build} -o %t.out | ||
// RUN: %{run} %t.out | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
// REQUIRES: usm_shared_allocations | ||
// RUN: %{build} -o %t.out | ||
// RUN: %{run} %t.out | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
// REQUIRES: usm_shared_allocations | ||
// RUN: %{build} -o %t.out | ||
// RUN: %{run} %t.out | ||
// | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
// REQUIRES: usm_shared_allocations | ||
// RUN: %{build} -o %t.out | ||
// RUN: %{run} %t.out | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
// REQUIRES: usm_shared_allocations | ||
// RUN: %{build} -o %t.out | ||
// RUN: %{run} %t.out | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
// REQUIRES: usm_shared_allocations | ||
// REQUIRES: aspect-atomic64 | ||
// RUN: %{build} -o %t.out | ||
// | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be
aspect-usm_shared_allocations
?llvm/sycl/test-e2e/lit.cfg.py
Lines 665 to 668 in d3d6e78
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I noticed that most tests were using aspect-{ASPECT_NAME} format but it seems that this format works as well. Indeed, if i remove it the tests go back to failing. Perhaps @againull can confirm that both work, that is, using aspect-usm_shared_allocations or just using usm_shared_allocations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it works, most likely it will never be run. One can achieve the same by
REQUIRES: dont-ever-run-me
.