Skip to content

[SYCL] Common Reference Semantics for accessor class #63

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

Merged
merged 1 commit into from
Apr 5, 2019
Merged

[SYCL] Common Reference Semantics for accessor class #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 15 additions & 35 deletions clang/lib/Sema/SemaSYCL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,21 +650,20 @@ static void buildArgTys(ASTContext &Context, CXXRecordDecl *KernelObj,
CreateAndAddPrmDsc(Fld, PointerType);

FieldDecl *AccessRangeFld =
getFieldDeclByName(RecordDecl, {"__impl", "AccessRange"});
getFieldDeclByName(RecordDecl, {"impl", "AccessRange"});
assert(AccessRangeFld &&
"The accessor must contain the AccessRange from the __impl field");
"The accessor.impl must contain the AccessRange field");
CreateAndAddPrmDsc(AccessRangeFld, AccessRangeFld->getType());

FieldDecl *MemRangeFld =
getFieldDeclByName(RecordDecl, {"__impl", "MemRange"});
getFieldDeclByName(RecordDecl, {"impl", "MemRange"});
assert(MemRangeFld &&
"The accessor must contain the MemRange from the __impl field");
"The accessor.impl must contain the MemRange field");
CreateAndAddPrmDsc(MemRangeFld, MemRangeFld->getType());

FieldDecl *OffsetFld =
getFieldDeclByName(RecordDecl, {"__impl", "Offset"});
assert(OffsetFld &&
"The accessor must contain the Offset from the __impl field");
getFieldDeclByName(RecordDecl, {"impl", "Offset"});
assert(OffsetFld && "The accessor.impl must contain the Offset field");
CreateAndAddPrmDsc(OffsetFld, OffsetFld->getType());
} else if (Util::isSyclStreamType(ArgTy)) {
// the parameter is a SYCL stream object
Expand Down Expand Up @@ -710,37 +709,18 @@ static void populateIntHeader(SYCLIntegrationHeader &H, const StringRef Name,
uint64_t Offset = Layout.getFieldOffset(Fld->getFieldIndex()) / 8;

if (Util::isSyclAccessorType(ArgTy)) {
// The parameter is a SYCL accessor object - split into three
// parameters, so need to generate three descriptors.
// ... first descriptor (translated to pointer kernel parameter):
// The parameter is a SYCL accessor object.
// The Info field of the parameter descriptor for accessor contains
// two template parameters packed into thid integer field:
Copy link
Contributor

@keryell keryell Apr 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I do not understand this sentence.

But great to have all this new documentation. I love reading a lot of comments while reading some code. :-)

By the way @vladimirlaz can you wait a little bit longer before merging the commits? Because of the different timezones we do not have time to just look at the commits before merging...
One more day should be enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree to wait till next working day to let everybody provide comments

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 tried to make the comment better than it was before (it though had some misprint at L714: thid -> this).
It is sad that it causes questions and needs some rephrasing/rewriting.

Would this one be better? (I could update this place sometime in future when fix this file again).

The 2nd argument of the call H.addParamDesc() below defines what is written
to the accessor parameter descriptor and its int field 'Info'.
For the template class accessor<Type, Dimensions, Mode, Target, IsPlaceHolder>
it keeps both 'Target' and 'Dimensions' packed into that int field 'Info' as:
(Target | (Dimensions << 11)).

// - target (e.g. global_buffer, constant_buffer, local);
// - dimension of the accessor.
const auto *AccTy = ArgTy->getAsCXXRecordDecl();
assert(AccTy && "accessor must be of a record type");
const auto *AccTmplTy = cast<ClassTemplateSpecializationDecl>(AccTy);
H.addParamDesc(SYCLIntegrationHeader::kind_accessor,
getAccessTarget(AccTmplTy), Offset);
// ... second descriptor (translated to access range kernel parameter):
FieldDecl *AccessRngFld =
getFieldDeclByName(AccTy, {"__impl", "AccessRange"}, &Offset);
uint64_t Sz =
Ctx.getTypeSizeInChars(AccessRngFld->getType()).getQuantity();
H.addParamDesc(SYCLIntegrationHeader::kind_std_layout,
static_cast<unsigned>(Sz), static_cast<unsigned>(Offset));
// ... third descriptor (translated to mem range kernel parameter):
// Get offset in bytes
Offset = Layout.getFieldOffset(Fld->getFieldIndex()) / 8;
FieldDecl *MemRngFld =
getFieldDeclByName(AccTy, {"__impl", "MemRange"}, &Offset);
Sz = Ctx.getTypeSizeInChars(MemRngFld->getType()).getQuantity();
H.addParamDesc(SYCLIntegrationHeader::kind_std_layout,
static_cast<unsigned>(Sz), static_cast<unsigned>(Offset));
// ... fourth descriptor (translated to id kernel parameter):
// Get offset in bytes
Offset = Layout.getFieldOffset(Fld->getFieldIndex()) / 8;
FieldDecl *OffstFld =
getFieldDeclByName(AccTy, {"__impl", "Offset"}, &Offset);
Sz = Ctx.getTypeSizeInChars(OffstFld->getType()).getQuantity();
H.addParamDesc(SYCLIntegrationHeader::kind_std_layout,
static_cast<unsigned>(Sz), static_cast<unsigned>(Offset));
int Dims = static_cast<int>(
AccTmplTy->getTemplateArgs()[1].getAsIntegral().getExtValue());
int Info = getAccessTarget(AccTmplTy) | (Dims << 11);
H.addParamDesc(SYCLIntegrationHeader::kind_accessor, Info, Offset);
} else if (Util::isSyclStreamType(ArgTy)) {
// the parameter is a SYCL stream object
llvm_unreachable("streams not supported yet");
Expand Down
7 changes: 4 additions & 3 deletions clang/test/CodeGenSYCL/Inputs/sycl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,15 @@ template <typename dataT, int dimensions, access::mode accessmode,
class accessor {

public:
void __init(__global dataT *Ptr, range<dimensions> AccessRange,
range<dimensions> MemRange, id<dimensions> Offset) {}
void use(void) const {}
template <typename... T>
void use(T... args) {}
template <typename... T>
void use(T... args) const {}
_ImplT<dimensions> __impl;
_ImplT<dimensions> impl;
private:
void __init(__global dataT *Ptr, range<dimensions> AccessRange,
range<dimensions> MemRange, id<dimensions> Offset) {}
};

class kernel {};
Expand Down
20 changes: 4 additions & 16 deletions clang/test/CodeGenSYCL/integration_header.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,28 +22,16 @@
// CHECK-NEXT: const kernel_param_desc_t kernel_signatures[] = {
// CHECK-NEXT: //--- _ZTSZ4mainE12first_kernel
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 4, 0 },
// CHECK-NEXT: { kernel_param_kind_t::kind_accessor, 2014, 4 },
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 1, 4 },
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 1, 5 },
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 1, 6 },
// CHECK-NEXT: { kernel_param_kind_t::kind_accessor, 2016, 7 },
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 1, 7 },
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 1, 8 },
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 1, 9 },
// CHECK-NEXT: { kernel_param_kind_t::kind_accessor, 4062, 4 },
// CHECK-NEXT: { kernel_param_kind_t::kind_accessor, 6112, 7 },
// CHECK-EMPTY:
// CHECK-NEXT: //--- _ZTSN16second_namespace13second_kernelIcEE
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 4, 0 },
// CHECK-NEXT: { kernel_param_kind_t::kind_accessor, 2016, 4 },
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 1, 4 },
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 1, 5 },
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 1, 6 },
// CHECK-NEXT: { kernel_param_kind_t::kind_accessor, 6112, 4 },
// CHECK-EMPTY:
// CHECK-NEXT: //--- _ZTS12third_kernelILi1Ei5pointIZ4mainE1XEE
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 4, 0 },
// CHECK-NEXT: { kernel_param_kind_t::kind_accessor, 2016, 4 },
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 1, 4 },
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 1, 5 },
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 1, 6 },
// CHECK-NEXT: { kernel_param_kind_t::kind_accessor, 6112, 4 },
// CHECK-EMPTY:
// CHECK-NEXT: };
//
Expand Down
5 changes: 1 addition & 4 deletions clang/test/CodeGenSYCL/struct_kernel_param.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@

// CHECK: const kernel_param_desc_t kernel_signatures[] = {
// CHECK-NEXT: //--- _ZTSZZ5test0vENK3$_0clERN2cl4sycl7handlerEE8MyKernel
// CHECK-NEXT: { kernel_param_kind_t::kind_accessor, 2014, 0 },
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 1, 0 },
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 1, 1 },
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 1, 2 },
// CHECK-NEXT: { kernel_param_kind_t::kind_accessor, 4062, 0 },
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 24, 4 },
// CHECK-EMPTY:
// CHECK-NEXT:};
Expand Down
4 changes: 2 additions & 2 deletions clang/test/SemaSYCL/Inputs/sycl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ class accessor {
public:
void use(void) const {}
void use(void*) const {}
_ImplT<dimensions> impl;
private:
void __init(__global dataT *Ptr, range<dimensions> AccessRange,
range<dimensions> MemRange, id<dimensions> Offset) {}

_ImplT<dimensions> __impl;
};

} // namespace sycl
Expand Down
Loading