Skip to content

[SYCL] Add atomic64 aspect decoration to atomic_ref<T *> #14052

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jun 13, 2024
Merged

[SYCL] Add atomic64 aspect decoration to atomic_ref<T *> #14052

merged 4 commits into from
Jun 13, 2024

Conversation

maksimsab
Copy link
Contributor

@maksimsab maksimsab commented Jun 5, 2024 •
edited
Loading

atomic_ref<T *> uses 64-bit atomics and it should be decorated with the corresponding aspect.

fixes: #12743

@maksimsab
[SYCL] Add atomic64 aspect decoration to atomic_ref<T *>
7a48dab 
atomic_ref<T *> uses 64-bit atomics and it should be decorated with the
corresponding aspect.
@maksimsab maksimsab requested a review from a team as a code owner June 5, 2024 13:18
@maksimsab maksimsab requested a review from bso-intel June 5, 2024 13:18
@maksimsab maksimsab requested a review from a team as a code owner June 5, 2024 13:26
Alcpz
Alcpz requested changes Jun 5, 2024
View reviewed changes
Copy link
Contributor

@Alcpz Alcpz left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syclcompat/math_ops.cpp and syclcompat/atomic_comp_exchange.cpp also mention this issue. The changes to atomic_arith.cpp have to be applied there as well.

@maksimsab
Copy link
Contributor Author

@Alcpz Thank you for pointing out. I changed them accordingly.

Alcpz
Alcpz approved these changes Jun 5, 2024
View reviewed changes
Copy link
Contributor

@Alcpz Alcpz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks! 😄

@maksimsab
Copy link
Contributor Author

@intel/llvm-gatekeepers Can we merge it?

@steffenlarsen
Copy link
Contributor

Re-triggering stalled CUDA workflow.

@maksimsab
Copy link
Contributor Author

strange error in lit configuration:

lit.py: /__w/llvm/llvm/llvm/sycl/test-e2e/lit.cfg.py:665: error: Cannot detect device aspect for cuda:gpu

I will try to sync sycl branch.

@maksimsab maksimsab requested review from a team as code owners June 12, 2024 12:02
@maksimsab maksimsab force-pushed the add_atomic64_decoration branch from 130e8bc to 7960729 Compare June 12, 2024 13:47
mdtoguchi
mdtoguchi reviewed Jun 12, 2024
View reviewed changes
Copy link
Contributor

@mdtoguchi mdtoguchi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inadvertent add for driver review? I see nothing from our team that requires approval.

@maksimsab maksimsab removed the request for review from a team June 12, 2024 14:26
@maksimsab
Copy link
Contributor Author

@mdtoguchi Yes, sorry for inconvenience.

@maksimsab
Copy link
Contributor Author

@intel/llvm-gatekeepers Can we merge it?

@dm-vodopyanov dm-vodopyanov merged commit da3b5df into intel:sycl Jun 13, 2024
14 checks passed
@maksimsab maksimsab deleted the add_atomic64_decoration branch June 13, 2024 12:00
ianayl pushed a commit to ianayl/sycl that referenced this pull request Jun 13, 2024
@maksimsab @ianayl
[SYCL] Add atomic64 aspect decoration to atomic_ref<T *> (intel#14052)
bf585a0 
atomic_ref<T *> uses 64-bit atomics and it should be decorated with the
corresponding aspect.

fixes: intel#12743
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mdtoguchi mdtoguchi mdtoguchi left review comments

@Alcpz Alcpz Alcpz approved these changes

@bso-intel bso-intel bso-intel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Device code not split in modules when using aspect::atomic64
6 participants
@maksimsab @steffenlarsen @Alcpz @bso-intel @mdtoguchi @dm-vodopyanov