From 3b328fa21123e10c85e11dde1d53684228c8051a Mon Sep 17 00:00:00 2001 From: Alexey Bader Date: Sun, 26 May 2019 13:35:51 +0300 Subject: [PATCH 1/3] [SYCL][NFC] Remove code from comments. Removed comments are redundant and complicate maintenance. Signed-off-by: Alexey Bader --- clang/lib/Sema/SemaSYCL.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index b4a9369d0df0d..2d491709f966e 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -809,7 +809,6 @@ static void buildArgTys(ASTContext &Context, CXXRecordDecl *KernelObj, assert(InitMethod && "sampler must have __init method"); // sampler __init method has only one parameter - // void __init(__ocl_sampler_t *Sampler) auto *FuncDecl = cast(InitMethod); ParmVarDecl *SamplerArg = FuncDecl->getParamDecl(0); assert(SamplerArg && "sampler __init method must have sampler parameter"); @@ -912,7 +911,6 @@ static void populateIntHeader(SYCLIntegrationHeader &H, const StringRef Name, assert(InitMethod && "sampler must have __init method"); // sampler __init method has only one argument - // void __init(__ocl_sampler_t *Sampler) auto *FuncDecl = cast(InitMethod); ParmVarDecl *SamplerArg = FuncDecl->getParamDecl(0); assert(SamplerArg && "sampler __init method must have sampler parameter"); From 809cdc3302f3519f0aa8530a0860e1573965b45a Mon Sep 17 00:00:00 2001 From: Alexey Bader Date: Fri, 24 May 2019 14:15:20 +0300 Subject: [PATCH 2/3] [SYCL] Update documentation with OpenCL types re-use proposal Signed-off-by: Alexey Bader --- sycl/doc/SYCL_compiler_and_runtime_design.md | 64 ++++++++------------ 1 file changed, 25 insertions(+), 39 deletions(-) diff --git a/sycl/doc/SYCL_compiler_and_runtime_design.md b/sycl/doc/SYCL_compiler_and_runtime_design.md index d7c3cc5e5ce8e..cc581a744e567 100644 --- a/sycl/doc/SYCL_compiler_and_runtime_design.md +++ b/sycl/doc/SYCL_compiler_and_runtime_design.md @@ -291,17 +291,28 @@ produced by OpenCL C front-end compiler. It's a regular function, which can conflict with user code produced from C++ source. - -SYCL compiler uses solution developed for OpenCL C++ compiler prototype: +SYCL compiler uses modified solution developed for OpenCL C++ compiler +prototype: - Compiler: https://github.com/KhronosGroup/SPIR/tree/spirv-1.1 - Headers: https://github.com/KhronosGroup/libclcxx -SPIR-V types and operations that do not have LLVM equivalents are **declared** +Our solution re-uses OpenCL data types like sampler, event, image types, etc, +but we use different spelling to avoid potential conflicts with C++ code. +Spelling convention for the OpenCL types enabled in SYCL mode is: + +``` +__ocl_ // e.g. __ocl_sampler_t, __ocl_event_t +``` + +Operations using OpenCL types use special naming convention described in this +[document](https://github.com/KhronosGroup/SPIRV-LLVM-Translator/blob/master/docs/SPIRVRepresentationInLLVM.rst). +This solution allows us avoid SYCL specialization in SPIR-V translator and +leverage clang infrastructure developed for OpenCL types. + +SPIR-V operations that do not have LLVM equivalents are **declared** (but not defined) in the headers and satisfy following requirements: -- the type must be pre-declared as a C++ class in `cl::__spirv` namespace -- the type must not have actual definition in C++ program - the operation is expressed in C++ as `extern` function not throwing C++ exceptions - the operation must not have the actual definition in C++ program @@ -310,42 +321,17 @@ For example, the following C++ code is successfully recognized and translated into SPIR-V operation `OpGroupAsyncCopy`: ```C++ -namespace cl { - namespace __spirv { - // This class does not have definition, it is only predeclared here. - // The pointers to this class objects can be passed to or returned from - // SPIR-V built-in functions. Only in such cases the class is recognized - // as SPIR-V type OpTypeEvent. - class OpTypeEvent; - - template - extern OpTypeEvent *OpGroupAsyncCopy(int32_t Scope, __local dataT *Dest, - __global dataT *Src, size_t NumElements, - size_t Stride, OpTypeEvent *E) noexcept; - } // namespace __spirv -} // namespace cl - -cl::__spirv::OpTypeEvent *e = - cl::__spirv::OpGroupAsyncCopy(cl::__spirv::Scope::Workgroup, - dst, src, numElements, 1, 0); +template +extern __ocl_event_t +__spirv_OpGroupAsyncCopy(int32_t Scope, __local dataT *Dest, + __global dataT *Src, size_t NumElements, + size_t Stride, __ocl_event_t E) noexcept; + +__ocl_event_t e = + __spirv_OpGroupAsyncCopy(cl::__spirv::Scope::Workgroup, + dst, src, numElements, 1, E); ``` -OpenCL C++ compiler uses a special module pass in clang that transforms the -names of C++ classes, globals and functions from the namespace `cl::__spirv::` -to -["SPIR-V representation in LLVM IR"](https://github.com/KhronosGroup/SPIRV-LLVM-Translator/blob/master/docs/SPIRVRepresentationInLLVM.rst) -which is recognized by the LLVM IR to SPIR-V translator. - -In the OpenCL C++ prototype project the pass is located at the directory: -`lib/CodeGen/OclCxxRewrite`. The file with the pass is: -`lib/CodeGen/OclCxxRewrite/BifNameReflower.cpp`. The other files in -`lib/CodeGen/OclCxxRewrite` are utility files implementing Itanium demangler -and other helping functionality. - -That LLVM module pass has been ported from OpenCL C++ prototype to the SYCL -compiler as is. It made possible using simple declarations of C++ classes and -external functions as if they were the SPIR-V specific types and operations. - #### Some details and agreements on using SPIR-V special types and operations The SPIR-V specific C++ enumerators and classes are declared in the file: From ec5e1b3c65bcd20521b84b8759ef765c0b8c494e Mon Sep 17 00:00:00 2001 From: Alexey Bader Date: Tue, 21 May 2019 14:45:25 +0300 Subject: [PATCH 3/3] [SYCL] Enable OpenCL diagnostics for sampler in SYCL mode Selectively enabled a few OpenCL diagnostics for sampler type as some diagnostics trigger on SYCL use cases. For instance, OpenCL kernel in SYCL mode initializes lambda captures with the kernel argument values. This initialization code for sampler emits errors because OpenCL disallows sampler on the right hand side of the binary operators - these are supposed to be used only by built-in functions. Another potential issue is the lambda object itself - captured sampler is a member of the lambda object and OpenCL doesn't allow composite types with samplers. SPIR-V produced from SYCL should be okay as lambda object can be removed by standard LLVM transformation passes. Signed-off-by: Alexey Bader --- clang/lib/Sema/SemaDecl.cpp | 40 ++++++++++++++++++------------------- clang/lib/Sema/SemaExpr.cpp | 2 +- clang/lib/Sema/SemaInit.cpp | 10 +++++----- clang/lib/Sema/SemaType.cpp | 8 +++++--- 4 files changed, 31 insertions(+), 29 deletions(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index a035f200fce61..e28323b66bc52 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -6347,6 +6347,26 @@ NamedDecl *Sema::ActOnVariableDeclarator( return nullptr; } + if (R->isSamplerT()) { + // OpenCL v1.2 s6.9.b p4: + // The sampler type cannot be used with the __local and __global address + // space qualifiers. + if (R.getAddressSpace() == LangAS::opencl_local || + R.getAddressSpace() == LangAS::opencl_global) { + Diag(D.getIdentifierLoc(), diag::err_wrong_sampler_addressspace); + } + + // OpenCL v1.2 s6.12.14.1: + // A global sampler must be declared with either the constant address + // space qualifier or with the const qualifier. + if (DC->isTranslationUnit() && + !(R.getAddressSpace() == LangAS::opencl_constant || + R.isConstQualified())) { + Diag(D.getIdentifierLoc(), diag::err_opencl_nonconst_global_sampler); + D.setInvalidType(); + } + } + if (getLangOpts().OpenCL) { // OpenCL v2.0 s6.9.b - Image type can only be used as a function argument. // OpenCL v2.0 s6.13.16.1 - Pipe type can only be used as a function @@ -6392,26 +6412,6 @@ NamedDecl *Sema::ActOnVariableDeclarator( } } - if (R->isSamplerT()) { - // OpenCL v1.2 s6.9.b p4: - // The sampler type cannot be used with the __local and __global address - // space qualifiers. - if (R.getAddressSpace() == LangAS::opencl_local || - R.getAddressSpace() == LangAS::opencl_global) { - Diag(D.getIdentifierLoc(), diag::err_wrong_sampler_addressspace); - } - - // OpenCL v1.2 s6.12.14.1: - // A global sampler must be declared with either the constant address - // space qualifier or with the const qualifier. - if (DC->isTranslationUnit() && - !(R.getAddressSpace() == LangAS::opencl_constant || - R.isConstQualified())) { - Diag(D.getIdentifierLoc(), diag::err_opencl_nonconst_global_sampler); - D.setInvalidType(); - } - } - // OpenCL v1.2 s6.9.r: // The event type cannot be used with the __local, __constant and __global // address space qualifiers. diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index fbbdc666dc979..4c8788003251e 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -13046,7 +13046,7 @@ ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc, bool CanOverflow = false; bool ConvertHalfVec = false; - if (getLangOpts().OpenCL) { + if (getLangOpts().OpenCL || getLangOpts().SYCLIsDevice) { QualType Ty = InputExpr->getType(); // The only legal unary operation for atomics is '&'. if ((Opc != UO_AddrOf && Ty->isAtomicType()) || diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 45456aff364dd..593946fffa2a8 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -5319,9 +5319,9 @@ static bool TryOCLSamplerInitialization(Sema &S, InitializationSequence &Sequence, QualType DestType, Expr *Initializer) { - if (!S.getLangOpts().OpenCL || !DestType->isSamplerT() || + if (!DestType->isSamplerT() || (!Initializer->isIntegerConstantExpr(S.Context) && - !Initializer->getType()->isSamplerT())) + !Initializer->getType()->isSamplerT())) return false; Sequence.AddOCLSamplerInitStep(DestType); @@ -5634,6 +5634,9 @@ void InitializationSequence::InitializeFrom(Sema &S, bool allowObjCWritebackConversion = S.getLangOpts().ObjCAutoRefCount && Entity.isParameterKind(); + if (TryOCLSamplerInitialization(S, *this, DestType, Initializer)) + return; + // We're at the end of the line for C: it's either a write-back conversion // or it's a C assignment. There's no need to check anything else. if (!S.getLangOpts().CPlusPlus) { @@ -5643,9 +5646,6 @@ void InitializationSequence::InitializeFrom(Sema &S, return; } - if (TryOCLSamplerInitialization(S, *this, DestType, Initializer)) - return; - if (TryOCLZeroOpaqueTypeInitialization(S, *this, DestType, Initializer)) return; diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index f15c06f5b3612..1092cc53ad8ad 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -2322,7 +2322,7 @@ QualType Sema::BuildArrayType(QualType T, ArrayType::ArraySizeModifier ASM, // OpenCL v2.0 s6.12.5 - Arrays of blocks are not supported. // OpenCL v2.0 s6.16.13.1 - Arrays of pipe type are not supported. // OpenCL v2.0 s6.9.b - Arrays of image/sampler type are not supported. - if (getLangOpts().OpenCL) { + if (getLangOpts().OpenCL || getLangOpts().SYCLIsDevice) { const QualType ArrType = Context.getBaseElementType(T); if (ArrType->isBlockPointerType() || ArrType->isPipeType() || ArrType->isSamplerT() || ArrType->isImageType()) { @@ -4409,7 +4409,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state, // OpenCL v2.0 s6.9b - Pointer to image/sampler cannot be used. // OpenCL v2.0 s6.13.16.1 - Pointer to pipe cannot be used. // OpenCL v2.0 s6.12.5 - Pointers to Blocks are not allowed. - if (LangOpts.OpenCL) { + if (LangOpts.OpenCL || LangOpts.SYCLIsDevice) { if (T->isImageType() || T->isSamplerT() || T->isPipeType() || T->isBlockPointerType()) { S.Diag(D.getIdentifierLoc(), diag::err_opencl_pointer_to_type) << T; @@ -4607,7 +4607,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state, } } - if (LangOpts.OpenCL) { + if (LangOpts.OpenCL || LangOpts.SYCLIsDevice) { // OpenCL v2.0 s6.12.5 - A block cannot be the return value of a // function. if (T->isBlockPointerType() || T->isImageType() || T->isSamplerT() || @@ -4619,7 +4619,9 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state, // OpenCL doesn't support variadic functions and blocks // (s6.9.e and s6.12.5 OpenCL v2.0) except for printf. // We also allow here any toolchain reserved identifiers. + // FIXME: Use deferred diagnostics engine to skip host side issues. if (FTI.isVariadic && + !LangOpts.SYCLIsDevice && !(D.getIdentifier() && ((D.getIdentifier()->getName() == "printf" && (LangOpts.OpenCLCPlusPlus || LangOpts.OpenCLVersion >= 120)) ||