Skip to content

[SYCL][UR] Use static cast instead of ur::cast #14830

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

Merged
merged 3 commits into from
Aug 6, 2024

Conversation

hdelan
Copy link
Contributor

@hdelan hdelan commented Jul 29, 2024

UR cast is using reinterpret cast under the hood, which was failing for an
unsigned long being cast to an unsigned long long. From the C++ spec:

An expression of integral, enumeration, pointer, or pointer-to-member
type can be converted to its own type. The resulting value is the same as
the value of expression.

Since unsigned long is not the same type as unsigned long long, the
reinterpret_cast fails.

Ping @aarongreig

@omarahmed1111
Copy link
Contributor

@hdelan This is also a fix for the tests mentioned in this PR. Could you remove the unsupported check on these tests in this PR. As I will close the other PR as this should be a better fix for it.

@hdelan
Copy link
Contributor Author

hdelan commented Aug 2, 2024

@hdelan This is also a fix for the tests mentioned in this PR. Could you remove the unsupported check on these tests in this PR. As I will close the other PR as this should be a better fix for it.

Have done so. Thanks @omarahmed1111

@hdelan hdelan temporarily deployed to WindowsCILock August 6, 2024 09:52 — with GitHub Actions Inactive
Hugh Delaney added 3 commits August 6, 2024 10:53
UR cast is using reinterpret cast under the hood, which was failing for
an unsigned long being cast to an unsigned long long. This is probably a
compiler bug but this works around the issue for now.
@npmiller
Copy link
Contributor

npmiller commented Aug 6, 2024

ping @intel/llvm-reviewers-runtime @bso-intel this patch is small and important as it fixes the oneMKL build for CUDA and HIP

dm-vodopyanov
dm-vodopyanov previously approved these changes Aug 6, 2024
@dm-vodopyanov dm-vodopyanov changed the title [UR] Use static cast instead of ur::cast [SYCL][UR] Use static cast instead of ur::cast Aug 6, 2024
@dm-vodopyanov dm-vodopyanov dismissed their stale review August 6, 2024 11:46

Dismissing the review

@hdelan
Copy link
Contributor Author

hdelan commented Aug 6, 2024

Thanks @KseniyaTikhomirova . Ping @intel/llvm-gatekeepers this can be merged.

@sarnex
Copy link
Contributor

sarnex commented Aug 6, 2024

Unlikely HIP is related, merging now.

@sarnex sarnex merged commit d6991cd into intel:sycl Aug 6, 2024
14 checks passed
AlexeySachkov pushed a commit to AlexeySachkov/llvm that referenced this pull request Nov 26, 2024
UR cast is using reinterpret cast under the hood, which was failing for
an
unsigned long being cast to an unsigned long long. From the C++ spec:

> An expression of integral, enumeration, pointer, or pointer-to-member 
type can be converted to its own type. The resulting value is the same
as
the value of expression.

Since `unsigned long` is not the same type as `unsigned long long`, the
`reinterpret_cast` fails.

Ping @aarongreig
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants