From 9cd5f703f117bbea724d5017e77804cf61fd822c Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Wed, 8 Apr 2020 08:02:19 -0700 Subject: [PATCH 01/17] WIP on 'visitor' model for the SemaSYCL refactor Signed-off-by: Erich Keane --- clang/lib/Sema/SemaSYCL.cpp | 240 ++++++++++++++++++++++++++++-------- 1 file changed, 186 insertions(+), 54 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index e795947add373..d86982980fba7 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1256,16 +1256,6 @@ static void populateIntHeader(SYCLIntegrationHeader &H, const StringRef Name, } } -// Creates a mangled kernel name for given kernel name type -static std::string constructKernelName(QualType KernelNameType, - MangleContext &MC) { - SmallString<256> Result; - llvm::raw_svector_ostream Out(Result); - - MC.mangleTypeName(KernelNameType, Out); - return std::string(Out.str()); -} - static FunctionDecl * CreateOpenCLKernelDeclaration(ASTContext &Context, StringRef Name, ArrayRef ParamDescs) { @@ -1307,6 +1297,153 @@ CreateOpenCLKernelDeclaration(ASTContext &Context, StringRef Name, return OpenCLKernel; } +// The first template argument to the kernel function is used to identify the +// kernel itself. +static QualType calculateKernelNameType(ASTContext &Ctx, + FunctionDecl *KernelCallerFunc) { + // TODO: Not sure what the 'fully qualified type's purpose is here, the type + // itself should have its full qualified name, so figure out what the purpose + // is. + const TemplateArgumentList *TAL = + KernelCallerFunc->getTemplateSpecializationArgs(); + return TypeName::getFullyQualifiedType(TAL->get(0).getAsType(), Ctx, + /*WithGlobalNSPrefix=*/true); +} + +// Gets a name for the kernel caller func, calculated from the first template argument. +static std::string constructKernelName(Sema &S, FunctionDecl *KernelCallerFunc, + MangleContext &MC) { + QualType KernelNameType = + calculateKernelNameType(S.getASTContext(), KernelCallerFunc); + + if (S.getLangOpts().SYCLUnnamedLambda) + return PredefinedExpr::ComputeName( + S.getASTContext(), PredefinedExpr::UniqueStableNameType, KernelNameType); + + SmallString<256> Result; + llvm::raw_svector_ostream Out(Result); + + MC.mangleTypeName(KernelNameType, Out); + return std::string(Out.str()); +} + +// anonymous namespace so these don't get linkage. +namespace { +// A base type that the SYCL OpenCL Kernel construction task uses to implement +// individual tasks. +template +class SyclKernelFieldHandler { +protected: + Sema &SemaRef; + SyclKernelFieldHandler(Sema &S) : SemaRef(S) {} + +public: + // TODO: Should the default behavior DO anything? Also, do these need any + // more params? + void handleSyclAccessorType(const FieldDecl *, QualType){} + void handleSyclSamplerType(const FieldDecl *, QualType){} + void handleSyclSpecConstantType(const FieldDecl *, QualType){} + void handleStructType(const FieldDecl *, QualType){} + void handleReferenceType(const FieldDecl *, QualType){} + void handlePointerType(const FieldDecl *, QualType){} + void handleArrayType(const FieldDecl *, QualType){} + void handleScalarType(const FieldDecl *, QualType){} +}; + +// A type to check the valididty of all of the argument types. +class SYCLKernelFieldChecker : public SyclKernelFieldHandler { + bool AllArgsValid = true; + DiagnosticsEngine &Diag; +public: + SYCLKernelFieldChecker(Sema &S) + : SyclKernelFieldHandler(S), Diag(S.getASTContext().getDiagnostics()) {} + bool isValid() { return AllArgsValid; } + + void handleSyclReferenceType(const FieldDecl *FD, QualType ArgTy) { + AllArgsValid = false; + Diag.Report(FD->getLocation(), diag::err_bad_kernel_param_type) << ArgTy; + } + void handleSyclStructType(const FieldDecl *FD, QualType ArgTy) { + if (SemaRef.getASTContext().getLangOpts().SYCLStdLayoutKernelParams && + !ArgTy->isStandardLayoutType()) { + AllArgsValid = false; + Diag.Report(FD->getLocation(), diag::err_sycl_non_std_layout_type) + << ArgTy; + } else { + CXXRecordDecl *RD = ArgTy->getAsCXXRecordDecl(); + if (!RD->hasTrivialCopyConstructor()) { + AllArgsValid = false; + Diag.Report(FD->getLocation(), + diag::err_sycl_non_trivially_copy_ctor_dtor_type) + << 0 << ArgTy; + } else if (!RD->hasTrivialDestructor()) { + AllArgsValid = false; + Diag.Report(FD->getLocation(), + diag::err_sycl_non_trivially_copy_ctor_dtor_type) + << 1 << ArgTy; + } + } + } +}; + +// A type to Create and own the FunctionDecl for the kernel. +class SYCLKernelHeader : public SyclKernelFieldHandler { + SYCLKernelFieldChecker &ArgChecker; + FunctionDecl *KernelObj = nullptr; + +public: + // TODO: More params necessary for the kernel obj header. + SYCLKernelHeader(Sema &S, SYCLKernelFieldChecker &ArgChecker) + : SyclKernelFieldHandler(S), ArgChecker(ArgChecker) {} + ~SYCLKernelHeader() { + // TODO: Should 'body' do this? Does it need to do stuff in its destructor? + if (KernelObj && ArgChecker.isValid()) + SemaRef.addSyclDeviceDecl(KernelObj); + } +}; + +class SYCLKernelBody : public SyclKernelFieldHandler { + SYCLKernelHeader &Header; + +public: + SYCLKernelBody(Sema &S, SYCLKernelHeader &H) + : SyclKernelFieldHandler(S), Header(H) {} +}; +} + +template +void VisitKernelFields(RecordDecl::field_range Fields, Handlers& ... handlers) { + + // Implements the 'for-each-visitor' pattern. +#define KF_FOR_EACH(FUNC) \ + (void)std::initializer_list{(handlers.FUNC(Field, ArgTy), 0)...} + + for (const auto &Field : Fields){ + QualType ArgTy = Field->getType(); + + if (Util::isSyclAccessorType(ArgTy)) + KF_FOR_EACH(handleSyclAccessorType); + else if (Util::isSyclSamplerType(ArgTy)) + KF_FOR_EACH(handleSyclSamplerType); + else if (Util::isSyclSpecConstantType(ArgTy)) + KF_FOR_EACH(handleSyclSpecConstantType); + else if (ArgTy->isStructureOrClassType()) + KF_FOR_EACH(handleStructType); + else if (ArgTy->isReferenceType()) + KF_FOR_EACH(handleReferenceType); + else if (ArgTy->isPointerType()) + KF_FOR_EACH(handlePointerType); + else if (ArgTy->isArrayType()) + KF_FOR_EACH(handleArrayType); + else if (ArgTy->isScalarType()) + KF_FOR_EACH(handleScalarType); + else + llvm_unreachable("Unsupported kernel parameter type"); + } +#undef KF_FOR_EACH +} + + // Generates the OpenCL kernel using KernelCallerFunc (kernel caller // function) defined is SYCL headers. // Generated OpenCL kernel contains the body of the kernel caller function, @@ -1331,52 +1468,47 @@ CreateOpenCLKernelDeclaration(ASTContext &Context, StringRef Name, // void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, MangleContext &MC) { - CXXRecordDecl *LE = getKernelObjectType(KernelCallerFunc); - assert(LE && "invalid kernel caller"); + // The first argument to the KernelCallerFunc is the lambda object. + CXXRecordDecl *KernelLambda = getKernelObjectType(KernelCallerFunc); + assert(KernelLambda && "invalid kernel caller"); + std::string KernelName = constructKernelName(*this, KernelCallerFunc, MC); - // Build list of kernel arguments - llvm::SmallVector ParamDescs; - if (!buildArgTys(getASTContext(), LE, ParamDescs)) - return; + SYCLKernelFieldChecker checker(*this); + SYCLKernelHeader header(*this, checker); + SYCLKernelBody body(*this, header); - // Extract name from kernel caller parameters and mangle it. - const TemplateArgumentList *TemplateArgs = - KernelCallerFunc->getTemplateSpecializationArgs(); - assert(TemplateArgs && "No template argument info"); - QualType KernelNameType = TypeName::getFullyQualifiedType( - TemplateArgs->get(0).getAsType(), getASTContext(), true); - - std::string Name; - // TODO SYCLIntegrationHeader also computes a unique stable name. It should - // probably lose this responsibility and only use the name provided here. - if (getLangOpts().SYCLUnnamedLambda) - Name = PredefinedExpr::ComputeName( - getASTContext(), PredefinedExpr::UniqueStableNameExpr, KernelNameType); - else - Name = constructKernelName(KernelNameType, MC); - - // TODO Maybe don't emit integration header inside the Sema? - populateIntHeader(getSyclIntegrationHeader(), Name, KernelNameType, LE); - - FunctionDecl *OpenCLKernel = - CreateOpenCLKernelDeclaration(getASTContext(), Name, ParamDescs); - - ContextRAII FuncContext(*this, OpenCLKernel); - - // Let's copy source location of a functor/lambda to emit nicer diagnostics - OpenCLKernel->setLocation(LE->getLocation()); - - // If the source function is implicitly inline, the kernel should be marked - // such as well. This allows the kernel to be ODR'd if there are multiple uses - // in different translation units. - OpenCLKernel->setImplicitlyInline(KernelCallerFunc->isInlined()); - - ConstructingOpenCLKernel = true; - CompoundStmt *OpenCLKernelBody = - CreateOpenCLKernelBody(*this, KernelCallerFunc, OpenCLKernel); - ConstructingOpenCLKernel = false; - OpenCLKernel->setBody(OpenCLKernelBody); - addSyclDeviceDecl(OpenCLKernel); + VisitKernelFields(KernelLambda->fields(), checker, header, body); + + /* + // Build list of kernel arguments + llvm::SmallVector ParamDescs; + if (!buildArgTys(getASTContext(), LE, ParamDescs)) + return; + + // TODO Maybe don't emit integration header inside the Sema? + populateIntHeader(getSyclIntegrationHeader(), Name, KernelNameType, LE); + + FunctionDecl *OpenCLKernel = + CreateOpenCLKernelDeclaration(getASTContext(), Name, ParamDescs); + + ContextRAII FuncContext(*this, OpenCLKernel); + + // Let's copy source location of a functor/lambda to emit nicer diagnostics + OpenCLKernel->setLocation(LE->getLocation()); + + // If the source function is implicitly inline, the kernel should be marked + // such as well. This allows the kernel to be ODR'd if there are multiple + uses + // in different translation units. + OpenCLKernel->setImplicitlyInline(KernelCallerFunc->isInlined()); + + ConstructingOpenCLKernel = true; + CompoundStmt *OpenCLKernelBody = + CreateOpenCLKernelBody(*this, KernelCallerFunc, OpenCLKernel); + ConstructingOpenCLKernel = false; + OpenCLKernel->setBody(OpenCLKernelBody); + addSyclDeviceDecl(OpenCLKernel); + */ } void Sema::MarkDevice(void) { From 007b7539e63f4326e02eb0d013413e25bc099069 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Wed, 8 Apr 2020 08:48:11 -0700 Subject: [PATCH 02/17] clang format Signed-off-by: Erich Keane --- 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 d86982980fba7..f5550c5e83df7 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1310,15 +1310,17 @@ static QualType calculateKernelNameType(ASTContext &Ctx, /*WithGlobalNSPrefix=*/true); } -// Gets a name for the kernel caller func, calculated from the first template argument. +// Gets a name for the kernel caller func, calculated from the first template +// argument. static std::string constructKernelName(Sema &S, FunctionDecl *KernelCallerFunc, MangleContext &MC) { QualType KernelNameType = calculateKernelNameType(S.getASTContext(), KernelCallerFunc); if (S.getLangOpts().SYCLUnnamedLambda) - return PredefinedExpr::ComputeName( - S.getASTContext(), PredefinedExpr::UniqueStableNameType, KernelNameType); + return PredefinedExpr::ComputeName(S.getASTContext(), + PredefinedExpr::UniqueStableNameType, + KernelNameType); SmallString<256> Result; llvm::raw_svector_ostream Out(Result); From 6f8e747bd6e1edd8674d90138d8623bb377d88e3 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Thu, 9 Apr 2020 18:53:10 -0700 Subject: [PATCH 03/17] Massive set of additions/changes. Implemneted the declaration generator. Still need IntHeader and kernel body generation Signed-off-by: Erich Keane --- clang/lib/Sema/SemaSYCL.cpp | 491 ++++++++++++++++++++---------------- 1 file changed, 279 insertions(+), 212 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index f5550c5e83df7..e8d5a49f7cce4 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1012,33 +1012,8 @@ static target getAccessTarget(const ClassTemplateSpecializationDecl *AccTy) { // in the following function we extract types of kernel object fields and add it // to the array with kernel parameters descriptors. // Returns true if all arguments are successfully built. -static bool buildArgTys(ASTContext &Context, CXXRecordDecl *KernelObj, +/*static bool buildArgTys(ASTContext &Context, CXXRecordDecl *KernelObj, SmallVectorImpl &ParamDescs) { - auto CreateAndAddPrmDsc = [&](const FieldDecl *Fld, const QualType &ArgType) { - // Create a parameter descriptor and append it to the result - ParamDescs.push_back(makeParamDesc(Fld, ArgType)); - }; - - // Creates a parameter descriptor for SYCL special object - SYCL accessor or - // sampler. - // All special SYCL objects must have __init method. We extract types for - // kernel parameters from __init method parameters. We will use __init method - // and kernel parameters which we build here to initialize special objects in - // the kernel body. - auto createSpecialSYCLObjParamDesc = [&](const FieldDecl *Fld, - const QualType &ArgTy) { - const auto *RecordDecl = ArgTy->getAsCXXRecordDecl(); - assert(RecordDecl && "Special SYCL object must be of a record type"); - - CXXMethodDecl *InitMethod = getMethodByName(RecordDecl, InitMethodName); - assert(InitMethod && "The accessor/sampler must have the __init method"); - unsigned NumParams = InitMethod->getNumParams(); - for (size_t I = 0; I < NumParams; ++I) { - ParmVarDecl *PD = InitMethod->getParamDecl(I); - CreateAndAddPrmDsc(Fld, PD->getType().getCanonicalType()); - } - }; - // Create parameter descriptor for accessor in case when it's wrapped with // some class. // TODO: Do we need support case when sampler is wrapped with some class or @@ -1079,65 +1054,16 @@ static bool buildArgTys(ASTContext &Context, CXXRecordDecl *KernelObj, QualType ArgTy = Fld->getType(); if (Util::isSyclAccessorType(ArgTy) || Util::isSyclSamplerType(ArgTy)) { createSpecialSYCLObjParamDesc(Fld, ArgTy); - } else if (Util::isSyclSpecConstantType(ArgTy)) { - // Specialization constants are not added as arguments. } else if (ArgTy->isStructureOrClassType()) { - if (Context.getLangOpts().SYCLStdLayoutKernelParams) { - if (!ArgTy->isStandardLayoutType()) { - Context.getDiagnostics().Report(Fld->getLocation(), - diag::err_sycl_non_std_layout_type) - << ArgTy; - AllArgsAreValid = false; - continue; - } - } - - CXXRecordDecl *RD = - cast(ArgTy->getAs()->getDecl()); - if (!RD->hasTrivialCopyConstructor()) { - Context.getDiagnostics().Report( - Fld->getLocation(), - diag::err_sycl_non_trivially_copy_ctor_dtor_type) - << 0 << ArgTy; - AllArgsAreValid = false; - continue; - } - if (!RD->hasTrivialDestructor()) { - Context.getDiagnostics().Report( - Fld->getLocation(), - diag::err_sycl_non_trivially_copy_ctor_dtor_type) - << 1 << ArgTy; - AllArgsAreValid = false; - continue; - } - CreateAndAddPrmDsc(Fld, ArgTy); // Create descriptors for each accessor field in the class or struct createParamDescForWrappedAccessors(Fld, ArgTy); - } else if (ArgTy->isReferenceType()) { - Context.getDiagnostics().Report( - Fld->getLocation(), diag::err_bad_kernel_param_type) << ArgTy; - AllArgsAreValid = false; - } else if (ArgTy->isPointerType()) { - // Pointer Arguments need to be in the global address space - QualType PointeeTy = ArgTy->getPointeeType(); - Qualifiers Quals = PointeeTy.getQualifiers(); - Quals.setAddressSpace(LangAS::opencl_global); - PointeeTy = - Context.getQualifiedType(PointeeTy.getUnqualifiedType(), Quals); - QualType ModTy = Context.getPointerType(PointeeTy); - - CreateAndAddPrmDsc(Fld, ModTy); - } else if (ArgTy->isScalarType()) { - CreateAndAddPrmDsc(Fld, ArgTy); - } else { - llvm_unreachable("Unsupported kernel parameter type"); - } } return AllArgsAreValid; } +*/ /// Adds necessary data describing given kernel to the integration header. /// \param H the integration header object @@ -1256,47 +1182,6 @@ static void populateIntHeader(SYCLIntegrationHeader &H, const StringRef Name, } } -static FunctionDecl * -CreateOpenCLKernelDeclaration(ASTContext &Context, StringRef Name, - ArrayRef ParamDescs) { - - DeclContext *DC = Context.getTranslationUnitDecl(); - QualType RetTy = Context.VoidTy; - SmallVector ArgTys; - - // Extract argument types from the descriptor array: - std::transform( - ParamDescs.begin(), ParamDescs.end(), std::back_inserter(ArgTys), - [](const ParamDesc &PD) -> QualType { return std::get<0>(PD); }); - FunctionProtoType::ExtProtoInfo Info(CC_OpenCLKernel); - QualType FuncTy = Context.getFunctionType(RetTy, ArgTys, Info); - DeclarationName DN = DeclarationName(&Context.Idents.get(Name)); - - FunctionDecl *OpenCLKernel = FunctionDecl::Create( - Context, DC, SourceLocation(), SourceLocation(), DN, FuncTy, - Context.getTrivialTypeSourceInfo(RetTy), SC_None); - - llvm::SmallVector Params; - int i = 0; - for (const auto &PD : ParamDescs) { - auto P = ParmVarDecl::Create(Context, OpenCLKernel, SourceLocation(), - SourceLocation(), std::get<1>(PD), - std::get<0>(PD), std::get<2>(PD), SC_None, 0); - P->setScopeInfo(0, i++); - P->setIsUsed(); - Params.push_back(P); - } - OpenCLKernel->setParams(Params); - - OpenCLKernel->addAttr(OpenCLKernelAttr::CreateImplicit(Context)); - OpenCLKernel->addAttr(AsmLabelAttr::CreateImplicit(Context, Name)); - OpenCLKernel->addAttr(ArtificialAttr::CreateImplicit(Context)); - - // Add kernel to translation unit to see it in AST-dump - DC->addDecl(OpenCLKernel); - return OpenCLKernel; -} - // The first template argument to the kernel function is used to identify the // kernel itself. static QualType calculateKernelNameType(ASTContext &Ctx, @@ -1331,6 +1216,39 @@ static std::string constructKernelName(Sema &S, FunctionDecl *KernelCallerFunc, // anonymous namespace so these don't get linkage. namespace { +template +static void VisitKernelFields(RecordDecl::field_range Fields, + Handlers &... handlers) { + +// Implements the 'for-each-visitor' pattern. +#define KF_FOR_EACH(FUNC) \ + (void)std::initializer_list { (handlers.FUNC(Field, ArgTy), 0)... } + + for (const auto &Field : Fields) { + QualType ArgTy = Field->getType(); + + if (Util::isSyclAccessorType(ArgTy)) + KF_FOR_EACH(handleSyclAccessorType); + else if (Util::isSyclSamplerType(ArgTy)) + KF_FOR_EACH(handleSyclSamplerType); + else if (Util::isSyclSpecConstantType(ArgTy)) + KF_FOR_EACH(handleSyclSpecConstantType); + else if (ArgTy->isStructureOrClassType()) + KF_FOR_EACH(handleStructType); + else if (ArgTy->isReferenceType()) + KF_FOR_EACH(handleReferenceType); + else if (ArgTy->isPointerType()) + KF_FOR_EACH(handlePointerType); + else if (ArgTy->isArrayType()) + KF_FOR_EACH(handleArrayType); + else if (ArgTy->isScalarType()) + KF_FOR_EACH(handleScalarType); + else + KF_FOR_EACH(handleOtherType); + } +#undef KF_FOR_EACH +} + // A base type that the SYCL OpenCL Kernel construction task uses to implement // individual tasks. template @@ -1340,120 +1258,263 @@ class SyclKernelFieldHandler { SyclKernelFieldHandler(Sema &S) : SemaRef(S) {} public: - // TODO: Should the default behavior DO anything? Also, do these need any - // more params? - void handleSyclAccessorType(const FieldDecl *, QualType){} - void handleSyclSamplerType(const FieldDecl *, QualType){} - void handleSyclSpecConstantType(const FieldDecl *, QualType){} - void handleStructType(const FieldDecl *, QualType){} - void handleReferenceType(const FieldDecl *, QualType){} - void handlePointerType(const FieldDecl *, QualType){} - void handleArrayType(const FieldDecl *, QualType){} - void handleScalarType(const FieldDecl *, QualType){} + // Mark these virutal so that we can use override in the implementer classes, + // despite virtual dispatch never being used. + virtual void handleSyclAccessorType(const FieldDecl *, QualType){} + virtual void handleSyclSamplerType(const FieldDecl *, QualType){} + virtual void handleSyclSpecConstantType(const FieldDecl *, QualType){} + virtual void handleStructType(const FieldDecl *, QualType){} + virtual void handleReferenceType(const FieldDecl *, QualType){} + virtual void handlePointerType(const FieldDecl *, QualType){} + virtual void handleArrayType(const FieldDecl *, QualType){} + virtual void handleScalarType(const FieldDecl *, QualType){} + // Most handlers shouldn't be handling this, just the field checker. + virtual void handleOtherType(const FieldDecl *, QualType){} }; // A type to check the valididty of all of the argument types. -class SYCLKernelFieldChecker : public SyclKernelFieldHandler { - bool AllArgsValid = true; +class SyclKernelFieldChecker + : public SyclKernelFieldHandler { + bool IsInvalid = false; DiagnosticsEngine &Diag; public: - SYCLKernelFieldChecker(Sema &S) + SyclKernelFieldChecker(Sema &S) : SyclKernelFieldHandler(S), Diag(S.getASTContext().getDiagnostics()) {} - bool isValid() { return AllArgsValid; } + bool isValid() { return !IsInvalid; } - void handleSyclReferenceType(const FieldDecl *FD, QualType ArgTy) { - AllArgsValid = false; - Diag.Report(FD->getLocation(), diag::err_bad_kernel_param_type) << ArgTy; + void handleReferenceType(const FieldDecl *FD, QualType ArgTy) final { + IsInvalid = Diag.Report(FD->getLocation(), diag::err_bad_kernel_param_type) + << ArgTy; } - void handleSyclStructType(const FieldDecl *FD, QualType ArgTy) { + void handleStructType(const FieldDecl *FD, QualType ArgTy) final { if (SemaRef.getASTContext().getLangOpts().SYCLStdLayoutKernelParams && - !ArgTy->isStandardLayoutType()) { - AllArgsValid = false; - Diag.Report(FD->getLocation(), diag::err_sycl_non_std_layout_type) + !ArgTy->isStandardLayoutType()) + IsInvalid = + Diag.Report(FD->getLocation(), diag::err_sycl_non_std_layout_type) << ArgTy; - } else { + else { CXXRecordDecl *RD = ArgTy->getAsCXXRecordDecl(); - if (!RD->hasTrivialCopyConstructor()) { - AllArgsValid = false; - Diag.Report(FD->getLocation(), - diag::err_sycl_non_trivially_copy_ctor_dtor_type) + if (!RD->hasTrivialCopyConstructor()) + + IsInvalid = + Diag.Report(FD->getLocation(), + diag::err_sycl_non_trivially_copy_ctor_dtor_type) << 0 << ArgTy; - } else if (!RD->hasTrivialDestructor()) { - AllArgsValid = false; - Diag.Report(FD->getLocation(), - diag::err_sycl_non_trivially_copy_ctor_dtor_type) + else if (!RD->hasTrivialDestructor()) + IsInvalid = + Diag.Report(FD->getLocation(), + diag::err_sycl_non_trivially_copy_ctor_dtor_type) << 1 << ArgTy; - } } } + + // We should be able to ahndle this, so we made it part of the visitor, but + // this is 'to be implemented'. + void handleArrayType(const FieldDecl *FD, QualType ArgTy) final { + IsInvalid = Diag.Report(FD->getLocation(), diag::err_bad_kernel_param_type) + << ArgTy; + } + + void handleOtherType(const FieldDecl *FD, QualType ArgTy) final { + IsInvalid = Diag.Report(FD->getLocation(), diag::err_bad_kernel_param_type) + << ArgTy; + } }; -// A type to Create and own the FunctionDecl for the kernel. -class SYCLKernelHeader : public SyclKernelFieldHandler { - SYCLKernelFieldChecker &ArgChecker; - FunctionDecl *KernelObj = nullptr; +// A type that handles the accessor recursion and acts as a base for +// SyclKernelDeclCreator. It doesn't 'own' anything other than the KernelObj +// pointer and functionality required to add a param. +class SyclKernelDeclBase + : public SyclKernelFieldHandler { +protected: + FunctionDecl *KernelObj; + llvm::SmallVectorImpl &ArgTys; + llvm::SmallVectorImpl &Params; + + void addParam(const FieldDecl *FD, QualType ArgTy) { + ASTContext &Ctx = SemaRef.getASTContext(); + // TODO: should we split this function up? These ops NEED to happen in + // lockstep, so leaning toward leaving htis as just a somewhat long function + // :/ + + // Create a new ParmVarDecl based on the new info. + ParamDesc newParamDesc = makeParamDesc(FD, ArgTy); + auto *NewParam = ParmVarDecl::Create( + SemaRef.getASTContext(), KernelObj, SourceLocation(), SourceLocation(), + std::get<1>(newParamDesc), std::get<0>(newParamDesc), + std::get<2>(newParamDesc), SC_None, /*DefArg*/ nullptr); + + NewParam->setScopeInfo(0, Params.size()); + NewParam->setIsUsed(); + + Params.push_back(NewParam); + + // Create the new type. + ArgTys.push_back(ArgTy); + FunctionProtoType::ExtProtoInfo Info(CC_OpenCLKernel); + QualType FuncType = Ctx.getFunctionType(Ctx.VoidTy, ArgTys, Info); + + // Set type, note until the destructor of the owner of KernelObj is called + // (to set the parameters), we cannot access the parameters from KernelObj + // without memory problems. + KernelObj->setType(FuncType); + } + + // All special SYCL objects must have __init method. We extract types for + // kernel parameters from __init method parameters. We will use __init method + // and kernel parameters which we build here to initialize special objects in + // the kernel body. + void handleSpecialType(const FieldDecl *FD, QualType ArgTy) { + const auto *RecordDecl = ArgTy->getAsCXXRecordDecl(); + assert(RecordDecl && "Special SYCL object must be of a record type"); + + CXXMethodDecl *InitMethod = getMethodByName(RecordDecl, InitMethodName); + assert(InitMethod && "The accessor/sampler must have the __init method"); + + for (const ParmVarDecl *Param : InitMethod->parameters()) + addParam(FD, Param->getType().getCanonicalType()); + } + public: - // TODO: More params necessary for the kernel obj header. - SYCLKernelHeader(Sema &S, SYCLKernelFieldChecker &ArgChecker) - : SyclKernelFieldHandler(S), ArgChecker(ArgChecker) {} - ~SYCLKernelHeader() { - // TODO: Should 'body' do this? Does it need to do stuff in its destructor? - if (KernelObj && ArgChecker.isValid()) - SemaRef.addSyclDeviceDecl(KernelObj); + SyclKernelDeclBase(Sema &S, FunctionDecl *KernelObj, + llvm::SmallVectorImpl &ArgTys, + llvm::SmallVectorImpl &Params) + : SyclKernelFieldHandler(S), KernelObj(KernelObj), ArgTys(ArgTys), + Params(Params) {} + + void handleSyclAccessorType(const FieldDecl *FD, QualType ArgTy) final { + handleSpecialType(FD, ArgTy); + } + + void handleStructType(const FieldDecl *FD, QualType ArgTy) override { + addParam(FD, ArgTy); + + // Create descriptors for each accessor field in the class or struct + const auto *Wrapper = ArgTy->getAsCXXRecordDecl(); + VisitKernelFields(Wrapper->fields(), *this); } }; -class SYCLKernelBody : public SyclKernelFieldHandler { - SYCLKernelHeader &Header; +// A type to Create and own the FunctionDecl for the kernel. +class SyclKernelDeclCreator + : public SyclKernelDeclBase { + // TODO: rather than this, should we consider a 'commit' function that + // finalizes under success only? + SyclKernelFieldChecker &ArgChecker; + Sema::ContextRAII FuncContext; + // Yes, the list of Parameters contains this info, but we use it often enough + // we shouldn't be recreating it constantly. QualTypes are cheap anyway. + llvm::SmallVector ArgTys; + llvm::SmallVector Params; + + static void setKernelImplicitAttrs(ASTContext &Context, FunctionDecl *FD, StringRef Name) { + // Set implict attributes. + FD->addAttr(OpenCLKernelAttr::CreateImplicit(Context)); + FD->addAttr(AsmLabelAttr::CreateImplicit(Context, Name)); + FD->addAttr(ArtificialAttr::CreateImplicit(Context)); + } + + static FunctionDecl *initKernelObj(ASTContext &Ctx, StringRef Name, + SourceLocation Loc, bool IsInline) { + // Create this with no prototype, and we can fix this up after we've seen + // all the params. + FunctionProtoType::ExtProtoInfo Info(CC_OpenCLKernel); + QualType FuncType = Ctx.getFunctionType(Ctx.VoidTy, {}, Info); + + FunctionDecl *FD = FunctionDecl::Create( + Ctx, Ctx.getTranslationUnitDecl(), Loc, Loc, &Ctx.Idents.get(Name), + FuncType, Ctx.getTrivialTypeSourceInfo(Ctx.VoidTy), SC_None); + FD->setImplicitlyInline(IsInline); + setKernelImplicitAttrs(Ctx, FD, Name); + + // Add kernel to translation unit to see it in AST-dump. + Ctx.getTranslationUnitDecl()->addDecl(FD); + return FD; + } public: - SYCLKernelBody(Sema &S, SYCLKernelHeader &H) - : SyclKernelFieldHandler(S), Header(H) {} -}; -} + SyclKernelDeclCreator(Sema &S, SyclKernelFieldChecker &ArgChecker, + StringRef Name, SourceLocation Loc, bool IsInline) + : SyclKernelDeclBase( + S, initKernelObj(S.getASTContext(), Name, Loc, IsInline), ArgTys, + Params), + ArgChecker(ArgChecker), FuncContext(SemaRef, KernelObj) {} -template -void VisitKernelFields(RecordDecl::field_range Fields, Handlers& ... handlers) { + ~SyclKernelDeclCreator() { + KernelObj->setParams(Params); - // Implements the 'for-each-visitor' pattern. -#define KF_FOR_EACH(FUNC) \ - (void)std::initializer_list{(handlers.FUNC(Field, ArgTy), 0)...} + if (ArgChecker.isValid()) + SemaRef.addSyclDeviceDecl(KernelObj); + } - for (const auto &Field : Fields){ - QualType ArgTy = Field->getType(); + void handleSyclSamplerType(const FieldDecl *FD, QualType ArgTy) final { + handleSpecialType(FD, ArgTy); + } - if (Util::isSyclAccessorType(ArgTy)) - KF_FOR_EACH(handleSyclAccessorType); - else if (Util::isSyclSamplerType(ArgTy)) - KF_FOR_EACH(handleSyclSamplerType); - else if (Util::isSyclSpecConstantType(ArgTy)) - KF_FOR_EACH(handleSyclSpecConstantType); - else if (ArgTy->isStructureOrClassType()) - KF_FOR_EACH(handleStructType); - else if (ArgTy->isReferenceType()) - KF_FOR_EACH(handleReferenceType); - else if (ArgTy->isPointerType()) - KF_FOR_EACH(handlePointerType); - else if (ArgTy->isArrayType()) - KF_FOR_EACH(handleArrayType); - else if (ArgTy->isScalarType()) - KF_FOR_EACH(handleScalarType); - else - llvm_unreachable("Unsupported kernel parameter type"); + void handlePointerType(const FieldDecl *FD, QualType ArgTy) final { + // TODO: Can we document what the heck this is doing?! + QualType PointeeTy = ArgTy->getPointeeType(); + Qualifiers Quals = PointeeTy.getQualifiers(); + Quals.setAddressSpace(LangAS::opencl_global); + PointeeTy = SemaRef.getASTContext().getQualifiedType( + PointeeTy.getUnqualifiedType(), Quals); + QualType ModTy = SemaRef.getASTContext().getPointerType(PointeeTy); + addParam(FD, ModTy); } -#undef KF_FOR_EACH + void handleScalarType(const FieldDecl *FD, QualType ArgTy) final { + addParam(FD, ArgTy); + } + + // This is implemented here because this is the only case where the recurse + // object is required. The base type is pretty cheap, so we might opt + // to just always create it (the way this one is implemented) and just put + // this implementation in the base. + void handleStructType(const FieldDecl *FD, QualType ArgTy) final { + addParam(FD, ArgTy); + + // Create descriptors for each accessor field in the class or struct + const auto *Wrapper = ArgTy->getAsCXXRecordDecl(); + SyclKernelDeclBase Recurse(SemaRef, KernelObj, ArgTys, Params); + VisitKernelFields(Wrapper->fields(), Recurse); + } + + void setBody(CompoundStmt *KB) { + KernelObj->setBody(KB); + } +}; + +class SyclKernelBodyCreator + : public SyclKernelFieldHandler { + SyclKernelDeclCreator &DeclCreator; + // TODO: When/Where does this get created? + CompoundStmt *KernelBody = nullptr; + +public: + SyclKernelBodyCreator(Sema &S, SyclKernelDeclCreator &DC) + : SyclKernelFieldHandler(S), DeclCreator(DC) { + } + ~SyclKernelBodyCreator() { + DeclCreator.setBody(KernelBody); + } +}; + +class SyclKernelIntHeaderCreator + : public SyclKernelFieldHandler { + public: + SyclKernelIntHeaderCreator(Sema &S) : SyclKernelFieldHandler(S) {} +}; } // Generates the OpenCL kernel using KernelCallerFunc (kernel caller -// function) defined is SYCL headers. +// function) defined is Sycl headers. // Generated OpenCL kernel contains the body of the kernel caller function, // receives OpenCL like parameters and additionally does some manipulation to // initialize captured lambda/functor fields with these parameters. -// SYCL runtime marks kernel caller function with sycl_kernel attribute. +// Sycl runtime marks kernel caller function with sycl_kernel attribute. // To be able to generate OpenCL kernel from KernelCallerFunc we put -// the following requirements to the function which SYCL runtime can mark with +// the following requirements to the function which Sycl runtime can mark with // sycl_kernel attribute: // - Must be template function with at least two template parameters. // First parameter must represent "unique kernel name" @@ -1475,41 +1536,47 @@ void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, assert(KernelLambda && "invalid kernel caller"); std::string KernelName = constructKernelName(*this, KernelCallerFunc, MC); - SYCLKernelFieldChecker checker(*this); - SYCLKernelHeader header(*this, checker); - SYCLKernelBody body(*this, header); + SyclKernelFieldChecker checker(*this); + SyclKernelDeclCreator kernel_decl(*this, checker, KernelName, + KernelLambda->getLocation(), + KernelCallerFunc->isInlined()); + SyclKernelBodyCreator kernel_body(*this, kernel_decl); + SyclKernelIntHeaderCreator int_header(*this); - VisitKernelFields(KernelLambda->fields(), checker, header, body); + ConstructingOpenCLKernel = true; + VisitKernelFields(KernelLambda->fields(), checker, kernel_decl, kernel_body, + int_header); + ConstructingOpenCLKernel = false; /* // Build list of kernel arguments - llvm::SmallVector ParamDescs; - if (!buildArgTys(getASTContext(), LE, ParamDescs)) - return; + //llvm::SmallVector ParamDescs; + //if (!buildArgTys(getASTContext(), LE, ParamDescs)) + // return; // TODO Maybe don't emit integration header inside the Sema? - populateIntHeader(getSyclIntegrationHeader(), Name, KernelNameType, LE); + ***populateIntHeader(getSyclIntegrationHeader(), Name, KernelNameType, LE); - FunctionDecl *OpenCLKernel = - CreateOpenCLKernelDeclaration(getASTContext(), Name, ParamDescs); + //FunctionDecl *OpenCLKernel = + // CreateOpenCLKernelDeclaration(getASTContext(), Name, ParamDescs); - ContextRAII FuncContext(*this, OpenCLKernel); + //ContextRAII FuncContext(*this, OpenCLKernel); // Let's copy source location of a functor/lambda to emit nicer diagnostics - OpenCLKernel->setLocation(LE->getLocation()); + //OpenCLKernel->setLocation(LE->getLocation()); // If the source function is implicitly inline, the kernel should be marked // such as well. This allows the kernel to be ODR'd if there are multiple uses // in different translation units. - OpenCLKernel->setImplicitlyInline(KernelCallerFunc->isInlined()); + //OpenCLKernel->setImplicitlyInline(KernelCallerFunc->isInlined()); - ConstructingOpenCLKernel = true; - CompoundStmt *OpenCLKernelBody = + //ConstructingOpenCLKernel = true; + ****CompoundStmt *OpenCLKernelBody = CreateOpenCLKernelBody(*this, KernelCallerFunc, OpenCLKernel); - ConstructingOpenCLKernel = false; - OpenCLKernel->setBody(OpenCLKernelBody); - addSyclDeviceDecl(OpenCLKernel); + //ConstructingOpenCLKernel = false; + //OpenCLKernel->setBody(OpenCLKernelBody); + //addSyclDeviceDecl(OpenCLKernel); */ } From 4d37b4ce4514b71565da82307caabeca00a991d6 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Thu, 9 Apr 2020 18:57:23 -0700 Subject: [PATCH 04/17] clang-format changes Signed-off-by: Erich Keane --- clang/lib/Sema/SemaSYCL.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index e8d5a49f7cce4..a092bbe40ba32 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1260,16 +1260,16 @@ class SyclKernelFieldHandler { public: // Mark these virutal so that we can use override in the implementer classes, // despite virtual dispatch never being used. - virtual void handleSyclAccessorType(const FieldDecl *, QualType){} - virtual void handleSyclSamplerType(const FieldDecl *, QualType){} - virtual void handleSyclSpecConstantType(const FieldDecl *, QualType){} - virtual void handleStructType(const FieldDecl *, QualType){} - virtual void handleReferenceType(const FieldDecl *, QualType){} - virtual void handlePointerType(const FieldDecl *, QualType){} - virtual void handleArrayType(const FieldDecl *, QualType){} - virtual void handleScalarType(const FieldDecl *, QualType){} + virtual void handleSyclAccessorType(const FieldDecl *, QualType) {} + virtual void handleSyclSamplerType(const FieldDecl *, QualType) {} + virtual void handleSyclSpecConstantType(const FieldDecl *, QualType) {} + virtual void handleStructType(const FieldDecl *, QualType) {} + virtual void handleReferenceType(const FieldDecl *, QualType) {} + virtual void handlePointerType(const FieldDecl *, QualType) {} + virtual void handleArrayType(const FieldDecl *, QualType) {} + virtual void handleScalarType(const FieldDecl *, QualType) {} // Most handlers shouldn't be handling this, just the field checker. - virtual void handleOtherType(const FieldDecl *, QualType){} + virtual void handleOtherType(const FieldDecl *, QualType) {} }; // A type to check the valididty of all of the argument types. From 01da1e7faf486187f8c7da9522735ec65e57e6de Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Thu, 9 Apr 2020 19:02:19 -0700 Subject: [PATCH 05/17] more clang-format changes Signed-off-by: Erich Keane --- clang/lib/Sema/SemaSYCL.cpp | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index a092bbe40ba32..9741fb7468e15 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1251,8 +1251,7 @@ static void VisitKernelFields(RecordDecl::field_range Fields, // A base type that the SYCL OpenCL Kernel construction task uses to implement // individual tasks. -template -class SyclKernelFieldHandler { +template class SyclKernelFieldHandler { protected: Sema &SemaRef; SyclKernelFieldHandler(Sema &S) : SemaRef(S) {} @@ -1277,6 +1276,7 @@ class SyclKernelFieldChecker : public SyclKernelFieldHandler { bool IsInvalid = false; DiagnosticsEngine &Diag; + public: SyclKernelFieldChecker(Sema &S) : SyclKernelFieldHandler(S), Diag(S.getASTContext().getDiagnostics()) {} @@ -1324,12 +1324,11 @@ class SyclKernelFieldChecker // A type that handles the accessor recursion and acts as a base for // SyclKernelDeclCreator. It doesn't 'own' anything other than the KernelObj // pointer and functionality required to add a param. -class SyclKernelDeclBase - : public SyclKernelFieldHandler { +class SyclKernelDeclBase : public SyclKernelFieldHandler { protected: FunctionDecl *KernelObj; llvm::SmallVectorImpl &ArgTys; - llvm::SmallVectorImpl &Params; + llvm::SmallVectorImpl &Params; void addParam(const FieldDecl *FD, QualType ArgTy) { ASTContext &Ctx = SemaRef.getASTContext(); @@ -1375,7 +1374,6 @@ class SyclKernelDeclBase addParam(FD, Param->getType().getCanonicalType()); } - public: SyclKernelDeclBase(Sema &S, FunctionDecl *KernelObj, llvm::SmallVectorImpl &ArgTys, @@ -1397,8 +1395,7 @@ class SyclKernelDeclBase }; // A type to Create and own the FunctionDecl for the kernel. -class SyclKernelDeclCreator - : public SyclKernelDeclBase { +class SyclKernelDeclCreator : public SyclKernelDeclBase { // TODO: rather than this, should we consider a 'commit' function that // finalizes under success only? SyclKernelFieldChecker &ArgChecker; @@ -1406,9 +1403,10 @@ class SyclKernelDeclCreator // Yes, the list of Parameters contains this info, but we use it often enough // we shouldn't be recreating it constantly. QualTypes are cheap anyway. llvm::SmallVector ArgTys; - llvm::SmallVector Params; + llvm::SmallVector Params; - static void setKernelImplicitAttrs(ASTContext &Context, FunctionDecl *FD, StringRef Name) { + static void setKernelImplicitAttrs(ASTContext &Context, FunctionDecl *FD, + StringRef Name) { // Set implict attributes. FD->addAttr(OpenCLKernelAttr::CreateImplicit(Context)); FD->addAttr(AsmLabelAttr::CreateImplicit(Context, Name)); @@ -1479,9 +1477,7 @@ class SyclKernelDeclCreator VisitKernelFields(Wrapper->fields(), Recurse); } - void setBody(CompoundStmt *KB) { - KernelObj->setBody(KB); - } + void setBody(CompoundStmt *KB) { KernelObj->setBody(KB); } }; class SyclKernelBodyCreator @@ -1492,20 +1488,16 @@ class SyclKernelBodyCreator public: SyclKernelBodyCreator(Sema &S, SyclKernelDeclCreator &DC) - : SyclKernelFieldHandler(S), DeclCreator(DC) { - } - ~SyclKernelBodyCreator() { - DeclCreator.setBody(KernelBody); - } + : SyclKernelFieldHandler(S), DeclCreator(DC) {} + ~SyclKernelBodyCreator() { DeclCreator.setBody(KernelBody); } }; class SyclKernelIntHeaderCreator : public SyclKernelFieldHandler { - public: - SyclKernelIntHeaderCreator(Sema &S) : SyclKernelFieldHandler(S) {} +public: + SyclKernelIntHeaderCreator(Sema &S) : SyclKernelFieldHandler(S) {} }; -} - +} // namespace // Generates the OpenCL kernel using KernelCallerFunc (kernel caller // function) defined is Sycl headers. From e5ed20c73dfdaeaee961fe8169680ed312e46a05 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Fri, 10 Apr 2020 06:37:40 -0700 Subject: [PATCH 06/17] A few spelling changes, move the kernel type changing Signed-off-by: Erich Keane --- clang/lib/Sema/SemaSYCL.cpp | 56 +++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 9741fb7468e15..8381eab8a43a4 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1216,32 +1216,33 @@ static std::string constructKernelName(Sema &S, FunctionDecl *KernelCallerFunc, // anonymous namespace so these don't get linkage. namespace { +// A visitor function that dispatches to functions as defined in +// SyclKernelFieldHandler for the purposes of kernel generation. template -static void VisitKernelFields(RecordDecl::field_range Fields, +static void VisitRecordFields(RecordDecl::field_range Fields, Handlers &... handlers) { - // Implements the 'for-each-visitor' pattern. #define KF_FOR_EACH(FUNC) \ - (void)std::initializer_list { (handlers.FUNC(Field, ArgTy), 0)... } + (void)std::initializer_list { (handlers.FUNC(Field, FieldTy), 0)... } for (const auto &Field : Fields) { - QualType ArgTy = Field->getType(); + QualType FieldTy = Field->getType(); - if (Util::isSyclAccessorType(ArgTy)) + if (Util::isSyclAccessorType(FieldTy)) KF_FOR_EACH(handleSyclAccessorType); - else if (Util::isSyclSamplerType(ArgTy)) + else if (Util::isSyclSamplerType(FieldTy)) KF_FOR_EACH(handleSyclSamplerType); - else if (Util::isSyclSpecConstantType(ArgTy)) + else if (Util::isSyclSpecConstantType(FieldTy)) KF_FOR_EACH(handleSyclSpecConstantType); - else if (ArgTy->isStructureOrClassType()) + else if (FieldTy->isStructureOrClassType()) KF_FOR_EACH(handleStructType); - else if (ArgTy->isReferenceType()) + else if (FieldTy->isReferenceType()) KF_FOR_EACH(handleReferenceType); - else if (ArgTy->isPointerType()) + else if (FieldTy->isPointerType()) KF_FOR_EACH(handlePointerType); - else if (ArgTy->isArrayType()) + else if (FieldTy->isArrayType()) KF_FOR_EACH(handleArrayType); - else if (ArgTy->isScalarType()) + else if (FieldTy->isScalarType()) KF_FOR_EACH(handleScalarType); else KF_FOR_EACH(handleOtherType); @@ -1331,7 +1332,6 @@ class SyclKernelDeclBase : public SyclKernelFieldHandler { llvm::SmallVectorImpl &Params; void addParam(const FieldDecl *FD, QualType ArgTy) { - ASTContext &Ctx = SemaRef.getASTContext(); // TODO: should we split this function up? These ops NEED to happen in // lockstep, so leaning toward leaving htis as just a somewhat long function // :/ @@ -1348,15 +1348,7 @@ class SyclKernelDeclBase : public SyclKernelFieldHandler { Params.push_back(NewParam); - // Create the new type. ArgTys.push_back(ArgTy); - FunctionProtoType::ExtProtoInfo Info(CC_OpenCLKernel); - QualType FuncType = Ctx.getFunctionType(Ctx.VoidTy, ArgTys, Info); - - // Set type, note until the destructor of the owner of KernelObj is called - // (to set the parameters), we cannot access the parameters from KernelObj - // without memory problems. - KernelObj->setType(FuncType); } // All special SYCL objects must have __init method. We extract types for @@ -1390,7 +1382,7 @@ class SyclKernelDeclBase : public SyclKernelFieldHandler { // Create descriptors for each accessor field in the class or struct const auto *Wrapper = ArgTy->getAsCXXRecordDecl(); - VisitKernelFields(Wrapper->fields(), *this); + VisitRecordFields(Wrapper->fields(), *this); } }; @@ -1413,8 +1405,8 @@ class SyclKernelDeclCreator : public SyclKernelDeclBase { FD->addAttr(ArtificialAttr::CreateImplicit(Context)); } - static FunctionDecl *initKernelObj(ASTContext &Ctx, StringRef Name, - SourceLocation Loc, bool IsInline) { + static FunctionDecl *createKernelDecl(ASTContext &Ctx, StringRef Name, + SourceLocation Loc, bool IsInline) { // Create this with no prototype, and we can fix this up after we've seen // all the params. FunctionProtoType::ExtProtoInfo Info(CC_OpenCLKernel); @@ -1435,11 +1427,15 @@ class SyclKernelDeclCreator : public SyclKernelDeclBase { SyclKernelDeclCreator(Sema &S, SyclKernelFieldChecker &ArgChecker, StringRef Name, SourceLocation Loc, bool IsInline) : SyclKernelDeclBase( - S, initKernelObj(S.getASTContext(), Name, Loc, IsInline), ArgTys, + S, createKernelDecl(S.getASTContext(), Name, Loc, IsInline), ArgTys, Params), ArgChecker(ArgChecker), FuncContext(SemaRef, KernelObj) {} ~SyclKernelDeclCreator() { + ASTContext &Ctx = SemaRef.getASTContext(); + FunctionProtoType::ExtProtoInfo Info(CC_OpenCLKernel); + QualType FuncType = Ctx.getFunctionType(Ctx.VoidTy, ArgTys, Info); + KernelObj->setType(FuncType); KernelObj->setParams(Params); if (ArgChecker.isValid()) @@ -1474,7 +1470,7 @@ class SyclKernelDeclCreator : public SyclKernelDeclBase { // Create descriptors for each accessor field in the class or struct const auto *Wrapper = ArgTy->getAsCXXRecordDecl(); SyclKernelDeclBase Recurse(SemaRef, KernelObj, ArgTys, Params); - VisitKernelFields(Wrapper->fields(), Recurse); + VisitRecordFields(Wrapper->fields(), Recurse); } void setBody(CompoundStmt *KB) { KernelObj->setBody(KB); } @@ -1500,13 +1496,13 @@ class SyclKernelIntHeaderCreator } // namespace // Generates the OpenCL kernel using KernelCallerFunc (kernel caller -// function) defined is Sycl headers. +// function) defined is SYCL headers. // Generated OpenCL kernel contains the body of the kernel caller function, // receives OpenCL like parameters and additionally does some manipulation to // initialize captured lambda/functor fields with these parameters. -// Sycl runtime marks kernel caller function with sycl_kernel attribute. +// SYCL runtime marks kernel caller function with sycl_kernel attribute. // To be able to generate OpenCL kernel from KernelCallerFunc we put -// the following requirements to the function which Sycl runtime can mark with +// the following requirements to the function which SYCL runtime can mark with // sycl_kernel attribute: // - Must be template function with at least two template parameters. // First parameter must represent "unique kernel name" @@ -1536,7 +1532,7 @@ void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, SyclKernelIntHeaderCreator int_header(*this); ConstructingOpenCLKernel = true; - VisitKernelFields(KernelLambda->fields(), checker, kernel_decl, kernel_body, + VisitRecordFields(KernelLambda->fields(), checker, kernel_decl, kernel_body, int_header); ConstructingOpenCLKernel = false; From f9724762cec2e809977dbdaa433b069307d6e5ab Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Fri, 10 Apr 2020 07:31:58 -0700 Subject: [PATCH 07/17] remove TODO comment, the function has now returned to a managable size :) --- clang/lib/Sema/SemaSYCL.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 8381eab8a43a4..61464d292965e 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1332,10 +1332,6 @@ class SyclKernelDeclBase : public SyclKernelFieldHandler { llvm::SmallVectorImpl &Params; void addParam(const FieldDecl *FD, QualType ArgTy) { - // TODO: should we split this function up? These ops NEED to happen in - // lockstep, so leaning toward leaving htis as just a somewhat long function - // :/ - // Create a new ParmVarDecl based on the new info. ParamDesc newParamDesc = makeParamDesc(FD, ArgTy); auto *NewParam = ParmVarDecl::Create( From 9cb607e71faa62bf7bd9a57fd4af1d1fe236f9af Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Fri, 10 Apr 2020 10:56:09 -0700 Subject: [PATCH 08/17] WIP with integration header Signed-off-by: Erich Keane --- clang/lib/Sema/SemaSYCL.cpp | 309 +++++++++++++++++------------------- 1 file changed, 146 insertions(+), 163 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 61464d292965e..c40b2ed413db7 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1007,64 +1007,6 @@ static target getAccessTarget(const ClassTemplateSpecializationDecl *AccTy) { AccTy->getTemplateArgs()[3].getAsIntegral().getExtValue()); } -// Creates list of kernel parameters descriptors using KernelObj (kernel object) -// Fields of kernel object must be initialized with SYCL kernel arguments so -// in the following function we extract types of kernel object fields and add it -// to the array with kernel parameters descriptors. -// Returns true if all arguments are successfully built. -/*static bool buildArgTys(ASTContext &Context, CXXRecordDecl *KernelObj, - SmallVectorImpl &ParamDescs) { - // Create parameter descriptor for accessor in case when it's wrapped with - // some class. - // TODO: Do we need support case when sampler is wrapped with some class or - // struct? - std::function - createParamDescForWrappedAccessors = - [&](const FieldDecl *Fld, const QualType &ArgTy) { - const auto *Wrapper = ArgTy->getAsCXXRecordDecl(); - for (const auto *WrapperFld : Wrapper->fields()) { - QualType FldType = WrapperFld->getType(); - if (FldType->isStructureOrClassType()) { - if (Util::isSyclAccessorType(FldType)) { - // Accessor field is found - create descriptor. - createSpecialSYCLObjParamDesc(WrapperFld, FldType); - } else if (Util::isSyclSpecConstantType(FldType)) { - // Don't try recursive search below. - } else { - // Field is some class or struct - recursively check for - // accessor fields. - createParamDescForWrappedAccessors(WrapperFld, FldType); - } - } - } - }; - - bool AllArgsAreValid = true; - // Run through kernel object fields and create corresponding kernel - // parameters descriptors. There are a several possible cases: - // - Kernel object field is a SYCL special object (SYCL accessor or SYCL - // sampler). These objects has a special initialization scheme - using - // __init method. - // - Kernel object field has a scalar type. In this case we should add - // kernel parameter with the same type. - // - Kernel object field has a structure or class type. Same handling as a - // scalar but we should check if this structure/class contains accessors - // and add parameter decriptor for them properly. - for (const auto *Fld : KernelObj->fields()) { - QualType ArgTy = Fld->getType(); - if (Util::isSyclAccessorType(ArgTy) || Util::isSyclSamplerType(ArgTy)) { - createSpecialSYCLObjParamDesc(Fld, ArgTy); - } else if (ArgTy->isStructureOrClassType()) { - CreateAndAddPrmDsc(Fld, ArgTy); - - // Create descriptors for each accessor field in the class or struct - createParamDescForWrappedAccessors(Fld, ArgTy); - } - - return AllArgsAreValid; -} -*/ - /// Adds necessary data describing given kernel to the integration header. /// \param H the integration header object /// \param Name kernel name @@ -1072,29 +1014,7 @@ static target getAccessTarget(const ClassTemplateSpecializationDecl *AccTy) { /// of single_task, parallel_for, etc) /// \param KernelObjTy kernel object type static void populateIntHeader(SYCLIntegrationHeader &H, const StringRef Name, - QualType NameType, CXXRecordDecl *KernelObjTy) { - - ASTContext &Ctx = KernelObjTy->getASTContext(); - const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(KernelObjTy); - const std::string StableName = PredefinedExpr::ComputeName( - Ctx, PredefinedExpr::UniqueStableNameExpr, NameType); - H.startKernel(Name, NameType, StableName, KernelObjTy->getLocation()); - - auto populateHeaderForAccessor = [&](const QualType &ArgTy, uint64_t Offset) { - // The parameter is a SYCL accessor object. - // The Info field of the parameter descriptor for accessor contains - // two template parameters packed into an integer field: - // - 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(AccTy); - int Dims = static_cast( - AccTmplTy->getTemplateArgs()[1].getAsIntegral().getExtValue()); - int Info = getAccessTarget(AccTmplTy) | (Dims << 11); - H.addParamDesc(SYCLIntegrationHeader::kind_accessor, Info, Offset); - }; - + QualType NameType, CXXRecordDecl *KernelObjTy) {/* std::function populateHeaderForWrappedAccessors = [&](const QualType &ArgTy, uint64_t Offset) { @@ -1180,7 +1100,7 @@ static void populateIntHeader(SYCLIntegrationHeader &H, const StringRef Name, llvm_unreachable("unsupported kernel parameter type"); } } -} +*/} // The first template argument to the kernel function is used to identify the // kernel itself. @@ -1198,15 +1118,14 @@ static QualType calculateKernelNameType(ASTContext &Ctx, // Gets a name for the kernel caller func, calculated from the first template // argument. static std::string constructKernelName(Sema &S, FunctionDecl *KernelCallerFunc, - MangleContext &MC) { + MangleContext &MC, bool StableName) { QualType KernelNameType = calculateKernelNameType(S.getASTContext(), KernelCallerFunc); - if (S.getLangOpts().SYCLUnnamedLambda) + if (StableName) return PredefinedExpr::ComputeName(S.getASTContext(), PredefinedExpr::UniqueStableNameType, KernelNameType); - SmallString<256> Result; llvm::raw_svector_ostream Out(Result); @@ -1216,15 +1135,32 @@ static std::string constructKernelName(Sema &S, FunctionDecl *KernelCallerFunc, // anonymous namespace so these don't get linkage. namespace { +// Implements the 'for-each-visitor' pattern. +#define KF_FOR_EACH(FUNC) \ + (void)std::initializer_list { (handlers.FUNC(Field, FieldTy), 0)... } + +template +static void VisitAccessorWrapperFields(RecordDecl::field_range Fields, + Handlers &... handlers) { + // TODO: Does this need to handle other types to support other things? I + // don't think so, but we'll see. Also want to see if any consumers need to + // handle these 'sub' structs. If so, we likely need to split the + // 'handleStructType' function into two. Do we need to do the same with + // sampler or spec constant? + for (const auto &Field : Fields) { + QualType FieldTy = Field->getType(); + if (Util::isSyclAccessorType(FieldTy)) + KF_FOR_EACH(handleSyclAccessorType); + else if (FieldTy->isStructureOrClassType()) + VisitAccessorWrapperFields(FieldTy->getAsRecordDecl()->fields(), + 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) { -// Implements the 'for-each-visitor' pattern. -#define KF_FOR_EACH(FUNC) \ - (void)std::initializer_list { (handlers.FUNC(Field, FieldTy), 0)... } - for (const auto &Field : Fields) { QualType FieldTy = Field->getType(); @@ -1234,9 +1170,11 @@ static void VisitRecordFields(RecordDecl::field_range Fields, KF_FOR_EACH(handleSyclSamplerType); else if (Util::isSyclSpecConstantType(FieldTy)) KF_FOR_EACH(handleSyclSpecConstantType); - else if (FieldTy->isStructureOrClassType()) + else if (FieldTy->isStructureOrClassType()) { KF_FOR_EACH(handleStructType); - else if (FieldTy->isReferenceType()) + VisitAccessorWrapperFields(FieldTy->getAsRecordDecl()->fields(), + handlers...); + } else if (FieldTy->isReferenceType()) KF_FOR_EACH(handleReferenceType); else if (FieldTy->isPointerType()) KF_FOR_EACH(handlePointerType); @@ -1247,8 +1185,8 @@ static void VisitRecordFields(RecordDecl::field_range Fields, else KF_FOR_EACH(handleOtherType); } -#undef KF_FOR_EACH } +#undef KF_FOR_EACH // A base type that the SYCL OpenCL Kernel construction task uses to implement // individual tasks. @@ -1322,14 +1260,13 @@ class SyclKernelFieldChecker } }; -// A type that handles the accessor recursion and acts as a base for -// SyclKernelDeclCreator. It doesn't 'own' anything other than the KernelObj -// pointer and functionality required to add a param. -class SyclKernelDeclBase : public SyclKernelFieldHandler { -protected: +// A type to Create and own the FunctionDecl for the kernel. +class SyclKernelDeclCreator + : public SyclKernelFieldHandler { FunctionDecl *KernelObj; - llvm::SmallVectorImpl &ArgTys; - llvm::SmallVectorImpl &Params; + llvm::SmallVector Params; + SyclKernelFieldChecker &ArgChecker; + Sema::ContextRAII FuncContext; void addParam(const FieldDecl *FD, QualType ArgTy) { // Create a new ParmVarDecl based on the new info. @@ -1343,8 +1280,6 @@ class SyclKernelDeclBase : public SyclKernelFieldHandler { NewParam->setIsUsed(); Params.push_back(NewParam); - - ArgTys.push_back(ArgTy); } // All special SYCL objects must have __init method. We extract types for @@ -1353,8 +1288,7 @@ class SyclKernelDeclBase : public SyclKernelFieldHandler { // the kernel body. void handleSpecialType(const FieldDecl *FD, QualType ArgTy) { const auto *RecordDecl = ArgTy->getAsCXXRecordDecl(); - assert(RecordDecl && "Special SYCL object must be of a record type"); - + assert(RecordDecl && "The accessor/sampler must be a RecordDecl"); CXXMethodDecl *InitMethod = getMethodByName(RecordDecl, InitMethodName); assert(InitMethod && "The accessor/sampler must have the __init method"); @@ -1362,36 +1296,6 @@ class SyclKernelDeclBase : public SyclKernelFieldHandler { addParam(FD, Param->getType().getCanonicalType()); } -public: - SyclKernelDeclBase(Sema &S, FunctionDecl *KernelObj, - llvm::SmallVectorImpl &ArgTys, - llvm::SmallVectorImpl &Params) - : SyclKernelFieldHandler(S), KernelObj(KernelObj), ArgTys(ArgTys), - Params(Params) {} - - void handleSyclAccessorType(const FieldDecl *FD, QualType ArgTy) final { - handleSpecialType(FD, ArgTy); - } - - void handleStructType(const FieldDecl *FD, QualType ArgTy) override { - addParam(FD, ArgTy); - - // Create descriptors for each accessor field in the class or struct - const auto *Wrapper = ArgTy->getAsCXXRecordDecl(); - VisitRecordFields(Wrapper->fields(), *this); - } -}; - -// A type to Create and own the FunctionDecl for the kernel. -class SyclKernelDeclCreator : public SyclKernelDeclBase { - // TODO: rather than this, should we consider a 'commit' function that - // finalizes under success only? - SyclKernelFieldChecker &ArgChecker; - Sema::ContextRAII FuncContext; - // Yes, the list of Parameters contains this info, but we use it often enough - // we shouldn't be recreating it constantly. QualTypes are cheap anyway. - llvm::SmallVector ArgTys; - llvm::SmallVector Params; static void setKernelImplicitAttrs(ASTContext &Context, FunctionDecl *FD, StringRef Name) { @@ -1422,14 +1326,19 @@ class SyclKernelDeclCreator : public SyclKernelDeclBase { public: SyclKernelDeclCreator(Sema &S, SyclKernelFieldChecker &ArgChecker, StringRef Name, SourceLocation Loc, bool IsInline) - : SyclKernelDeclBase( - S, createKernelDecl(S.getASTContext(), Name, Loc, IsInline), ArgTys, - Params), + : SyclKernelFieldHandler(S), + KernelObj(createKernelDecl(S.getASTContext(), Name, Loc, IsInline)), ArgChecker(ArgChecker), FuncContext(SemaRef, KernelObj) {} ~SyclKernelDeclCreator() { ASTContext &Ctx = SemaRef.getASTContext(); FunctionProtoType::ExtProtoInfo Info(CC_OpenCLKernel); + + SmallVector ArgTys; + std::transform(std::begin(Params), std::end(Params), + std::back_inserter(ArgTys), + [](const ParmVarDecl *PVD) { return PVD->getType(); }); + QualType FuncType = Ctx.getFunctionType(Ctx.VoidTy, ArgTys, Info); KernelObj->setType(FuncType); KernelObj->setParams(Params); @@ -1438,6 +1347,10 @@ class SyclKernelDeclCreator : public SyclKernelDeclBase { SemaRef.addSyclDeviceDecl(KernelObj); } + void handleSyclAccessorType(const FieldDecl *FD, QualType ArgTy) final { + handleSpecialType(FD, ArgTy); + } + void handleSyclSamplerType(const FieldDecl *FD, QualType ArgTy) final { handleSpecialType(FD, ArgTy); } @@ -1452,6 +1365,7 @@ class SyclKernelDeclCreator : public SyclKernelDeclBase { QualType ModTy = SemaRef.getASTContext().getPointerType(PointeeTy); addParam(FD, ModTy); } + void handleScalarType(const FieldDecl *FD, QualType ArgTy) final { addParam(FD, ArgTy); } @@ -1462,11 +1376,6 @@ class SyclKernelDeclCreator : public SyclKernelDeclBase { // this implementation in the base. void handleStructType(const FieldDecl *FD, QualType ArgTy) final { addParam(FD, ArgTy); - - // Create descriptors for each accessor field in the class or struct - const auto *Wrapper = ArgTy->getAsCXXRecordDecl(); - SyclKernelDeclBase Recurse(SemaRef, KernelObj, ArgTys, Params); - VisitRecordFields(Wrapper->fields(), Recurse); } void setBody(CompoundStmt *KB) { KernelObj->setBody(KB); } @@ -1486,8 +1395,90 @@ class SyclKernelBodyCreator class SyclKernelIntHeaderCreator : public SyclKernelFieldHandler { + SYCLIntegrationHeader &Header; + // Required for calculating the field offsets. + const ASTRecordLayout &Layout; + + uint64_t getFieldOffset(const FieldDecl *FD) const { + // TODO: FIX THIS FOR THE RECURSE CASE! + return Layout.getFieldOffset(FD->getFieldIndex()) / 8; + } + + void addParam(const FieldDecl *FD, QualType ArgTy) { + // TODO: fieldOffset is WRONG if this is in a wrapper! + uint64_t Size = + SemaRef.getASTContext().getTypeSizeInChars(ArgTy).getQuantity(); + Header.addParamDesc(SYCLIntegrationHeader::kind_std_layout, + static_cast(Size), + static_cast(getFieldOffset(FD))); + } + public: - SyclKernelIntHeaderCreator(Sema &S) : SyclKernelFieldHandler(S) {} + SyclKernelIntHeaderCreator(Sema &S, SYCLIntegrationHeader &H, + const ASTRecordLayout &LambdaLayout, + SourceLocation KernelLoc, QualType NameType, + StringRef Name, StringRef StableName) + : SyclKernelFieldHandler(S), Header(H), Layout(LambdaLayout) { + Header.startKernel(Name, NameType, StableName, KernelLoc); + } + + void handleSyclAccessorType(const FieldDecl *FD, QualType ArgTy) final { + // TODO: offset stuff is wrong again in the recursion case!? + const auto *AccTy = + cast(ArgTy->getAsRecordDecl()); + // TODO: Is this the right assert here? or is it exactly 2? + assert(AccTy->getTemplateArgs().size() >= 2 && + "Incorrect template args for Accessor Type"); + int Dims = static_cast( + AccTy->getTemplateArgs()[1].getAsIntegral().getExtValue()); + int Info = getAccessTarget(AccTy) | (Dims << 11); + Header.addParamDesc(SYCLIntegrationHeader::kind_accessor, Info, + getFieldOffset(FD)); + } + + void handleSyclSamplerType(const FieldDecl *FD, QualType ArgTy) final { + const auto *SamplerTy = ArgTy->getAsCXXRecordDecl(); + assert(SamplerTy && "Sampler type must be a C++ record type"); + CXXMethodDecl *InitMethod = getMethodByName(SamplerTy, InitMethodName); + assert(InitMethod && "sampler must have __init method"); + + // sampler __init method has only one argument + const ParmVarDecl *SamplerArg = InitMethod->getParamDecl(0); + assert(SamplerArg && "sampler __init method must have sampler parameter"); + + addParam(FD, SamplerArg->getType()); + } + + void handleSyclSpecConstantType(const FieldDecl *FD, QualType ArgTy) final { + const TemplateArgumentList &TemplateArgs = + cast(ArgTy->getAsRecordDecl()) + ->getTemplateInstantiationArgs(); + // TODO: Is this the right assert here? or is it exactly 2? + assert(TemplateArgs.size() >= 2 && + "Incorrect template args for Accessor Type"); + // Get specialization constant ID type, which is the second template + // argument. + QualType SpecConstIDTy = + TypeName::getFullyQualifiedType(TemplateArgs.get(1).getAsType(), + SemaRef.getASTContext(), true) + .getCanonicalType(); + const std::string SpecConstName = PredefinedExpr::ComputeName( + SemaRef.getASTContext(), PredefinedExpr::UniqueStableNameType, + SpecConstIDTy); + Header.addSpecConstant(SpecConstName, SpecConstIDTy); + } + + void handlePointerType(const FieldDecl *FD, QualType ArgTy) final { + addParam(FD, ArgTy); + } + void handleStructType(const FieldDecl *FD, QualType ArgTy) final { + addParam(FD, ArgTy); + } + void handleScalarType(const FieldDecl *FD, QualType ArgTy) final { + addParam(FD, ArgTy); + } + + }; } // namespace @@ -1518,14 +1509,25 @@ void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, // The first argument to the KernelCallerFunc is the lambda object. CXXRecordDecl *KernelLambda = getKernelObjectType(KernelCallerFunc); assert(KernelLambda && "invalid kernel caller"); - std::string KernelName = constructKernelName(*this, KernelCallerFunc, MC); + + // Calculate both names, since Integration headers need both. + std::string CalculatedName = + constructKernelName(*this, KernelCallerFunc, MC, /*StableName*/ false); + std::string StableName = + constructKernelName(*this, KernelCallerFunc, MC, /*StableName*/ true); + StringRef KernelName(getLangOpts().SYCLUnnamedLambda ? StableName + : CalculatedName); SyclKernelFieldChecker checker(*this); SyclKernelDeclCreator kernel_decl(*this, checker, KernelName, KernelLambda->getLocation(), KernelCallerFunc->isInlined()); SyclKernelBodyCreator kernel_body(*this, kernel_decl); - SyclKernelIntHeaderCreator int_header(*this); + SyclKernelIntHeaderCreator int_header( + *this, getSyclIntegrationHeader(), + Context.getASTRecordLayout(KernelLambda), KernelLambda->getLocation(), + calculateKernelNameType(Context, KernelCallerFunc), CalculatedName, + StableName); ConstructingOpenCLKernel = true; VisitRecordFields(KernelLambda->fields(), checker, kernel_decl, kernel_body, @@ -1533,28 +1535,9 @@ void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, ConstructingOpenCLKernel = false; /* - // Build list of kernel arguments - //llvm::SmallVector ParamDescs; - //if (!buildArgTys(getASTContext(), LE, ParamDescs)) - // return; - // TODO Maybe don't emit integration header inside the Sema? ***populateIntHeader(getSyclIntegrationHeader(), Name, KernelNameType, LE); - //FunctionDecl *OpenCLKernel = - // CreateOpenCLKernelDeclaration(getASTContext(), Name, ParamDescs); - - //ContextRAII FuncContext(*this, OpenCLKernel); - - // Let's copy source location of a functor/lambda to emit nicer diagnostics - //OpenCLKernel->setLocation(LE->getLocation()); - - // If the source function is implicitly inline, the kernel should be marked - // such as well. This allows the kernel to be ODR'd if there are multiple - uses - // in different translation units. - //OpenCLKernel->setImplicitlyInline(KernelCallerFunc->isInlined()); - //ConstructingOpenCLKernel = true; ****CompoundStmt *OpenCLKernelBody = CreateOpenCLKernelBody(*this, KernelCallerFunc, OpenCLKernel); From 39e988c8369f9fcdac4d94217415826f27b1a101 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Fri, 10 Apr 2020 12:34:28 -0700 Subject: [PATCH 09/17] Fix int header 'kind' calculation. Also, make sure we visit bases. Still need to fix the integration header offset calculation/keeping track Signed-off-by: Erich Keane --- clang/lib/Sema/SemaSYCL.cpp | 74 ++++++++++++++++++++++++++----------- 1 file changed, 52 insertions(+), 22 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index c40b2ed413db7..7c7ebe22fc0a0 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1151,9 +1151,17 @@ static void VisitAccessorWrapperFields(RecordDecl::field_range Fields, QualType FieldTy = Field->getType(); if (Util::isSyclAccessorType(FieldTy)) KF_FOR_EACH(handleSyclAccessorType); - else if (FieldTy->isStructureOrClassType()) + else if (FieldTy->isStructureOrClassType()) { + CXXRecordDecl *RD = FieldTy->getAsRecordDecl(); + RD->forAllBases( + [](CXXRecordDecl *Base) { + VisitAccessorWrapperFields(Base->fields(), handlers...); + return true; + }, + /*AllowShortCircuit*/ false); VisitAccessorWrapperFields(FieldTy->getAsRecordDecl()->fields(), handlers...); + } } } // A visitor function that dispatches to functions as defined in @@ -1172,6 +1180,19 @@ static void VisitRecordFields(RecordDecl::field_range Fields, KF_FOR_EACH(handleSyclSpecConstantType); else if (FieldTy->isStructureOrClassType()) { KF_FOR_EACH(handleStructType); + + CXXRecordDecl *RD = FieldTy->getAsRecordDecl(); + // Go through the fields of bases as well, the previous implementation + // missed these, so I presume this is going to be fixing a bug. This goes + // through all the bases in a non-guaranteed way, though it skips VBases + // which are otherwise not allowed anyway. + RD->forAllBases( + [](CXXRecordDecl *Base) { + VisitAccessorWrapperFields(Base->fields(), handlers...); + return true; + }, + /*AllowShortCircuit*/ false); + VisitAccessorWrapperFields(FieldTy->getAsRecordDecl()->fields(), handlers...); } else if (FieldTy->isReferenceType()) @@ -1190,12 +1211,13 @@ static void VisitRecordFields(RecordDecl::field_range Fields, // A base type that the SYCL OpenCL Kernel construction task uses to implement // individual tasks. -template class SyclKernelFieldHandler { +template class SyclKernelFieldVisitor { protected: Sema &SemaRef; SyclKernelFieldHandler(Sema &S) : SemaRef(S) {} public: + // Mark these virutal so that we can use override in the implementer classes, // despite virtual dispatch never being used. virtual void handleSyclAccessorType(const FieldDecl *, QualType) {} @@ -1396,38 +1418,47 @@ class SyclKernelBodyCreator class SyclKernelIntHeaderCreator : public SyclKernelFieldHandler { SYCLIntegrationHeader &Header; - // Required for calculating the field offsets. - const ASTRecordLayout &Layout; + const CXXRecordDecl *KernelLambda; + + // Keeping track of offsets as we go along is a little awkward, so see if just + // calculating each time is worth doing. Presumably, if the structure depth doesn't + // get insane, we shouldn't have a problem. + uint64_t getFieldOffsetHelper(const CXXRecordDecl *RD, const FieldDecl *FD) { + // TODO! + return 0; + } uint64_t getFieldOffset(const FieldDecl *FD) const { - // TODO: FIX THIS FOR THE RECURSE CASE! - return Layout.getFieldOffset(FD->getFieldIndex()) / 8; + // TODO: Figure out a better way to do this, having to recalculate this + // constantly is going to be expensive. + // TODO: Figure out how to calc lower down the structs. + // uint64_t CurOffset = SemaRef.getASTContext().getFieldOffset(FD) / 8; + return 0; } - void addParam(const FieldDecl *FD, QualType ArgTy) { - // TODO: fieldOffset is WRONG if this is in a wrapper! + void addParam(const FieldDecl *FD, QualType ArgTy, + SYCLIntegrationHeader::kernel_param_kind_t Kind) { uint64_t Size = SemaRef.getASTContext().getTypeSizeInChars(ArgTy).getQuantity(); - Header.addParamDesc(SYCLIntegrationHeader::kind_std_layout, - static_cast(Size), + Header.addParamDesc(Kind, static_cast(Size), static_cast(getFieldOffset(FD))); } public: SyclKernelIntHeaderCreator(Sema &S, SYCLIntegrationHeader &H, - const ASTRecordLayout &LambdaLayout, + const CXXRecordDecl *KernelLambda, SourceLocation KernelLoc, QualType NameType, StringRef Name, StringRef StableName) - : SyclKernelFieldHandler(S), Header(H), Layout(LambdaLayout) { - Header.startKernel(Name, NameType, StableName, KernelLoc); + : SyclKernelFieldHandler(S), Header(H), + KernelLambda(KernelLambda) { + Header.startKernel(Name, NameType, StableName, KernelLambda->GetLocation()); } void handleSyclAccessorType(const FieldDecl *FD, QualType ArgTy) final { // TODO: offset stuff is wrong again in the recursion case!? const auto *AccTy = cast(ArgTy->getAsRecordDecl()); - // TODO: Is this the right assert here? or is it exactly 2? - assert(AccTy->getTemplateArgs().size() >= 2 && + assert(AccTy->getTemplateArgs().size() == 2 && "Incorrect template args for Accessor Type"); int Dims = static_cast( AccTy->getTemplateArgs()[1].getAsIntegral().getExtValue()); @@ -1446,15 +1477,14 @@ class SyclKernelIntHeaderCreator const ParmVarDecl *SamplerArg = InitMethod->getParamDecl(0); assert(SamplerArg && "sampler __init method must have sampler parameter"); - addParam(FD, SamplerArg->getType()); + addParam(FD, SamplerArg->getType(), SYCLIntegrationHeader::kind_sampler); } void handleSyclSpecConstantType(const FieldDecl *FD, QualType ArgTy) final { const TemplateArgumentList &TemplateArgs = cast(ArgTy->getAsRecordDecl()) ->getTemplateInstantiationArgs(); - // TODO: Is this the right assert here? or is it exactly 2? - assert(TemplateArgs.size() >= 2 && + assert(TemplateArgs.size() == 2 && "Incorrect template args for Accessor Type"); // Get specialization constant ID type, which is the second template // argument. @@ -1469,13 +1499,13 @@ class SyclKernelIntHeaderCreator } void handlePointerType(const FieldDecl *FD, QualType ArgTy) final { - addParam(FD, ArgTy); + addParam(FD, ArgTy, SYCLIntegrationHeader::kind_pointer); } void handleStructType(const FieldDecl *FD, QualType ArgTy) final { - addParam(FD, ArgTy); + addParam(FD, ArgTy, SYCLIntegrationHeader::kind_std_layout); } void handleScalarType(const FieldDecl *FD, QualType ArgTy) final { - addParam(FD, ArgTy); + addParam(FD, ArgTy, SYCLIntegrationHeader::kind_std_layout); } @@ -1525,7 +1555,7 @@ void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, SyclKernelBodyCreator kernel_body(*this, kernel_decl); SyclKernelIntHeaderCreator int_header( *this, getSyclIntegrationHeader(), - Context.getASTRecordLayout(KernelLambda), KernelLambda->getLocation(), + KernelLambda, Context.getASTRecordLayout(KernelLambda), KernelLambda->getLocation(), calculateKernelNameType(Context, KernelCallerFunc), CalculatedName, StableName); From 055fcd4ffaf18901cf644a188ff224b1fdcec786 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Fri, 10 Apr 2020 14:07:53 -0700 Subject: [PATCH 10/17] small refactoring for base classes --- clang/lib/Sema/SemaSYCL.cpp | 267 +++++++++++++++--------------------- 1 file changed, 110 insertions(+), 157 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 7c7ebe22fc0a0..12ecdf2f2e3f0 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1001,107 +1001,22 @@ static ParamDesc makeParamDesc(const FieldDecl *Src, QualType Ty) { Ctx.getTrivialTypeSourceInfo(Ty)); } +static ParamDesc makeParamDesc(ASTContext &Ctx, const CXXBaseSpecifier &Src, + QualType Ty) { + // TODO: There is no name for the base available, but duplicate names are + // seemingly already possible, so we'll give them all the same name for now. + // This only happens with the accessor types. + std::string Name = "_arg__base"; + return std::make_tuple(Ty, &Ctx.Idents.get(Name), + Ctx.getTrivialTypeSourceInfo(Ty)); +} + /// \return the target of given SYCL accessor type static target getAccessTarget(const ClassTemplateSpecializationDecl *AccTy) { return static_cast( AccTy->getTemplateArgs()[3].getAsIntegral().getExtValue()); } -/// Adds necessary data describing given kernel to the integration header. -/// \param H the integration header object -/// \param Name kernel name -/// \param NameType type representing kernel name (first template argument -/// of single_task, parallel_for, etc) -/// \param KernelObjTy kernel object type -static void populateIntHeader(SYCLIntegrationHeader &H, const StringRef Name, - QualType NameType, CXXRecordDecl *KernelObjTy) {/* - std::function - populateHeaderForWrappedAccessors = [&](const QualType &ArgTy, - uint64_t Offset) { - const auto *Wrapper = ArgTy->getAsCXXRecordDecl(); - for (const auto *WrapperFld : Wrapper->fields()) { - QualType FldType = WrapperFld->getType(); - if (FldType->isStructureOrClassType()) { - ASTContext &WrapperCtx = Wrapper->getASTContext(); - const ASTRecordLayout &WrapperLayout = - WrapperCtx.getASTRecordLayout(Wrapper); - // Get offset (in bytes) of the field in wrapper class or struct - uint64_t OffsetInWrapper = - WrapperLayout.getFieldOffset(WrapperFld->getFieldIndex()) / 8; - if (Util::isSyclAccessorType(FldType)) { - // This is an accesor - populate the header appropriately - populateHeaderForAccessor(FldType, Offset + OffsetInWrapper); - } else { - // This is an other class or struct - recursively search for an - // accessor field - populateHeaderForWrappedAccessors(FldType, - Offset + OffsetInWrapper); - } - } - } - }; - - for (const auto Fld : KernelObjTy->fields()) { - QualType ActualArgType; - QualType ArgTy = Fld->getType(); - - // Get offset in bytes - uint64_t Offset = Layout.getFieldOffset(Fld->getFieldIndex()) / 8; - - if (Util::isSyclAccessorType(ArgTy)) { - populateHeaderForAccessor(ArgTy, Offset); - } else if (Util::isSyclSamplerType(ArgTy)) { - // The parameter is a SYCL sampler object - const auto *SamplerTy = ArgTy->getAsCXXRecordDecl(); - assert(SamplerTy && "sampler must be of a record type"); - - CXXMethodDecl *InitMethod = getMethodByName(SamplerTy, InitMethodName); - assert(InitMethod && "sampler must have __init method"); - - // sampler __init method has only one argument - auto *FuncDecl = cast(InitMethod); - ParmVarDecl *SamplerArg = FuncDecl->getParamDecl(0); - assert(SamplerArg && "sampler __init method must have sampler parameter"); - uint64_t Sz = Ctx.getTypeSizeInChars(SamplerArg->getType()).getQuantity(); - H.addParamDesc(SYCLIntegrationHeader::kind_sampler, - static_cast(Sz), static_cast(Offset)); - } else if (ArgTy->isPointerType()) { - uint64_t Sz = Ctx.getTypeSizeInChars(Fld->getType()).getQuantity(); - H.addParamDesc(SYCLIntegrationHeader::kind_pointer, - static_cast(Sz), static_cast(Offset)); - } else if (Util::isSyclSpecConstantType(ArgTy)) { - // Add specialization constant ID to the header. - auto *TmplSpec = - cast(ArgTy->getAsCXXRecordDecl()); - const TemplateArgumentList *TemplateArgs = - &TmplSpec->getTemplateInstantiationArgs(); - // Get specialization constant ID type, which is the second template - // argument. - QualType SpecConstIDTy = TypeName::getFullyQualifiedType( - TemplateArgs->get(1).getAsType(), Ctx, true) - .getCanonicalType(); - const std::string SpecConstName = PredefinedExpr::ComputeName( - Ctx, PredefinedExpr::UniqueStableNameExpr, SpecConstIDTy); - H.addSpecConstant(SpecConstName, SpecConstIDTy); - // Spec constant lambda capture does not become a kernel argument. - } else if (ArgTy->isStructureOrClassType() || ArgTy->isScalarType()) { - // the parameter is an object of standard layout type or scalar; - // the check for standard layout is done elsewhere - uint64_t Sz = Ctx.getTypeSizeInChars(Fld->getType()).getQuantity(); - H.addParamDesc(SYCLIntegrationHeader::kind_std_layout, - static_cast(Sz), static_cast(Offset)); - - // check for accessor fields in structure or class and populate the - // integration header appropriately - if (ArgTy->isStructureOrClassType()) { - populateHeaderForWrappedAccessors(ArgTy, Offset); - } - } else { - llvm_unreachable("unsupported kernel parameter type"); - } - } -*/} - // The first template argument to the kernel function is used to identify the // kernel itself. static QualType calculateKernelNameType(ASTContext &Ctx, @@ -1136,39 +1051,46 @@ static std::string constructKernelName(Sema &S, FunctionDecl *KernelCallerFunc, // anonymous namespace so these don't get linkage. namespace { // Implements the 'for-each-visitor' pattern. -#define KF_FOR_EACH(FUNC) \ - (void)std::initializer_list { (handlers.FUNC(Field, FieldTy), 0)... } - template -static void VisitAccessorWrapperFields(RecordDecl::field_range Fields, - Handlers &... handlers) { - // TODO: Does this need to handle other types to support other things? I - // don't think so, but we'll see. Also want to see if any consumers need to - // handle these 'sub' structs. If so, we likely need to split the - // 'handleStructType' function into two. Do we need to do the same with - // sampler or spec constant? - for (const auto &Field : Fields) { - QualType FieldTy = Field->getType(); - if (Util::isSyclAccessorType(FieldTy)) - KF_FOR_EACH(handleSyclAccessorType); - else if (FieldTy->isStructureOrClassType()) { - CXXRecordDecl *RD = FieldTy->getAsRecordDecl(); - RD->forAllBases( - [](CXXRecordDecl *Base) { - VisitAccessorWrapperFields(Base->fields(), handlers...); - return true; - }, - /*AllowShortCircuit*/ false); - VisitAccessorWrapperFields(FieldTy->getAsRecordDecl()->fields(), - handlers...); +static void VisitAccessorWrapper(CXXRecordDecl *Wrapper, + Handlers &... handlers); + +QualType getItemType(const FieldDecl *FD) { + return FD->getType(); +} +QualType getItemType(const CXXBaseSpecifier &BS) { + return BS.getType(); +} + +template +static void VisitAccessorWrapperHelper(RangeTy Range, Handlers &... handlers) { + for (const auto &Item : Range) { + QualType ItemTy = getItemType(Item); + if (Util::isSyclAccessorType(ItemTy)) + (void)std::initializer_list{ + (handlers.handleAccessorType(Item, ItemTy), 0)...}; + else if (ItemTy->isStructureOrClassType()) { + VisitAccessorWrapper(ItemTy->getAsCXXRecordDecl()); } } } + +template +static void VisitAccessorWrapper(CXXRecordDecl *Wrapper, + Handlers &... handlers) { + + VisitAccessorWrapperHelper(Wrapper->bases(), handlers...); + VisitAccessorWrapperHelper(Wrapper->fields(), 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) { +#define KF_FOR_EACH(FUNC) \ + (void)std::initializer_list { (handlers.FUNC(Field, FieldTy), 0)... } + for (const auto &Field : Fields) { QualType FieldTy = Field->getType(); @@ -1180,21 +1102,8 @@ static void VisitRecordFields(RecordDecl::field_range Fields, KF_FOR_EACH(handleSyclSpecConstantType); else if (FieldTy->isStructureOrClassType()) { KF_FOR_EACH(handleStructType); - - CXXRecordDecl *RD = FieldTy->getAsRecordDecl(); - // Go through the fields of bases as well, the previous implementation - // missed these, so I presume this is going to be fixing a bug. This goes - // through all the bases in a non-guaranteed way, though it skips VBases - // which are otherwise not allowed anyway. - RD->forAllBases( - [](CXXRecordDecl *Base) { - VisitAccessorWrapperFields(Base->fields(), handlers...); - return true; - }, - /*AllowShortCircuit*/ false); - - VisitAccessorWrapperFields(FieldTy->getAsRecordDecl()->fields(), - handlers...); + CXXRecordDecl *RD = FieldTy->getAsCXXRecordDecl(); + VisitAccessorWrapper(RD); } else if (FieldTy->isReferenceType()) KF_FOR_EACH(handleReferenceType); else if (FieldTy->isPointerType()) @@ -1206,20 +1115,26 @@ static void VisitRecordFields(RecordDecl::field_range Fields, else KF_FOR_EACH(handleOtherType); } -} #undef KF_FOR_EACH +} // A base type that the SYCL OpenCL Kernel construction task uses to implement // individual tasks. -template class SyclKernelFieldVisitor { +template class SyclKernelFieldHandler { protected: Sema &SemaRef; SyclKernelFieldHandler(Sema &S) : SemaRef(S) {} public: - // Mark these virutal so that we can use override in the implementer classes, // despite virtual dispatch never being used. + + //// TODO: Can these return 'bool' and we can short-circuit the handling? That + // way the field checker cna return true/false based on whether the rest + // should be still working. + + // Accessor can be a base class or a field decl, so both must be handled. + virtual void handleSyclAccessorType(const CXXBaseSpecifier &, QualType) {} virtual void handleSyclAccessorType(const FieldDecl *, QualType) {} virtual void handleSyclSamplerType(const FieldDecl *, QualType) {} virtual void handleSyclSpecConstantType(const FieldDecl *, QualType) {} @@ -1291,8 +1206,17 @@ class SyclKernelDeclCreator Sema::ContextRAII FuncContext; void addParam(const FieldDecl *FD, QualType ArgTy) { - // Create a new ParmVarDecl based on the new info. ParamDesc newParamDesc = makeParamDesc(FD, ArgTy); + addParam(newParamDesc, ArgTy); + } + + void addParam(const CXXBaseSpecifier &BS, QualType ArgTy) { + ParamDesc newParamDesc = makeParamDesc(SemaRef.getASTContext(), BS, ArgTy); + addParam(newParamDesc, ArgTy); + } + + void addParam(ParamDesc newParamDesc, QualType ArgTy) { + // Create a new ParmVarDecl based on the new info. auto *NewParam = ParmVarDecl::Create( SemaRef.getASTContext(), KernelObj, SourceLocation(), SourceLocation(), std::get<1>(newParamDesc), std::get<0>(newParamDesc), @@ -1369,6 +1293,17 @@ class SyclKernelDeclCreator SemaRef.addSyclDeviceDecl(KernelObj); } + void handleSyclAccessorType(const CXXBaseSpecifier &BS, + QualType ArgTy) final { + const auto *RecordDecl = ArgTy->getAsCXXRecordDecl(); + assert(RecordDecl && "The accessor/sampler must be a RecordDecl"); + CXXMethodDecl *InitMethod = getMethodByName(RecordDecl, InitMethodName); + assert(InitMethod && "The accessor/sampler must have the __init method"); + + for (const ParmVarDecl *Param : InitMethod->parameters()) + addParam(BS, Param->getType().getCanonicalType()); + } + void handleSyclAccessorType(const FieldDecl *FD, QualType ArgTy) final { handleSpecialType(FD, ArgTy); } @@ -1419,52 +1354,71 @@ class SyclKernelIntHeaderCreator : public SyclKernelFieldHandler { SYCLIntegrationHeader &Header; const CXXRecordDecl *KernelLambda; + int64_t CurOffset = 0; // Keeping track of offsets as we go along is a little awkward, so see if just // calculating each time is worth doing. Presumably, if the structure depth doesn't - // get insane, we shouldn't have a problem. - uint64_t getFieldOffsetHelper(const CXXRecordDecl *RD, const FieldDecl *FD) { + // get insane, we shouldn't have a problem with run time. + uint64_t getFieldOffset(const CXXRecordDecl *RD, const FieldDecl *FD) { + // TODO! + return 0; + } + uint64_t getBaseOffset(const CXXRecordDecl *RD, const FieldDecl *FD) { // TODO! return 0; } - uint64_t getFieldOffset(const FieldDecl *FD) const { - // TODO: Figure out a better way to do this, having to recalculate this - // constantly is going to be expensive. - // TODO: Figure out how to calc lower down the structs. - // uint64_t CurOffset = SemaRef.getASTContext().getFieldOffset(FD) / 8; + uint64_t getOffset(const CXXRecordDecl *RD) const { + // TODO: Figure this out! Offset of a base class. return 0; } + uint64_t getOffset(const FieldDecl *FD) const { + // TODO: Figure out how to calc lower down the structs, currently only gives + // the 'base' value. + return SemaRef.getASTContext().getFieldOffset(FD) / 8; + } void addParam(const FieldDecl *FD, QualType ArgTy, SYCLIntegrationHeader::kernel_param_kind_t Kind) { uint64_t Size = SemaRef.getASTContext().getTypeSizeInChars(ArgTy).getQuantity(); Header.addParamDesc(Kind, static_cast(Size), - static_cast(getFieldOffset(FD))); + static_cast(getOffset(FD))); } public: SyclKernelIntHeaderCreator(Sema &S, SYCLIntegrationHeader &H, const CXXRecordDecl *KernelLambda, - SourceLocation KernelLoc, QualType NameType, - StringRef Name, StringRef StableName) - : SyclKernelFieldHandler(S), Header(H), - KernelLambda(KernelLambda) { - Header.startKernel(Name, NameType, StableName, KernelLambda->GetLocation()); + QualType NameType, StringRef Name, + StringRef StableName) + : SyclKernelFieldHandler(S), Header(H), KernelLambda(KernelLambda) { + Header.startKernel(Name, NameType, StableName, KernelLambda->getLocation()); + } + + void handleSyclAccessorType(const CXXBaseSpecifier &BC, + QualType ArgTy) final { + const auto *AccTy = + cast(ArgTy->getAsRecordDecl()); + assert(AccTy->getTemplateArgs().size() >= 2 && + "Incorrect template args for Accessor Type"); + int Dims = static_cast( + AccTy->getTemplateArgs()[1].getAsIntegral().getExtValue()); + int Info = getAccessTarget(AccTy) | (Dims << 11); + Header.addParamDesc(SYCLIntegrationHeader::kind_accessor, Info, + // TODO: is this the right way? + getOffset(BC.getType()->getAsCXXRecordDecl())); } void handleSyclAccessorType(const FieldDecl *FD, QualType ArgTy) final { - // TODO: offset stuff is wrong again in the recursion case!? const auto *AccTy = cast(ArgTy->getAsRecordDecl()); - assert(AccTy->getTemplateArgs().size() == 2 && + assert(AccTy->getTemplateArgs().size() >= 2 && "Incorrect template args for Accessor Type"); int Dims = static_cast( AccTy->getTemplateArgs()[1].getAsIntegral().getExtValue()); int Info = getAccessTarget(AccTy) | (Dims << 11); Header.addParamDesc(SYCLIntegrationHeader::kind_accessor, Info, - getFieldOffset(FD)); + getOffset(FD)); } void handleSyclSamplerType(const FieldDecl *FD, QualType ArgTy) final { @@ -1555,9 +1509,8 @@ void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, SyclKernelBodyCreator kernel_body(*this, kernel_decl); SyclKernelIntHeaderCreator int_header( *this, getSyclIntegrationHeader(), - KernelLambda, Context.getASTRecordLayout(KernelLambda), KernelLambda->getLocation(), - calculateKernelNameType(Context, KernelCallerFunc), CalculatedName, - StableName); + KernelLambda, calculateKernelNameType(Context, KernelCallerFunc), + CalculatedName, StableName); ConstructingOpenCLKernel = true; VisitRecordFields(KernelLambda->fields(), checker, kernel_decl, kernel_body, From 936f36f8f2c31e40ead659eda6a167b0d837e27f Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Fri, 10 Apr 2020 14:49:01 -0700 Subject: [PATCH 11/17] First run at getting the offset correct... not sure if this works --- clang/lib/Sema/SemaSYCL.cpp | 82 +++++++++++++++++++++++++------------ 1 file changed, 55 insertions(+), 27 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 12ecdf2f2e3f0..111c33f92ef3d 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1050,10 +1050,6 @@ static std::string constructKernelName(Sema &S, FunctionDecl *KernelCallerFunc, // anonymous namespace so these don't get linkage. namespace { -// Implements the 'for-each-visitor' pattern. -template -static void VisitAccessorWrapper(CXXRecordDecl *Wrapper, - Handlers &... handlers); QualType getItemType(const FieldDecl *FD) { return FD->getType(); @@ -1062,25 +1058,39 @@ 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(RangeTy Range, Handlers &... handlers) { +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.handleAccessorType(Item, ItemTy), 0)...}; + (handlers.handleSyclAccessorType(Item, ItemTy), 0)...}; else if (ItemTy->isStructureOrClassType()) { - VisitAccessorWrapper(ItemTy->getAsCXXRecordDecl()); + VisitAccessorWrapper(Owner, Item, ItemTy->getAsCXXRecordDecl(), + handlers...); } } } -template -static void VisitAccessorWrapper(CXXRecordDecl *Wrapper, - Handlers &... handlers) { - - VisitAccessorWrapperHelper(Wrapper->bases(), handlers...); - VisitAccessorWrapperHelper(Wrapper->fields(), handlers...); +// poorly named Parent is the 'how we got here', basically just enough info for +// the offset adjustment to know what to do about the enter-struct info. +template +static void VisitAccessorWrapper(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...); + (void)std::initializer_list{ + (handlers.leaveStruct(Owner, Parent), 0)...}; } // A visitor function that dispatches to functions as defined in @@ -1103,7 +1113,7 @@ static void VisitRecordFields(RecordDecl::field_range Fields, else if (FieldTy->isStructureOrClassType()) { KF_FOR_EACH(handleStructType); CXXRecordDecl *RD = FieldTy->getAsCXXRecordDecl(); - VisitAccessorWrapper(RD); + VisitAccessorWrapper(nullptr, Field, RD, handlers...); } else if (FieldTy->isReferenceType()) KF_FOR_EACH(handleReferenceType); else if (FieldTy->isPointerType()) @@ -1145,6 +1155,17 @@ template class SyclKernelFieldHandler { virtual void handleScalarType(const FieldDecl *, QualType) {} // Most handlers shouldn't be handling this, just the field checker. virtual void handleOtherType(const FieldDecl *, QualType) {} + + // The following are only used for keeping track of where we are in the base + // class/field graph. Int Headers use this to calculate offset, most others + // don't have a need for these. + + virtual void enterStruct(const CXXRecordDecl *, const FieldDecl *) {} + virtual void leaveStruct(const CXXRecordDecl *, const FieldDecl *) {} + virtual void enterStruct(const CXXRecordDecl *, const CXXBaseSpecifier &) {} + virtual void leaveStruct(const CXXRecordDecl *, const CXXBaseSpecifier &) {} + // virtual void enterStruct(const FieldDecl *, CXXRecordDecl *Struct); + // virtual void leaveStruct(const FieldDecl *, CXXRecordDecl *Struct); }; // A type to check the valididty of all of the argument types. @@ -1356,18 +1377,6 @@ class SyclKernelIntHeaderCreator const CXXRecordDecl *KernelLambda; int64_t CurOffset = 0; - // Keeping track of offsets as we go along is a little awkward, so see if just - // calculating each time is worth doing. Presumably, if the structure depth doesn't - // get insane, we shouldn't have a problem with run time. - uint64_t getFieldOffset(const CXXRecordDecl *RD, const FieldDecl *FD) { - // TODO! - return 0; - } - uint64_t getBaseOffset(const CXXRecordDecl *RD, const FieldDecl *FD) { - // TODO! - return 0; - } - uint64_t getOffset(const CXXRecordDecl *RD) const { // TODO: Figure this out! Offset of a base class. return 0; @@ -1375,7 +1384,7 @@ class SyclKernelIntHeaderCreator uint64_t getOffset(const FieldDecl *FD) const { // TODO: Figure out how to calc lower down the structs, currently only gives // the 'base' value. - return SemaRef.getASTContext().getFieldOffset(FD) / 8; + return CurOffset + SemaRef.getASTContext().getFieldOffset(FD) / 8; } void addParam(const FieldDecl *FD, QualType ArgTy, @@ -1462,7 +1471,26 @@ class SyclKernelIntHeaderCreator addParam(FD, ArgTy, SYCLIntegrationHeader::kind_std_layout); } + // Keep track of the current struct offset. + void enterStruct(const CXXRecordDecl *, const FieldDecl *FD) final { + CurOffset += SemaRef.getASTContext().getFieldOffset(FD) / 8; + } + + void leaveStruct(const CXXRecordDecl *, const FieldDecl *FD) final { + CurOffset -= SemaRef.getASTContext().getFieldOffset(FD) / 8; + } + void enterStruct(const CXXRecordDecl *RD, const CXXBaseSpecifier &BS) final { + const ASTRecordLayout &Layout = + SemaRef.getASTContext().getASTRecordLayout(RD); + CurOffset += Layout.getBaseClassOffset(BS.getType()); + } + + void leaveStruct(const CXXRecordDecl *RD, const CXXBaseSpecifier &BS) final { + const ASTRecordLayout &Layout = + SemaRef.getASTContext().getASTRecordLayout(RD); + CurOffset -= Layout.getBaseClassOffset(BS.getType()); + } }; } // namespace From 128f7b1b69c6c9164792ccc00d3441666787d455 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Fri, 10 Apr 2020 14:56:51 -0700 Subject: [PATCH 12/17] Got the offset logic compiling/looking right. Also did a clang format. Quitting for the weekend, pushing so that others can have a look at the progress/help how they can --- clang/lib/Sema/SemaSYCL.cpp | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 111c33f92ef3d..a318d879d504c 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1051,12 +1051,8 @@ static std::string 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(); -} +QualType getItemType(const FieldDecl *FD) { return FD->getType(); } +QualType getItemType(const CXXBaseSpecifier &BS) { return BS.getType(); } // Implements the 'for-each-visitor' pattern. template @@ -1085,12 +1081,10 @@ template static void VisitAccessorWrapper(CXXRecordDecl *Owner, ParentTy &Parent, CXXRecordDecl *Wrapper, Handlers &... handlers) { - (void)std::initializer_list{ - (handlers.enterStruct(Owner, Parent), 0)...}; + (void)std::initializer_list{(handlers.enterStruct(Owner, Parent), 0)...}; VisitAccessorWrapperHelper(Wrapper, Wrapper->bases(), handlers...); VisitAccessorWrapperHelper(Wrapper, Wrapper->fields(), handlers...); - (void)std::initializer_list{ - (handlers.leaveStruct(Owner, Parent), 0)...}; + (void)std::initializer_list{(handlers.leaveStruct(Owner, Parent), 0)...}; } // A visitor function that dispatches to functions as defined in @@ -1263,7 +1257,6 @@ class SyclKernelDeclCreator addParam(FD, Param->getType().getCanonicalType()); } - static void setKernelImplicitAttrs(ASTContext &Context, FunctionDecl *FD, StringRef Name) { // Set implict attributes. @@ -1483,14 +1476,16 @@ class SyclKernelIntHeaderCreator void enterStruct(const CXXRecordDecl *RD, const CXXBaseSpecifier &BS) final { const ASTRecordLayout &Layout = SemaRef.getASTContext().getASTRecordLayout(RD); - CurOffset += Layout.getBaseClassOffset(BS.getType()); + CurOffset += Layout.getBaseClassOffset(BS.getType()->getAsCXXRecordDecl()) + .getQuantity(); } void leaveStruct(const CXXRecordDecl *RD, const CXXBaseSpecifier &BS) final { const ASTRecordLayout &Layout = SemaRef.getASTContext().getASTRecordLayout(RD); - CurOffset -= Layout.getBaseClassOffset(BS.getType()); - } + CurOffset -= Layout.getBaseClassOffset(BS.getType()->getAsCXXRecordDecl()) + .getQuantity(); + } }; } // namespace From 645c513d166ca38d170117c4fc290285b71385d1 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Fri, 10 Apr 2020 15:07:32 -0700 Subject: [PATCH 13/17] Format the createOpenCLKernel func, remove int header from comment --- clang/lib/Sema/SemaSYCL.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index a318d879d504c..97fccbbb4d4bc 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1531,9 +1531,9 @@ void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, KernelCallerFunc->isInlined()); SyclKernelBodyCreator kernel_body(*this, kernel_decl); SyclKernelIntHeaderCreator int_header( - *this, getSyclIntegrationHeader(), - KernelLambda, calculateKernelNameType(Context, KernelCallerFunc), - CalculatedName, StableName); + *this, getSyclIntegrationHeader(), KernelLambda, + calculateKernelNameType(Context, KernelCallerFunc), CalculatedName, + StableName); ConstructingOpenCLKernel = true; VisitRecordFields(KernelLambda->fields(), checker, kernel_decl, kernel_body, @@ -1541,9 +1541,6 @@ void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, ConstructingOpenCLKernel = false; /* - // TODO Maybe don't emit integration header inside the Sema? - ***populateIntHeader(getSyclIntegrationHeader(), Name, KernelNameType, LE); - //ConstructingOpenCLKernel = true; ****CompoundStmt *OpenCLKernelBody = CreateOpenCLKernelBody(*this, KernelCallerFunc, OpenCLKernel); From 7682da8ed33861cf1e78b3e5d33cb4f685b5e870 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Mon, 13 Apr 2020 05:58:39 -0700 Subject: [PATCH 14/17] Rename kernelobj to kerneldecl Signed-off-by: Erich Keane --- clang/lib/Sema/SemaSYCL.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 97fccbbb4d4bc..94bdbdc1fef73 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1215,7 +1215,7 @@ class SyclKernelFieldChecker // A type to Create and own the FunctionDecl for the kernel. class SyclKernelDeclCreator : public SyclKernelFieldHandler { - FunctionDecl *KernelObj; + FunctionDecl *KernelDecl; llvm::SmallVector Params; SyclKernelFieldChecker &ArgChecker; Sema::ContextRAII FuncContext; @@ -1233,7 +1233,7 @@ class SyclKernelDeclCreator void addParam(ParamDesc newParamDesc, QualType ArgTy) { // Create a new ParmVarDecl based on the new info. auto *NewParam = ParmVarDecl::Create( - SemaRef.getASTContext(), KernelObj, SourceLocation(), SourceLocation(), + SemaRef.getASTContext(), KernelDecl, SourceLocation(), SourceLocation(), std::get<1>(newParamDesc), std::get<0>(newParamDesc), std::get<2>(newParamDesc), SC_None, /*DefArg*/ nullptr); @@ -1287,8 +1287,8 @@ class SyclKernelDeclCreator SyclKernelDeclCreator(Sema &S, SyclKernelFieldChecker &ArgChecker, StringRef Name, SourceLocation Loc, bool IsInline) : SyclKernelFieldHandler(S), - KernelObj(createKernelDecl(S.getASTContext(), Name, Loc, IsInline)), - ArgChecker(ArgChecker), FuncContext(SemaRef, KernelObj) {} + KernelDecl(createKernelDecl(S.getASTContext(), Name, Loc, IsInline)), + ArgChecker(ArgChecker), FuncContext(SemaRef, KernelDecl) {} ~SyclKernelDeclCreator() { ASTContext &Ctx = SemaRef.getASTContext(); @@ -1300,11 +1300,11 @@ class SyclKernelDeclCreator [](const ParmVarDecl *PVD) { return PVD->getType(); }); QualType FuncType = Ctx.getFunctionType(Ctx.VoidTy, ArgTys, Info); - KernelObj->setType(FuncType); - KernelObj->setParams(Params); + KernelDecl->setType(FuncType); + KernelDecl->setParams(Params); if (ArgChecker.isValid()) - SemaRef.addSyclDeviceDecl(KernelObj); + SemaRef.addSyclDeviceDecl(KernelDecl); } void handleSyclAccessorType(const CXXBaseSpecifier &BS, @@ -1349,7 +1349,7 @@ class SyclKernelDeclCreator addParam(FD, ArgTy); } - void setBody(CompoundStmt *KB) { KernelObj->setBody(KB); } + void setBody(CompoundStmt *KB) { KernelDecl->setBody(KB); } }; class SyclKernelBodyCreator From 7735dc87761af4330fff2ea335b653f85fedcab7 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Mon, 13 Apr 2020 08:28:30 -0700 Subject: [PATCH 15/17] Framwork for body, added the stream type as a special type Signed-off-by: Erich Keane --- clang/lib/Sema/SemaSYCL.cpp | 73 ++++++++++++++++++++++++++++++++++--- 1 file changed, 67 insertions(+), 6 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 94bdbdc1fef73..8a5631bf69f3c 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -969,6 +969,7 @@ static CompoundStmt *CreateOpenCLKernelBody(Sema &S, // need to replace all refs to this kernel oject with refs to our clone // declared inside kernel body. Stmt *FunctionBody = KernelCallerFunc->getBody(); + ParmVarDecl *KernelObjParam = *(KernelCallerFunc->param_begin()); // DeclRefExpr with valid source location but with decl which is not marked @@ -1068,6 +1069,9 @@ static void VisitAccessorWrapperHelper(CXXRecordDecl *Owner, RangeTy Range, if (Util::isSyclAccessorType(ItemTy)) (void)std::initializer_list{ (handlers.handleSyclAccessorType(Item, ItemTy), 0)...}; + else if (Util::isSyclStreamType(ItemTy)) + (void)std::initializer_list{ + (handlers.handleSyclStreamType(Item, ItemTy), 0)...}; else if (ItemTy->isStructureOrClassType()) { VisitAccessorWrapper(Owner, Item, ItemTy->getAsCXXRecordDecl(), handlers...); @@ -1104,6 +1108,8 @@ static void VisitRecordFields(RecordDecl::field_range Fields, KF_FOR_EACH(handleSyclSamplerType); else if (Util::isSyclSpecConstantType(FieldTy)) KF_FOR_EACH(handleSyclSpecConstantType); + else if (Util::isSyclStreamType(FieldTy)) + KF_FOR_EACH(handleSyclStreamType); else if (FieldTy->isStructureOrClassType()) { KF_FOR_EACH(handleStructType); CXXRecordDecl *RD = FieldTy->getAsCXXRecordDecl(); @@ -1142,6 +1148,7 @@ template class SyclKernelFieldHandler { virtual void handleSyclAccessorType(const FieldDecl *, QualType) {} virtual void handleSyclSamplerType(const FieldDecl *, QualType) {} virtual void handleSyclSpecConstantType(const FieldDecl *, QualType) {} + virtual void handleSyclStreamType(const FieldDecl *, QualType) {} virtual void handleStructType(const FieldDecl *, QualType) {} virtual void handleReferenceType(const FieldDecl *, QualType) {} virtual void handlePointerType(const FieldDecl *, QualType) {} @@ -1350,18 +1357,71 @@ class SyclKernelDeclCreator } void setBody(CompoundStmt *KB) { KernelDecl->setBody(KB); } + + FunctionDecl *getKernelDecl() { return KernelDecl; } }; class SyclKernelBodyCreator : public SyclKernelFieldHandler { SyclKernelDeclCreator &DeclCreator; - // TODO: When/Where does this get created? - CompoundStmt *KernelBody = nullptr; + llvm::SmallVector BodyStmts; + llvm::SmallVector FinalizeStmts; + llvm::SmallVector InitExprs; + + // Using the statements/init expressions that we've created, this generates + // the kernel body compound stmt. CompoundStmt needs to know its number of + // statements in advance to allocate it, so we cannot do this as we go along. + CompountStmt *createKernelBody() { + // TODO: Can we hold off on creating KernelObjClone to here? + + Expr *ILE = new (SemaRef.getASTContext()) + InitListExpr(S.Context, SourceLocation(), InitExprs, SourceLocation()); + // TODO!!! ILE->setType(QualType(LC->getTypeForDecl(), 0)); + // KernelObjectClone->setInit(ILE); + + // TODO: More kernel object init with KernelBodyTransform. + + BodyStmts.insert(std::end(BodyStmts), std::begin(FinalizeStmts), + std::begin(FinalizeStmts)); + return CompoundStmt::Create(SemaRef.getASTContext(), BodyStmts, {}, {}); + } + + // TODO: not sure what this does yet, name is a placeholder for future use. + void doSomethingForParallelForWorkGroup() { + } public: - SyclKernelBodyCreator(Sema &S, SyclKernelDeclCreator &DC) - : SyclKernelFieldHandler(S), DeclCreator(DC) {} - ~SyclKernelBodyCreator() { DeclCreator.setBody(KernelBody); } + SyclKernelBodyCreator(Sema &S, SyclKernelDeclCreator &DC, KernelInvocationKind K) + : SyclKernelFieldHandler(S), DeclCreator(DC) { + // TODO: Something special with the lambda when InvokeParallelForWorkGroup. + if (K == InvokeParallelForWorkGroup) + do somethingForparalellForWorkGroup(); + } + ~SyclKernelBodyCreator() { + CompoundStmt *KernelBody = createKernelBody(); + DeclCreator.setBody(KernelBody); + } + + void handleSyclAccessorType(FieldDecl *FD, QualType Ty) final { + // TODO: Creates init sequence and inits special sycl obj + } + + void handleSyclSamplerType(FieldDecl *FD, QualType Ty) final { + // TODO: Creates init sequence and inits special sycl obj + } + + void handleSyclStreamType(FieldDecl *FD, QualType Ty) final { + // TODO: Creates init/finalize sequence and inits special sycl obj + } + + void handleStructType(FieldDecl *FD, QualType Ty) final { + // TODO: a bunch of work doing inits, note this has a little more than + // scalar. + } + void handleScalarType(FieldDecl *FD, QualType Ty) final { + // TODO: a bunch of work doing inits. + } + }; class SyclKernelIntHeaderCreator @@ -1529,7 +1589,8 @@ void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc, SyclKernelDeclCreator kernel_decl(*this, checker, KernelName, KernelLambda->getLocation(), KernelCallerFunc->isInlined()); - SyclKernelBodyCreator kernel_body(*this, kernel_decl); + SyclKernelBodyCreator kernel_body(*this, kernel_decl, + getKernelInvocationKind(KernelCallerFunc)); SyclKernelIntHeaderCreator int_header( *this, getSyclIntegrationHeader(), KernelLambda, calculateKernelNameType(Context, KernelCallerFunc), CalculatedName, From a779ab2dda811b37873a155d758ee5961e5798eb Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Mon, 13 Apr 2020 08:36:09 -0700 Subject: [PATCH 16/17] Fix all the build errors from the last patch Signed-off-by: Erich Keane --- clang/lib/Sema/SemaSYCL.cpp | 39 ++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 8a5631bf69f3c..d8a04d6d85d9e 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1148,6 +1148,7 @@ template class SyclKernelFieldHandler { virtual void handleSyclAccessorType(const FieldDecl *, QualType) {} virtual void handleSyclSamplerType(const FieldDecl *, QualType) {} virtual void handleSyclSpecConstantType(const FieldDecl *, QualType) {} + virtual void handleSyclStreamType(const CXXBaseSpecifier &, QualType) {} virtual void handleSyclStreamType(const FieldDecl *, QualType) {} virtual void handleStructType(const FieldDecl *, QualType) {} virtual void handleReferenceType(const FieldDecl *, QualType) {} @@ -1371,11 +1372,11 @@ class SyclKernelBodyCreator // Using the statements/init expressions that we've created, this generates // the kernel body compound stmt. CompoundStmt needs to know its number of // statements in advance to allocate it, so we cannot do this as we go along. - CompountStmt *createKernelBody() { + CompoundStmt *createKernelBody() { // TODO: Can we hold off on creating KernelObjClone to here? - Expr *ILE = new (SemaRef.getASTContext()) - InitListExpr(S.Context, SourceLocation(), InitExprs, SourceLocation()); + Expr *ILE = new (SemaRef.getASTContext()) InitListExpr( + SemaRef.getASTContext(), SourceLocation(), InitExprs, SourceLocation()); // TODO!!! ILE->setType(QualType(LC->getTypeForDecl(), 0)); // KernelObjectClone->setInit(ILE); @@ -1391,37 +1392,47 @@ class SyclKernelBodyCreator } public: - SyclKernelBodyCreator(Sema &S, SyclKernelDeclCreator &DC, KernelInvocationKind K) + SyclKernelBodyCreator(Sema &S, SyclKernelDeclCreator &DC, + KernelInvocationKind K) : SyclKernelFieldHandler(S), DeclCreator(DC) { - // TODO: Something special with the lambda when InvokeParallelForWorkGroup. - if (K == InvokeParallelForWorkGroup) - do somethingForparalellForWorkGroup(); - } + // TODO: Something special with the lambda when InvokeParallelForWorkGroup. + if (K == InvokeParallelForWorkGroup) + doSomethingForParallelForWorkGroup(); + } ~SyclKernelBodyCreator() { CompoundStmt *KernelBody = createKernelBody(); DeclCreator.setBody(KernelBody); } - void handleSyclAccessorType(FieldDecl *FD, QualType Ty) final { + void handleSyclAccessorType(const FieldDecl *FD, QualType Ty) final { + // TODO: Creates init sequence and inits special sycl obj + } + + void handleSyclAccessorType(const CXXBaseSpecifier &BS, QualType Ty) final { // TODO: Creates init sequence and inits special sycl obj } - void handleSyclSamplerType(FieldDecl *FD, QualType Ty) final { + + void handleSyclSamplerType(const FieldDecl *FD, QualType Ty) final { // TODO: Creates init sequence and inits special sycl obj } - void handleSyclStreamType(FieldDecl *FD, QualType Ty) final { + void handleSyclStreamType(const FieldDecl *FD, QualType Ty) final { + // TODO: Creates init/finalize sequence and inits special sycl obj + } + + void handleSyclStreamType(const CXXBaseSpecifier &BS, QualType Ty) final { // TODO: Creates init/finalize sequence and inits special sycl obj } - void handleStructType(FieldDecl *FD, QualType Ty) final { + + void handleStructType(const FieldDecl *FD, QualType Ty) final { // TODO: a bunch of work doing inits, note this has a little more than // scalar. } - void handleScalarType(FieldDecl *FD, QualType Ty) final { + void handleScalarType(const FieldDecl *FD, QualType Ty) final { // TODO: a bunch of work doing inits. } - }; class SyclKernelIntHeaderCreator From 30d446bbd3836fb7d2da7124b5c12fef605c910d Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Mon, 13 Apr 2020 11:11:41 -0700 Subject: [PATCH 17/17] Add getParamVarDeclsForCurrentField, which provides a way for the body creator to get the list of parameters for the currently handled field Signed-off-by: Erich Keane --- clang/lib/Sema/SemaSYCL.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index d8a04d6d85d9e..30ea342073209 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1227,6 +1227,9 @@ class SyclKernelDeclCreator llvm::SmallVector Params; SyclKernelFieldChecker &ArgChecker; Sema::ContextRAII FuncContext; + // Holds the last handled field's first parameter. This doesn't store an + // iterator as push_back invalidates iterators. + size_t LastParamIndex = 0; void addParam(const FieldDecl *FD, QualType ArgTy) { ParamDesc newParamDesc = makeParamDesc(FD, ArgTy); @@ -1248,6 +1251,7 @@ class SyclKernelDeclCreator NewParam->setScopeInfo(0, Params.size()); NewParam->setIsUsed(); + LastParamIndex = Params.size(); Params.push_back(NewParam); } @@ -1261,8 +1265,12 @@ class SyclKernelDeclCreator CXXMethodDecl *InitMethod = getMethodByName(RecordDecl, InitMethodName); assert(InitMethod && "The accessor/sampler must have the __init method"); + // Don't do -1 here because we count on this to be the first parameter added + // (if any). + size_t ParamIndex = Params.size(); for (const ParmVarDecl *Param : InitMethod->parameters()) addParam(FD, Param->getType().getCanonicalType()); + LastParamIndex = ParamIndex; } static void setKernelImplicitAttrs(ASTContext &Context, FunctionDecl *FD, @@ -1322,8 +1330,12 @@ class SyclKernelDeclCreator CXXMethodDecl *InitMethod = getMethodByName(RecordDecl, InitMethodName); assert(InitMethod && "The accessor/sampler must have the __init method"); + // Don't do -1 here because we count on this to be the first parameter added + // (if any). + size_t ParamIndex = Params.size(); for (const ParmVarDecl *Param : InitMethod->parameters()) addParam(BS, Param->getType().getCanonicalType()); + LastParamIndex = ParamIndex; } void handleSyclAccessorType(const FieldDecl *FD, QualType ArgTy) final { @@ -1360,6 +1372,11 @@ class SyclKernelDeclCreator void setBody(CompoundStmt *KB) { KernelDecl->setBody(KB); } FunctionDecl *getKernelDecl() { return KernelDecl; } + + llvm::ArrayRef getParamVarDeclsForCurrentField() { + return ArrayRef(std::begin(Params) + LastParamIndex, + std::end(Params)); + } }; class SyclKernelBodyCreator