-
Notifications
You must be signed in to change notification settings - Fork 768
[SYCL] Add check for atomic64 support #5430
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
Conversation
Yes, it is called if it is not removed. And I think it should be removed and not called for any backend.
From: denis-kabanov ***@***.***>
Sent: Wednesday, February 2, 2022 1:17 PM
To: intel/llvm ***@***.***>
Cc: Maslov, Sergey V ***@***.***>; Review requested ***@***.***>
Subject: Re: [intel/llvm] [SYCL] Add check for atomic64 support (PR #5430)
@denis-kabanov commented on this pull request.
________________________________
In sycl/plugins/level_zero/pi_level_zero.cpp<#5430 (comment)>:
+ case PI_DEVICE_INFO_ATOMIC_64: {
+ if (Device->ZeDeviceModuleProperties->flags &
+ ZE_DEVICE_MODULE_FLAG_INT64_ATOMICS) {
+ bool result = true;
+ std::memcpy(ParamValue, &result, sizeof(bool));
+ return PI_SUCCESS;
+ }
+ return PI_INVALID_VALUE;
+ }
I don't understand what do you mean by "all backends for cuda"
I mean this specialization is called on any backend if we write device.has(aspect::atomic64).
—
Reply to this email directly, view it on GitHub<#5430 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ALTQIYGLSN6RMSWHM3TDG7LUZF7JFANCNFSM5NGJ4CNQ>.
Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because your review was requested.Message ID: ***@***.******@***.***>>
|
@denis-kabanov, could you please add E2E and/or unit test? |
Tests will be added to llvm-test-suite after merge of this PR. |
@smaslov-intel @againull friendly ping in case it dropped off your radars |
Fail of "SYCL / Linux / L0 GEN9 LLVM Test Suite" is unrelated to this PR. |
Fails of "SYCL / Linux / HIP AMDGPU LLVM Test Suite" look legit. |
Yes, this fail is legit, therefore (while there is no implementation on the hip backend) I created a PR with expecting of failure. |
…c64)' (#861) Expect failure of some tests that have 'has(aspect::atomic64)' because hip backend does not have such check implementation. PR with specialization removing for atomic64 check: intel/llvm#5430 Github issue for implementating such check on hip backend: intel/llvm#5570 **Should be merged at the same time with mentioned [PR](intel/llvm#5430
…c64)' (intel/llvm-test-suite#861) Expect failure of some tests that have 'has(aspect::atomic64)' because hip backend does not have such check implementation. PR with specialization removing for atomic64 check: intel#5430 Github issue for implementating such check on hip backend: intel#5570 **Should be merged at the same time with mentioned [PR](intel#5430
Added check for atomic64 support for OpenCL and L0.