[SYCL] Deprecate ext::oneapi::sub_group #10263
Conversation
sub_group extension is deprecated in favor of standard SYCL2020 sub_group.
KornevNikita
requested review from
dkhaldi,
YuriPlyakhin,
yubingex007-a11y and
a team
as code owners
| @@ -50,7 +50,7 @@ static const char *LegalSYCLFunctions[] = { | |||
| "^sycl::_V1::multi_ptr<.+>::.+", | |||
| "^sycl::_V1::nd_item<.+>::.+", | |||
| "^sycl::_V1::group<.+>::.+", | |||
| "^sycl::_V1::sub_group<.+>::.+", | |||
| "^sycl::_V1::sub_group::.+", | |||
There was a problem hiding this comment.
Shouldn't line 62 be changed?
"^sycl::_V1::ext::oneapi::sub_group::.+",
btw, did this trigger any tests fails (or found by, say, grepping)?
There was a problem hiding this comment.
btw, did this trigger any tests fails (or found by, say, grepping)?
No, on the contrary - there were lot of ESIMD test failures locally after sub_group migration. So this change fixed these failures (honestly I don't fully know what it does, I did it some kind of randomly).
I'm not sure about the line 62. What change do you think we should do? Something like that?
"^sycl::_V1::ext::oneapi::sub_group",
There was a problem hiding this comment.
btw, did this trigger any tests fails (or found by, say, grepping)?
No, on the contrary - there were lot of ESIMD test failures locally after sub_group migration. So this change fixed these failures (honestly I don't fully know what it does, I did it some kind of randomly).
I'm not sure about the line
62. What change do you think we should do? Something like that?"^sycl::_V1::ext::oneapi::sub_group",
"Deprecation" shouldn't mean "removing", right, so why the tests started failing?
I do not exactly know what these regexes match, but given that the ext::oneapi::sub_group is deprecated my thinking was that line having "^sycl::_V1::ext::oneapi::sub_group::.+", should be modified (if it needs to be modified).
There was a problem hiding this comment.
It turned out we don't need this change, so I restored it - 1821be2.
Probably I added it because originally I deleted ext::oneapi::sub_group class to see if there any dependencies remained and then there were ESIMD test failures.
Is it still an issue?
There was a problem hiding this comment.
Is it still an issue?
Not for this patch, this will need to be changed when ext::oneapi::sub_group class will be deleted.
There was a problem hiding this comment.
Nope, it's still there, so I have to restore this change.
******************** TEST 'SYCL :: ESIMD/regression/globals.cpp' FAILED ********************
Script:
--
: 'RUN: at line 3'; D:/github/_work/llvm/llvm/install/bin/clang++.exe -fsycl -fsycl-targets=spir64 D:\github\_work\llvm\llvm\llvm\sycl\test-e2e\ESIMD\regression\globals.cpp -o D:\github\_work\llvm\llvm\build-e2e\ESIMD\regression\Output\globals.cpp.tmp.out
: 'RUN: at line 4'; env ONEAPI_DEVICE_SELECTOR=ext_oneapi_level_zero:gpu D:\github\_work\llvm\llvm\build-e2e\ESIMD\regression\Output\globals.cpp.tmp.out
--
Exit Code: 1
Command Output (stdout):
--
$ ":" "RUN: at line 3"
$ "D:/github/_work/llvm/llvm/install/bin/clang++.exe" "-fsycl" "-fsycl-targets=spir64" "D:\github\_work\llvm\llvm\llvm\sycl\test-e2e\ESIMD\regression\globals.cpp" "-o" "D:\github\_work\llvm\llvm\build-e2e\ESIMD\regression\Output\globals.cpp.tmp.out"
# command stderr:
error: function 'sycl::_V1::sub_group::get_local_id() const' is not supported in ESIMD context
error: function 'sycl::_V1::sub_group::get_local_range() const' is not supported in ESIMD context
error: function 'sycl::_V1::sub_group::get_max_local_range() const' is not supported in ESIMD context
error: function 'sycl::_V1::sub_group::get_local_id() const' is not supported in ESIMD context
error: function 'sycl::_V1::sub_group::get_local_range() const' is not supported in ESIMD context
error: function 'sycl::_V1::sub_group::get_max_local_range() const' is not supported in ESIMD context
error: function 'sycl::_V1::sub_group::get_local_id() const' is not supported in ESIMD context
error: function 'sycl::_V1::sub_group::get_local_range() const' is not supported in ESIMD context
error: function 'sycl::_V1::sub_group::get_max_local_range() const' is not supported in ESIMD context
9 errors generated.
error: command failed with exit status: 1
There was a problem hiding this comment.
Can you please explain these failures? Tests should not regress due to deprecating a class. Is there another cause?
There was a problem hiding this comment.
I guess tests fail because they now using sycl::sub_group::method name, not sycl::ext::oneapi::sub_group::method_name. I not only deprecated this class, I also replaced all uses of old sub_group by the new one. I think this is the issue.
There was a problem hiding this comment.
And it looks like this "^sycl::_V1::sub_group<.+>::.+", signature isn't correct
This reverts commit 1821be2.
|
Failure is unrelated #10324 |
sub_group extension is deprecated in favor of standard SYCL2020 sub_group.