-
Notifications
You must be signed in to change notification settings - Fork 788
[SYCL] Fix LastDeviceIds assignment for Platform w/o device #5695
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
/summary:run |
1 similar comment
/summary:run |
Hi @bso-intel could you please review? |
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.
LGTM
Why these llvm-test-suite tests are failing? |
When platform doesn't have device LastDeviceIds[] for the platform will stay 0 that will affect device enumeration for the next platforms. Now we do adjustment of that if no devices for platform was obtained from backend. Testing: E2E test in llvm-test_suite/SYCL/Regression/device_num.cpp when Mesa is installed Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
ca28a05
331aba1
to
ca28a05
Compare
Hi @bader unfortunately, it is not clear right now, can not reproduce it locally. These tests were changed 2 days ago and have corresponding RT change, could be that not both changes were picked up for testing. Did rebase of my commit to verify this assumption. |
Okay. Please, use |
Hi @bso-intel , @alexbatashev, your reviews were dismissed after rebase, could you please take a look again? |
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.
LGTM
When platform has no devices LastDeviceIds[] for the platform will stay 0 that will affect device enumeration for the next platforms.
Now we do adjustment if no devices for platform was obtained from backend.
Testing: issue is found/partly covered by E2E test in llvm-test_suite/SYCL/Regression/device_num.cpp
Signed-off-by: Tikhomirova, Kseniya [email protected]