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

Fix typo with "accessor::get_pointer" #549

Merged

Conversation

gmlueck
Copy link
Contributor

@gmlueck gmlueck commented Apr 16, 2024

It appears that we introduced a typo with #432 with the return type of accessor::get_pointer. In the synopsis, we specified it as:

global_ptr<DataT> get_pointer() const noexcept

But in the description, it seems that we made a typo and said:

global_ptr<access::decorated::legacy> get_pointer() const noexcept

I believe these are both wrong, and it should be:

global_ptr<value_type> get_pointer() const noexcept

Using value_type is better than DataT because value_type is const when the accessor is read-only.

It appears that we introduced a typo with KhronosGroup#432 with the return type of
`accessor::get_pointer`.  In the synopsis, we specified it as:

```
global_ptr<DataT> get_pointer() const noexcept
```

But in the description, it seems that we made a typo and said:

```
global_ptr<access::decorated::legacy> get_pointer() const noexcept
```

I believe these are both wrong, and it should be:

```
global_ptr<value_type> get_pointer() const noexcept
```

Using `value_type` is better than `DataT` because `value_type` is const
when the accessor is read-only.
AlexeySachkov pushed a commit to intel/llvm that referenced this pull request Apr 25, 2024
Deprecated a few APIs which are not in SYCL 2020 spec:
- `accessor::get_multi_ptr()` should be available only when `AccessTarget ==
target::device`.
- `marray<DataT, N>::operator++/--`should be available only when
`DataT != bool`.

Updated `multi_ptr::get_pointer()` to use correct type alias in accordance with KhronosGroup/SYCL-Docs#549

---------

Signed-off-by: Hu, Peisen <peisen.hu@intel.com>
Copy link

@greglueck greglueck left a comment

Choose a reason for hiding this comment

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

Checking to see if a non-member can approve a PR.

@AerialMantis AerialMantis added the Agenda To be discussed during a SYCL committee meeting label Jun 5, 2024
@gmlueck gmlueck removed the Agenda To be discussed during a SYCL committee meeting label Jun 12, 2024
Copy link
Member

@keryell keryell left a comment

Choose a reason for hiding this comment

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

Thanks.

@keryell
Copy link
Member

keryell commented Jun 13, 2024

@gmlueck
Copy link
Contributor Author

gmlueck commented Jun 13, 2024

I have the feeling that https://github.com/KhronosGroup/SYCL-Docs/pull/432/files#diff-498c175f1b8454ffa58e5f5d9db2c295f04199f9833a50e48159b9534a4cb496R68 looks weird too.

Actually, that is not wrong because value_type and DataT are always the same for local_accessor. However, I agree that it can be confusing. I tried to clarify this in #566.

@gmlueck gmlueck merged commit 1b8c0c0 into KhronosGroup:SYCL-2020/master Jun 13, 2024
2 checks passed
@gmlueck gmlueck deleted the gmlueck/get-pointer-typo branch June 13, 2024 20:35
keryell pushed a commit that referenced this pull request Sep 10, 2024
Fix typo with "accessor::get_pointer"
gmlueck added a commit that referenced this pull request Nov 7, 2024
Fix typo with "accessor::get_pointer"

(cherry picked from commit 1b8c0c0)
gmlueck added a commit that referenced this pull request Nov 7, 2024
Fix typo with "accessor::get_pointer"

(cherry picked from commit 1b8c0c0)
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