From 16f1d0c91ce44d757841b6d91eaef06d1f10522b Mon Sep 17 00:00:00 2001 From: Elizabeth Andrews Date: Mon, 8 Jun 2020 05:25:56 -0700 Subject: [PATCH 1/5] First (incomplete) patch decomposing base classes and struct fields. Currently, struct kernel parameters are passed as a whole, along with individual parameters for special SYCL types resulting in special SYCL paramters being passed twice. To avoid this, we now decompose structs, passing all fields as top level parameters. TO DO: in follow up commit: Add condition to decompose only if struct contains special SYCL type Signed-off-by: Elizabeth Andrews --- clang/lib/Sema/SemaSYCL.cpp | 140 ++++++++++++++++++++++++------------ 1 file changed, 96 insertions(+), 44 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index b3ecda585cf3e..3563dd59d5425 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -677,73 +677,83 @@ constructKernelName(Sema &S, FunctionDecl *KernelCallerFunc, // anonymous namespace so these don't get linkage. namespace { -QualType getItemType(const FieldDecl *FD) { return FD->getType(); } -QualType getItemType(const CXXBaseSpecifier &BS) { return BS.getType(); } - // Implements the 'for-each-visitor' pattern. template -static void VisitAccessorWrapper(CXXRecordDecl *Owner, ParentTy &Parent, - CXXRecordDecl *Wrapper, - Handlers &... handlers); - -template -static void VisitAccessorWrapperHelper(CXXRecordDecl *Owner, RangeTy Range, - Handlers &... handlers) { - for (const auto &Item : Range) { - QualType ItemTy = getItemType(Item); - if (Util::isSyclAccessorType(ItemTy)) - (void)std::initializer_list{ - (handlers.handleSyclAccessorType(Item, ItemTy), 0)...}; - else if (ItemTy->isStructureOrClassType()) { - VisitAccessorWrapper(Owner, Item, ItemTy->getAsCXXRecordDecl(), - handlers...); - if (Util::isSyclStreamType(ItemTy)) - (void)std::initializer_list{ - (handlers.handleSyclStreamType(Item, ItemTy), 0)...}; - } +static void VisitRecord(CXXRecordDecl *Owner, ParentTy &Parent, + CXXRecordDecl *Wrapper, Handlers &... handlers); + +template +static void VisitRecordHelper(CXXRecordDecl *Owner, + clang::CXXRecordDecl::base_class_range Range, + Handlers &... handlers) { + for (const auto &Base : Range) { + QualType BaseTy = Base.getType(); + VisitRecord(Owner, Base, BaseTy->getAsCXXRecordDecl(), handlers...); } } +template +static void VisitRecordHelper(CXXRecordDecl *Owner, + clang::RecordDecl::field_range Range, + Handlers &... handlers) { + VisitRecordFields(Owner, handlers...); +} + // Parent contains the FieldDecl or CXXBaseSpecifier that was used to enter // the Wrapper structure that we're currently visiting. Owner is the parent type // (which doesn't exist in cases where it is a FieldDecl in the 'root'), and // Wrapper is the current struct being unwrapped. template -static void VisitAccessorWrapper(CXXRecordDecl *Owner, ParentTy &Parent, - CXXRecordDecl *Wrapper, - Handlers &... handlers) { +static void VisitRecord(CXXRecordDecl *Owner, ParentTy &Parent, + CXXRecordDecl *Wrapper, Handlers &... handlers) { (void)std::initializer_list{(handlers.enterStruct(Owner, Parent), 0)...}; - VisitAccessorWrapperHelper(Wrapper, Wrapper->bases(), handlers...); - VisitAccessorWrapperHelper(Wrapper, Wrapper->fields(), handlers...); + VisitRecordHelper(Wrapper, Wrapper->bases(), handlers...); + VisitRecordHelper(Wrapper, Wrapper->fields(), handlers...); (void)std::initializer_list{(handlers.leaveStruct(Owner, Parent), 0)...}; } +int getFieldNumber(const CXXRecordDecl *BaseDecl) { + int Members = 0; + for (const auto *Field : BaseDecl->fields()) + ++Members; + + return Members; +} + +template +static void VisitFunctorBases(CXXRecordDecl *KernelFunctor, + Handlers &... handlers) { + VisitRecordHelper(KernelFunctor, KernelFunctor->bases(), handlers...); +} + // A visitor function that dispatches to functions as defined in // SyclKernelFieldHandler for the purposes of kernel generation. template -static void VisitRecordFields(RecordDecl::field_range Fields, - Handlers &... handlers) { +static void VisitRecordFields(CXXRecordDecl *Owner, Handlers &... handlers) { #define KF_FOR_EACH(FUNC) \ (void)std::initializer_list { (handlers.FUNC(Field, FieldTy), 0)... } - for (const auto &Field : Fields) { + for (const auto &Field : Owner->fields()) { QualType FieldTy = Field->getType(); if (Util::isSyclAccessorType(FieldTy)) + // FIXME: Does this still work? Check KF_FOR_EACH(handleSyclAccessorType); else if (Util::isSyclSamplerType(FieldTy)) + // FIXME: Does this still work? Check KF_FOR_EACH(handleSyclSamplerType); else if (Util::isSyclSpecConstantType(FieldTy)) + // FIXME: Does this still work? Check KF_FOR_EACH(handleSyclSpecConstantType); else if (Util::isSyclStreamType(FieldTy)) { // Stream actually wraps accessors, so do recursion + // FIXME: Does this still work? Check CXXRecordDecl *RD = FieldTy->getAsCXXRecordDecl(); - VisitAccessorWrapper(nullptr, Field, RD, handlers...); + VisitRecord(nullptr, Field, RD, handlers...); KF_FOR_EACH(handleSyclStreamType); } else if (FieldTy->isStructureOrClassType()) { - KF_FOR_EACH(handleStructType); CXXRecordDecl *RD = FieldTy->getAsCXXRecordDecl(); - VisitAccessorWrapper(nullptr, Field, RD, handlers...); + VisitRecord(Owner, Field, RD, handlers...); } else if (FieldTy->isReferenceType()) KF_FOR_EACH(handleReferenceType); else if (FieldTy->isPointerType()) @@ -1169,16 +1179,16 @@ class SyclKernelBodyCreator void handleSpecialType(FieldDecl *FD, QualType Ty) { const auto *RecordDecl = Ty->getAsCXXRecordDecl(); // Perform initialization only if it is field of kernel object - if (MemberExprBases.size() == 1) { - InitializedEntity Entity = - InitializedEntity::InitializeMember(FD, &VarEntity); - // Initialize with the default constructor. - InitializationKind InitKind = - InitializationKind::CreateDefault(SourceLocation()); - InitializationSequence InitSeq(SemaRef, Entity, InitKind, None); - ExprResult MemberInit = InitSeq.Perform(SemaRef, Entity, InitKind, None); - InitExprs.push_back(MemberInit.get()); - } + // if (MemberExprBases.size() == 1) { + InitializedEntity Entity = + InitializedEntity::InitializeMember(FD, &VarEntity); + // Initialize with the default constructor. + InitializationKind InitKind = + InitializationKind::CreateDefault(SourceLocation()); + InitializationSequence InitSeq(SemaRef, Entity, InitKind, None); + ExprResult MemberInit = InitSeq.Perform(SemaRef, Entity, InitKind, None); + InitExprs.push_back(MemberInit.get()); + // } createSpecialMethodCall(RecordDecl, MemberExprBases.back(), InitMethodName, FD); } @@ -1253,6 +1263,46 @@ class SyclKernelBodyCreator } void leaveStruct(const CXXRecordDecl *, FieldDecl *FD) final { + + const CXXRecordDecl *RD = FD->getType()->getAsCXXRecordDecl(); + + if (!RD) + return; + + int NumberOfFields = getFieldNumber(RD); + int popOut = NumberOfFields + RD->getNumBases(); + llvm::SmallVector BaseInitExprs; + for (int I = 0; I < popOut; I++) { + BaseInitExprs.push_back(InitExprs.back()); + InitExprs.pop_back(); + } + std::reverse(BaseInitExprs.begin(), BaseInitExprs.end()); + + Expr *ILE = new (SemaRef.getASTContext()) + InitListExpr(SemaRef.getASTContext(), SourceLocation(), BaseInitExprs, + SourceLocation()); + ILE->setType(QualType(RD->getTypeForDecl(), 0)); + InitExprs.push_back(ILE); + + } + + void leaveStruct(const CXXRecordDecl *RD, const CXXBaseSpecifier &BS) final { + + const CXXRecordDecl *BaseClass = BS.getType()->getAsCXXRecordDecl(); + int NumberOfFields = getFieldNumber(BaseClass); + llvm::SmallVector BaseInitExprs; + for (int I = 0; I < NumberOfFields; I++) { + BaseInitExprs.push_back(InitExprs.back()); + InitExprs.pop_back(); + } + std::reverse(BaseInitExprs.begin(), BaseInitExprs.end()); + + Expr *ILE = new (SemaRef.getASTContext()) + InitListExpr(SemaRef.getASTContext(), SourceLocation(), BaseInitExprs, + SourceLocation()); + ILE->setType(QualType(BaseClass->getTypeForDecl(), 0)); + InitExprs.push_back(ILE); + MemberExprBases.pop_back(); } @@ -1447,7 +1497,9 @@ void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, StableName); ConstructingOpenCLKernel = true; - VisitRecordFields(KernelLambda->fields(), checker, kernel_decl, kernel_body, + VisitFunctorBases(KernelLambda, checker, kernel_decl, kernel_body, + int_header); + VisitRecordFields(KernelLambda, checker, kernel_decl, kernel_body, int_header); ConstructingOpenCLKernel = false; } From 5e739931bd537fa6e59e60caa936bca82adae8c2 Mon Sep 17 00:00:00 2001 From: Elizabeth Andrews Date: Mon, 8 Jun 2020 08:08:12 -0700 Subject: [PATCH 2/5] Refactored and fixed base class crash Signed-off-by: Elizabeth Andrews --- clang/lib/Sema/SemaSYCL.cpp | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 3563dd59d5425..4a2fc045b828e 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1262,10 +1262,7 @@ class SyclKernelBodyCreator MemberExprBases.push_back(BuildMemberExpr(MemberExprBases.back(), FD)); } - void leaveStruct(const CXXRecordDecl *, FieldDecl *FD) final { - - const CXXRecordDecl *RD = FD->getType()->getAsCXXRecordDecl(); - + void addStructInit(const CXXRecordDecl *RD) { if (!RD) return; @@ -1284,26 +1281,22 @@ class SyclKernelBodyCreator ILE->setType(QualType(RD->getTypeForDecl(), 0)); InitExprs.push_back(ILE); + //MemberExprBases.pop_back(); + } - void leaveStruct(const CXXRecordDecl *RD, const CXXBaseSpecifier &BS) final { + void leaveStruct(const CXXRecordDecl *, FieldDecl *FD) final { - const CXXRecordDecl *BaseClass = BS.getType()->getAsCXXRecordDecl(); - int NumberOfFields = getFieldNumber(BaseClass); - llvm::SmallVector BaseInitExprs; - for (int I = 0; I < NumberOfFields; I++) { - BaseInitExprs.push_back(InitExprs.back()); - InitExprs.pop_back(); - } - std::reverse(BaseInitExprs.begin(), BaseInitExprs.end()); + const CXXRecordDecl *RD = FD->getType()->getAsCXXRecordDecl(); - Expr *ILE = new (SemaRef.getASTContext()) - InitListExpr(SemaRef.getASTContext(), SourceLocation(), BaseInitExprs, - SourceLocation()); - ILE->setType(QualType(BaseClass->getTypeForDecl(), 0)); - InitExprs.push_back(ILE); + addStructInit(RD); - MemberExprBases.pop_back(); + } + + void leaveStruct(const CXXRecordDecl *RD, const CXXBaseSpecifier &BS) final { + + const CXXRecordDecl *BaseClass = BS.getType()->getAsCXXRecordDecl(); + addStructInit(BaseClass); } using SyclKernelFieldHandler::enterStruct; From 13a15f5bfc29a62653aff250e5dcd2b6ac0d394f Mon Sep 17 00:00:00 2001 From: Elizabeth Andrews Date: Mon, 8 Jun 2020 17:41:33 -0700 Subject: [PATCH 3/5] Diagnostics should still be emitted for non-standard layout structs. Signed-off-by: Elizabeth Andrews --- clang/lib/Sema/SemaSYCL.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 4a2fc045b828e..9b205b705e318 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -752,6 +752,8 @@ static void VisitRecordFields(CXXRecordDecl *Owner, Handlers &... handlers) { VisitRecord(nullptr, Field, RD, handlers...); KF_FOR_EACH(handleSyclStreamType); } else if (FieldTy->isStructureOrClassType()) { + // Handle diagnostics for non-standard layout + KF_FOR_EACH(handleStructType); CXXRecordDecl *RD = FieldTy->getAsCXXRecordDecl(); VisitRecord(Owner, Field, RD, handlers...); } else if (FieldTy->isReferenceType()) @@ -1000,7 +1002,7 @@ class SyclKernelDeclCreator } void handleStructType(FieldDecl *FD, QualType FieldTy) final { - addParam(FD, FieldTy); + //addParam(FD, FieldTy); } void handleSyclStreamType(FieldDecl *FD, QualType FieldTy) final { @@ -1251,7 +1253,7 @@ class SyclKernelBodyCreator } void handleStructType(FieldDecl *FD, QualType FieldTy) final { - createExprForStructOrScalar(FD); + //createExprForStructOrScalar(FD); } void handleScalarType(FieldDecl *FD, QualType FieldTy) final { @@ -1399,7 +1401,7 @@ class SyclKernelIntHeaderCreator addParam(FD, FieldTy, SYCLIntegrationHeader::kind_pointer); } void handleStructType(FieldDecl *FD, QualType FieldTy) final { - addParam(FD, FieldTy, SYCLIntegrationHeader::kind_std_layout); + //addParam(FD, FieldTy, SYCLIntegrationHeader::kind_std_layout); } void handleScalarType(FieldDecl *FD, QualType FieldTy) final { addParam(FD, FieldTy, SYCLIntegrationHeader::kind_std_layout); From bc5dd54985169918d4e06706459d6aed2b224899 Mon Sep 17 00:00:00 2001 From: Elizabeth Andrews Date: Mon, 8 Jun 2020 20:51:20 -0700 Subject: [PATCH 4/5] Do not visit accessor and stream record. Signed-off-by: Elizabeth Andrews --- clang/lib/Sema/SemaSYCL.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 9b205b705e318..8269a9d3be686 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -688,7 +688,14 @@ static void VisitRecordHelper(CXXRecordDecl *Owner, Handlers &... handlers) { for (const auto &Base : Range) { QualType BaseTy = Base.getType(); - VisitRecord(Owner, Base, BaseTy->getAsCXXRecordDecl(), handlers...); + if (Util::isSyclAccessorType(BaseTy)) + (void)std::initializer_list{ + (handlers.handleSyclAccessorType(Base, BaseTy), 0)...}; + else if (Util::isSyclStreamType(BaseTy)) + (void)std::initializer_list{ + (handlers.handleSyclStreamType(Base, BaseTy), 0)...}; + else + VisitRecord(Owner, Base, BaseTy->getAsCXXRecordDecl(), handlers...); } } From f31653d184d46f5b6c3a5a512447d957084edea9 Mon Sep 17 00:00:00 2001 From: Elizabeth Andrews Date: Mon, 8 Jun 2020 20:55:44 -0700 Subject: [PATCH 5/5] ClangFormat changes Signed-off-by: Elizabeth Andrews --- clang/lib/Sema/SemaSYCL.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 8269a9d3be686..2aa65f5f60044 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -690,11 +690,11 @@ static void VisitRecordHelper(CXXRecordDecl *Owner, QualType BaseTy = Base.getType(); if (Util::isSyclAccessorType(BaseTy)) (void)std::initializer_list{ - (handlers.handleSyclAccessorType(Base, BaseTy), 0)...}; + (handlers.handleSyclAccessorType(Base, BaseTy), 0)...}; else if (Util::isSyclStreamType(BaseTy)) (void)std::initializer_list{ - (handlers.handleSyclStreamType(Base, BaseTy), 0)...}; - else + (handlers.handleSyclStreamType(Base, BaseTy), 0)...}; + else VisitRecord(Owner, Base, BaseTy->getAsCXXRecordDecl(), handlers...); } } @@ -1009,7 +1009,7 @@ class SyclKernelDeclCreator } void handleStructType(FieldDecl *FD, QualType FieldTy) final { - //addParam(FD, FieldTy); + // addParam(FD, FieldTy); } void handleSyclStreamType(FieldDecl *FD, QualType FieldTy) final { @@ -1260,7 +1260,7 @@ class SyclKernelBodyCreator } void handleStructType(FieldDecl *FD, QualType FieldTy) final { - //createExprForStructOrScalar(FD); + // createExprForStructOrScalar(FD); } void handleScalarType(FieldDecl *FD, QualType FieldTy) final { @@ -1408,7 +1408,7 @@ class SyclKernelIntHeaderCreator addParam(FD, FieldTy, SYCLIntegrationHeader::kind_pointer); } void handleStructType(FieldDecl *FD, QualType FieldTy) final { - //addParam(FD, FieldTy, SYCLIntegrationHeader::kind_std_layout); + // addParam(FD, FieldTy, SYCLIntegrationHeader::kind_std_layout); } void handleScalarType(FieldDecl *FD, QualType FieldTy) final { addParam(FD, FieldTy, SYCLIntegrationHeader::kind_std_layout);