Skip to content

[SYCL] Added a constructor to local accessor, fixed accessor_impl constructors #85

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 3 commits into from
Apr 19, 2019
Merged

[SYCL] Added a constructor to local accessor, fixed accessor_impl constructors #85

merged 3 commits into from
Apr 19, 2019

Conversation

v-klochkov
Copy link
Contributor

No description provided.

bader
bader requested changes Apr 17, 2019
View reviewed changes
Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

I think it's commit get into this PR by mistake.
Could you remove it, please?

bader
bader previously approved these changes Apr 17, 2019
View reviewed changes
keryell
keryell reviewed Apr 17, 2019
View reviewed changes
Copy link
Contributor

@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.

Perhaps you could remove old C style programming here.

sycl/include/CL/sycl/accessor.hpp Outdated
@@ -723,10 +713,10 @@ class accessor
Dimensions == 0),
buffer<DataT, 1>>::type &bufferRef)
#ifdef __SYCL_DEVICE_ONLY__
: impl((dataT *)detail::getSyclObjImpl(bufferRef)->BufPtr) {
: impl((_ValueType *)detail::getSyclObjImpl(bufferRef)->BufPtr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you are on it, what about removing these old-C-style casts?

sycl/include/CL/sycl/accessor.hpp Outdated
@@ -765,11 +755,10 @@ class accessor
// Pass nullptr as a pointer to mem and use buffers from the ctor
// arguments to avoid the need in adding utility functions for
// dummy/default initialization of range fields.
: impl(nullptr, (handler *)nullptr) {}
: impl((_ValueType *)nullptr) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Idem

sycl/include/CL/sycl/accessor.hpp Outdated
#else // !__SYCL_DEVICE_ONLY__
: impl(std::make_shared<_ImplT>(
(dataT *)detail::getSyclObjImpl(bufferRef)->BufPtr,
&commandGroupHandlerRef)) {
(_ValueType *)detail::getSyclObjImpl(bufferRef)->BufPtr)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Idem

sycl/include/CL/sycl/accessor.hpp Outdated
@@ -804,11 +793,11 @@ class accessor
Dimensions > 0),
buffer<DataT, Dimensions>>::type &bufferRef)
#ifdef __SYCL_DEVICE_ONLY__
: impl((dataT *)detail::getSyclObjImpl(bufferRef)->BufPtr,
: impl((_ValueType *)detail::getSyclObjImpl(bufferRef)->BufPtr,
Copy link
Contributor

Choose a reason for hiding this comment

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

Idem

sycl/include/CL/sycl/accessor.hpp Outdated
bufferRef.MemRange, bufferRef.MemRange) {
#else
: impl(std::make_shared<_ImplT>(
(dataT *)detail::getSyclObjImpl(bufferRef)->BufPtr,
(_ValueType *)detail::getSyclObjImpl(bufferRef)->BufPtr,
Copy link
Contributor

Choose a reason for hiding this comment

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

Idem

sycl/include/CL/sycl/accessor.hpp Outdated
(dataT *)detail::getSyclObjImpl(bufferRef)->BufPtr,
bufferRef.MemRange, bufferRef.MemRange,
&commandGroupHandlerRef)) {
(_ValueType *)detail::getSyclObjImpl(bufferRef)->BufPtr,
Copy link
Contributor

Choose a reason for hiding this comment

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

Idem

sycl/include/CL/sycl/accessor.hpp Outdated
@@ -896,11 +883,11 @@ class accessor
// arguments to avoid the need in adding utility functions for
// dummy/default initialization of range<Dimensions> and
// id<Dimension> fields.
: impl(nullptr, Range, bufferRef.MemRange, Offset) {}
: impl((_ValueType *)nullptr, Range, bufferRef.MemRange, Offset) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Idem

sycl/include/CL/sycl/accessor.hpp Outdated
#else // !__SYCL_DEVICE_ONLY__
: impl(std::make_shared<_ImplT>(
(dataT *)detail::getSyclObjImpl(bufferRef)->BufPtr, Range,
bufferRef.MemRange, Offset)) {
(_ValueType *)detail::getSyclObjImpl(bufferRef)->BufPtr,
Copy link
Contributor

Choose a reason for hiding this comment

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

Idem

sycl/include/CL/sycl/accessor.hpp Outdated
@@ -941,12 +928,11 @@ class accessor
// arguments to avoid the need in adding utility functions for
// dummy/default initialization of range<Dimensions> and
// id<Dimension> fields.
: impl(nullptr, Range, bufferRef.MemRange,
&commandGroupHandlerRef, Offset) {}
: impl((_ValueType *)nullptr, Range, bufferRef.MemRange, Offset) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the cast required?

sycl/include/CL/sycl/accessor.hpp Outdated
#else // !__SYCL_DEVICE_ONLY__
: impl(std::make_shared<_ImplT>(
(dataT *)detail::getSyclObjImpl(bufferRef)->BufPtr, Range,
bufferRef.MemRange, &commandGroupHandlerRef, Offset)) {
(_ValueType *)detail::getSyclObjImpl(bufferRef)->BufPtr,
Copy link
Contributor

Choose a reason for hiding this comment

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

Idem

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed the patch - used reinterpret_cast instead of C-style conversion and removed C-style conversion before nullptr. Thank you.

@bader
Copy link
Contributor

bader commented Apr 18, 2019

@keryell, are you okay with the changes?

keryell
keryell approved these changes Apr 18, 2019
View reviewed changes
Copy link
Contributor

@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.

Thank you!

@bader bader merged commit d3bdea1 into intel:sycl Apr 19, 2019
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Feb 23, 2023
@vladimirlaz @kbobrovs
[SYCL] Add gpu-intel-dg1 feature to LIT infra (intel#85)
e5c0c0c 
That allows to write and run DG1 specific tests.
Also documentation is updated to cover gaps.

Co-authored-by: kbobrovs <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bader bader bader approved these changes

@keryell keryell keryell 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.
3 participants
@v-klochkov @bader @keryell