Skip to content
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

[SYCL] Address multi_ptr in spec vs intel/llvm mismatches #439

Merged
merged 10 commits into from
Jul 20, 2023

Conversation

mmoadeli
Copy link
Contributor

@mmoadeli mmoadeli commented Jul 11, 2023

  • Remove the 'explicit' keyword from the multi_ptr::operator pointer() function.
  • Include a generic space constraint in the multi_ptr::ctor from accessor and accessor(local) to support generic_space.
  • Implement the multi_ptr::ctor from local_accessor, including support for generic_space.
  • Enable implicit conversion of multi_ptr from accessors with const types.
  • Add deduction guides for the deprecated access::target::constant_buffer and access::target::local.
  • Implement support for the constant_space specialization of non-legacy multi_ptr.

* Remove explicit from multi_ptr<VoidType>::operator pointer() function.
* Add generic soace to constraints of multi_ptr<Legacy>::ctor from accessor and accessor(local) with generic_space.
* Add multi_ptr<Legacy>::ctor from local_accessor (including generic_space)
* Implicit conversion of multi_ptr from accessors with const types.
* Sync spec multi_ptr deduction guides with impl.
@mmoadeli mmoadeli changed the title Address multi_ptr spec / intel/llvm mismatches Address multi_ptr spec - intel/llvm mismatches Jul 11, 2023
@mmoadeli mmoadeli changed the title Address multi_ptr spec - intel/llvm mismatches Address multi_ptr in spec vs intel/llvm mismatches Jul 11, 2023
@mmoadeli mmoadeli changed the title Address multi_ptr in spec vs intel/llvm mismatches [SYCL] Address multi_ptr in spec vs intel/llvm mismatches Jul 12, 2023
adoc/headers/multipointer.h Show resolved Hide resolved
adoc/headers/multipointer.h Outdated Show resolved Hide resolved
adoc/headers/multipointerlegacy.h Outdated Show resolved Hide resolved
adoc/headers/multipointerlegacy.h Outdated Show resolved Hide resolved
@gmlueck
Copy link
Contributor

gmlueck commented Jul 13, 2023

One comment also about the description:

Sync spec multi_ptr deduction guides with intel/llvm.

I'd change this part of the description to:

Add deduction guides for the deprecated access::target::constant_buffer and access::target::local.

This makes it more clear about what changed.

adoc/headers/multipointer.h Show resolved Hide resolved
adoc/headers/multipointer.h Outdated Show resolved Hide resolved
- Replace access::target::device with target::device in decuction guides
- Replace  access::decorated::legacy wit access::decorated::no in deduction guide specialised with constant_space.
- Use same comments for legacy and non-legacy multi_ptr ctors.
@mmoadeli
Copy link
Contributor Author

thanks @gmlueck @AerialMantis for comments. They are addressed.

adoc/headers/multipointer.h Outdated Show resolved Hide resolved
adoc/headers/multipointer.h Outdated Show resolved Hide resolved
@mmoadeli mmoadeli requested a review from gmlueck July 17, 2023 21:14
@fraggamuffin
Copy link

LGTM

@keryell keryell merged commit 02f7d6a into KhronosGroup:SYCL-2020/master Jul 20, 2023
1 check passed
keryell added a commit that referenced this pull request Sep 10, 2024
[SYCL] Address multi_ptr in spec vs intel/llvm mismatches

Remove the 'explicit' keyword from the multi_ptr::operator pointer() function.
Include a generic space constraint in the multi_ptr::ctor from accessor and accessor(local) to support generic_space.
Implement the multi_ptr::ctor from local_accessor, including support for generic_space.
Enable implicit conversion of multi_ptr from accessors with const types.
Add deduction guides for the deprecated access::target::constant_buffer and access::target::local.
Implement support for the constant_space specialization of non-legacy multi_ptr.
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.

5 participants