Skip to content

[SYCL] Support the cases when accessor is wrapped to some class #82

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 1 commit into from
Apr 20, 2019
Merged

[SYCL] Support the cases when accessor is wrapped to some class #82

merged 1 commit into from
Apr 20, 2019

Conversation

againull
Copy link
Contributor

Current implementation supports only the cases when captured accessor
is a top level object. But accessors could be wrapped to some classes.
Frontend should properly handle this case and intialize accessors using
appropriate kernel parameters.

Signed-off-by: Artur Gainullin artur.gainullin@intel.com

romanovvlad
romanovvlad previously approved these changes Apr 16, 2019
View reviewed changes
bader
bader previously approved these changes Apr 16, 2019
View reviewed changes
@againull
Copy link
Contributor Author

Sorry, looks like I found some possible problem in my patch. Will reopen once resolved.

@againull againull closed this Apr 16, 2019
@againull againull reopened this Apr 19, 2019
@againull againull dismissed stale reviews from bader and romanovvlad via b4292a3 April 19, 2019 16:03
@againull
[SYCL] Support the cases when accessor is wrapped to some class
1377dc6 
Current implementation supports only the cases when captured accessor
is a top level object. But accessors could be wrapped to some classes.
Frontend should properly handle this case and intialize accessors using
appropriate kernel parameters.

Signed-off-by: Artur Gainullin <artur.gainullin@intel.com>
@romanovvlad romanovvlad merged commit 223adac into intel:sycl Apr 20, 2019
keryell
keryell reviewed Apr 20, 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.

It looks like my review has been forgotten in the pending limbo for a few days... :-(
I submit it now, even if it is too late.

clang/lib/Sema/SemaSYCL.cpp
Expr *Base) {
for (auto *WrapperFld : CRD->fields()) {
QualType FldType = WrapperFld->getType();
CXXRecordDecl *WrapperFldCRD = FldType->getAsCXXRecordDecl();
Copy link
Contributor

Choose a reason for hiding this comment

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

auto instead of CXXRecordDecl since the type is obvious?

clang/lib/Sema/SemaSYCL.cpp
} else {
// Field is a structure or class so change the wrapper object
// and recursively search for accessor field.
DeclAccessPair WrapperFieldDAP =
Copy link
Contributor

Choose a reason for hiding this comment

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

auto instead of DeclAccessPair since the type is obvious?

clang/lib/Sema/SemaSYCL.cpp
};

QualType FieldType = Field->getType();
CXXRecordDecl *CRD = FieldType->getAsCXXRecordDecl();
Copy link
Contributor

Choose a reason for hiding this comment

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

auto instead of CXXRecordDecl since the type is obvious?

clang/test/CodeGenSYCL/wrapped-accessor.cpp
auto acc_wrapped = AccWrapper<decltype(acc)>{acc};
kernel<class wrapped_access>(
[=]() {
acc_wrapped.accessor.use();
Copy link
Contributor

Choose a reason for hiding this comment

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

While I appreciate this feature, I have the feeling it is a kind of magical SYCL extension...
To discuss inside the SYCL committee...

sycl/test/basic_tests/accessor/accessor.cpp

template <typename Acc> struct Wrapper2 {
Wrapper1 w1;
AccWrapper<Acc> wrapped;
Copy link
Contributor

Choose a reason for hiding this comment

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

I have the feeling that means something like "capture of non-static storage is allowed by copy if the object is trivially-copiable, standard layout, or a combination with accessors."

sycl/test/basic_tests/accessor/accessor.cpp
cgh.parallel_for<class wrapped_access3>(
sycl::range<1>(buf.get_count()), [=](sycl::item<1> it) {
auto idx = it.get_linear_id();
wr3.w2.wrapped.accessor[idx] = 333;
Copy link
Contributor

Choose a reason for hiding this comment

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

Ouch! :-)

clang/lib/Sema/SemaSYCL.cpp
@@ -515,11 +515,52 @@ CreateSYCLKernelBody(Sema &S, FunctionDecl *KernelCallerFunc, DeclContext *DC) {
S, ((*ParamItr++))->getOriginalType(), ParamDREs[2]));
ParamStmts.push_back(getExprForRangeOrOffset(
S, ((*ParamItr++))->getOriginalType(), ParamDREs[3]));
// kernel_obj.accessor.__init(_ValueType*, range<int>, range<int>,
// id<int>)
// [kenrel_obj or wrapper object].accessor.__init(_ValueType*,
Copy link
Contributor

Choose a reason for hiding this comment

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

kenrel_obj ?

@againull againull deleted the accessor-wrapper branch December 3, 2022 00:01
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Feb 23, 2023
@vladimirlaz
[SYCL] Fix tests (intel#82)
e0de9af 
* remove duplicated files;
 * mark test unsupported if it requires level_zero headers and the
   headers are not availble.
iclsrc pushed a commit that referenced this pull request Sep 21, 2023
Lazy deference underlying object for shared/weak/unique_ptr synthetic…
9faf8b5 
…… (#67069)

We noticed some performance issue while in lldb-vscode for grabing the
name of the SBValue. Profiling shows SBValue::GetName() can cause
synthetic children provider of shared/unique_ptr to deference underlying
object and complete it type.

This patch lazily moves the dereference from synthetic child provider's
Update() method to GetChildAtIndex() so that SBValue::GetName() won't
trigger the slow code path.

Here is the culprit slow code path:
```
...
frame #59: 0x00007ff4102e0660 liblldb.so.15`SymbolFileDWARF::CompleteType(this=<unavailable>, compiler_type=0x00007ffdd9829450) at SymbolFileDWARF.cpp:1567:25 [opt]
...
frame #67: 0x00007ff40fdf9bd4 liblldb.so.15`lldb_private::ValueObject::Dereference(this=0x0000022bb5dfe980, error=0x00007ffdd9829970) at ValueObject.cpp:2672:41 [opt]
frame #68: 0x00007ff41011bb0a liblldb.so.15`(anonymous namespace)::LibStdcppSharedPtrSyntheticFrontEnd::Update(this=0x000002298fb94380) at LibStdcpp.cpp:403:40 [opt]
frame #69: 0x00007ff41011af9a liblldb.so.15`lldb_private::formatters::LibStdcppSharedPtrSyntheticFrontEndCreator(lldb_private::CXXSyntheticChildren*, std::shared_ptr<lldb_private::ValueObject>) [inlined] (anonymous namespace)::LibStdcppSharedPtrSyntheticFrontEnd::LibStdcppSharedPtrSyntheticFrontEnd(this=0x000002298fb94380, valobj_sp=<unavailable>) at LibStdcpp.cpp:371:5 [opt]
...
frame #78: 0x00007ff40fdf6e42 liblldb.so.15`lldb_private::ValueObject::CalculateSyntheticValue(this=0x000002296c66a500) at ValueObject.cpp:1836:27 [opt]
frame #79: 0x00007ff40fdf1939 liblldb.so.15`lldb_private::ValueObject::GetSyntheticValue(this=<unavailable>) at ValueObject.cpp:1867:3 [opt]
frame #80: 0x00007ff40fc89008 liblldb.so.15`ValueImpl::GetSP(this=0x0000022c71b90de0, stop_locker=0x00007ffdd9829d00, lock=0x00007ffdd9829d08, error=0x00007ffdd9829d18) at SBValue.cpp:141:46 [opt]
frame #81: 0x00007ff40fc7d82a liblldb.so.15`lldb::SBValue::GetSP(ValueLocker&) const [inlined] ValueLocker::GetLockedSP(this=0x00007ffdd9829d00, in_value=<unavailable>) at SBValue.cpp:208:21 [opt]
frame #82: 0x00007ff40fc7d817 liblldb.so.15`lldb::SBValue::GetSP(this=0x00007ffdd9829d90, locker=0x00007ffdd9829d00) const at SBValue.cpp:1047:17 [opt]
frame #83: 0x00007ff40fc7da6f liblldb.so.15`lldb::SBValue::GetName(this=0x00007ffdd9829d90) at SBValue.cpp:294:32 [opt]
...
```

Differential Revision: https://reviews.llvm.org/D159542
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keryell keryell keryell left review comments

@romanovvlad romanovvlad romanovvlad approved these changes

@vladimirlaz vladimirlaz vladimirlaz approved these changes

@bader bader bader left review comments

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

Successfully merging this pull request may close these issues.
5 participants
@againull @bader @keryell @romanovvlad @vladimirlaz