From 2bae2bb205f704404b2073553f4086ffb4ba6c4c Mon Sep 17 00:00:00 2001 From: James Brodman Date: Wed, 26 Jun 2019 13:46:10 -0400 Subject: [PATCH 1/5] Modify clang to: 1) Support pointer types in Integration Header 2) Put kernel pointer params in the global addr space 3) Fix CheckSYCLType to support proper recursion through pointers Signed-off-by: James Brodman --- clang/include/clang/Sema/Sema.h | 3 +- clang/lib/Sema/SemaSYCL.cpp | 35 +++++++++++++++++++---- clang/test/CodeGenSYCL/usm-int-header.cpp | 33 +++++++++++++++++++++ clang/test/SemaSYCL/usm-params.cpp | 32 +++++++++++++++++++++ 4 files changed, 97 insertions(+), 6 deletions(-) create mode 100644 clang/test/CodeGenSYCL/usm-int-header.cpp create mode 100644 clang/test/SemaSYCL/usm-params.cpp diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 8d021f7e00d49..fcd345c6a6721 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -301,7 +301,8 @@ class SYCLIntegrationHeader { kind_accessor = kind_first, kind_std_layout, kind_sampler, - kind_last = kind_sampler + kind_pointer, + kind_last = kind_pointer }; public: diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 516d37dc2b05d..eb612651d60f4 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -331,6 +331,11 @@ class MarkDeviceFunction : public RecursiveASTVisitor { private: bool CheckSYCLType(QualType Ty, SourceRange Loc) { + llvm::DenseSet visited; + return CheckSYCLType(Ty, Loc, visited); + } + + bool CheckSYCLType(QualType Ty, SourceRange Loc, llvm::DenseSet &Visited) { if (Ty->isVariableArrayType()) { SemaRef.Diag(Loc.getBegin(), diag::err_vla_unsupported); return false; @@ -339,6 +344,11 @@ class MarkDeviceFunction : public RecursiveASTVisitor { while (Ty->isAnyPointerType() || Ty->isArrayType()) Ty = QualType{Ty->getPointeeOrArrayElementType(), 0}; + // Pointers complicate recursion. Add this type to Visited. + // If already there, bail out. + if (!Visited.insert(Ty).second) + return true; + if (const auto *CRD = Ty->getAsCXXRecordDecl()) { if (CRD->isPolymorphic()) { SemaRef.Diag(CRD->getLocation(), diag::err_sycl_virtual_types); @@ -347,25 +357,25 @@ class MarkDeviceFunction : public RecursiveASTVisitor { } for (const auto &Field : CRD->fields()) { - if (!CheckSYCLType(Field->getType(), Field->getSourceRange())) { + if (!CheckSYCLType(Field->getType(), Field->getSourceRange(), Visited)) { SemaRef.Diag(Loc.getBegin(), diag::note_sycl_used_here); return false; } } } else if (const auto *RD = Ty->getAsRecordDecl()) { for (const auto &Field : RD->fields()) { - if (!CheckSYCLType(Field->getType(), Field->getSourceRange())) { + if (!CheckSYCLType(Field->getType(), Field->getSourceRange(), Visited)) { SemaRef.Diag(Loc.getBegin(), diag::note_sycl_used_here); return false; } } } else if (const auto *FPTy = dyn_cast(Ty)) { for (const auto &ParamTy : FPTy->param_types()) - if (!CheckSYCLType(ParamTy, Loc)) + if (!CheckSYCLType(ParamTy, Loc, Visited)) return false; - return CheckSYCLType(FPTy->getReturnType(), Loc); + return CheckSYCLType(FPTy->getReturnType(), Loc, Visited); } else if (const auto *FTy = dyn_cast(Ty)) { - return CheckSYCLType(FTy->getReturnType(), Loc); + return CheckSYCLType(FTy->getReturnType(), Loc, Visited); } return true; } @@ -755,6 +765,16 @@ static void buildArgTys(ASTContext &Context, CXXRecordDecl *KernelObj, // Create descriptors for each accessor field in the class or struct createParamDescForWrappedAccessors(Fld, ArgTy); + } 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 { @@ -842,6 +862,10 @@ static void populateIntHeader(SYCLIntegrationHeader &H, const StringRef Name, 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 (ArgTy->isStructureOrClassType() || ArgTy->isScalarType()) { // the parameter is an object of standard layout type or scalar; // the check for standard layout is done elsewhere @@ -1006,6 +1030,7 @@ static const char *paramKind2Str(KernelParamKind K) { CASE(accessor); CASE(std_layout); CASE(sampler); + CASE(pointer); default: return ""; } diff --git a/clang/test/CodeGenSYCL/usm-int-header.cpp b/clang/test/CodeGenSYCL/usm-int-header.cpp new file mode 100644 index 0000000000000..c3f9a0440da58 --- /dev/null +++ b/clang/test/CodeGenSYCL/usm-int-header.cpp @@ -0,0 +1,33 @@ +// RUN: %clang -I %S/Inputs --sycl -Xclang -fsycl-int-header=%t.h %s -c -o kernel.spv +// RUN: FileCheck -input-file=%t.h %s + +// CHECK:{ kernel_param_kind_t::kind_pointer, 8, 0 }, +// CHECK:{ kernel_param_kind_t::kind_pointer, 8, 8 }, + +//==-------------------usm-params.cpp - USM kernel param aspace test -------==// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + + +#include + +template +__attribute__((sycl_kernel)) void kernel(Func kernelFunc) { + kernelFunc(); +} + +int main() { + int* x; + float* y; + + kernel([=]() { + *x = 42; + *y = 3.14; + }); +} + + diff --git a/clang/test/SemaSYCL/usm-params.cpp b/clang/test/SemaSYCL/usm-params.cpp new file mode 100644 index 0000000000000..2068653f70692 --- /dev/null +++ b/clang/test/SemaSYCL/usm-params.cpp @@ -0,0 +1,32 @@ +// RUN: %clang_cc1 -std=c++11 -I %S/Inputs -fsycl-is-device -ast-dump %s | FileCheck %s + +//==-------------------usm-params.cpp - USM kernel param aspace test -------==// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + + +#include + +using namespace cl::sycl; + +template +__attribute__((sycl_kernel)) void kernel(Func kernelFunc) { + kernelFunc(); +} + +int main() { + int* x; + float* y; + + kernel([=]() { + *x = 42; + *y = 3.14; + }); +} + + +// CHECK: FunctionDecl {{.*}}usm_test 'void (__global int *, __global float *)' From 93e4f61ce7216ba3deee30b69f17b4c928678412 Mon Sep 17 00:00:00 2001 From: James Brodman Date: Wed, 26 Jun 2019 15:12:49 -0400 Subject: [PATCH 2/5] Modify test to set proper addr space for the pointer argument. Signed-off-by: James Brodman --- clang/test/CodeGenSYCL/debug-info-srcpos-kernel.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/clang/test/CodeGenSYCL/debug-info-srcpos-kernel.cpp b/clang/test/CodeGenSYCL/debug-info-srcpos-kernel.cpp index 51390aa8c7a15..c18fd59ae7ea2 100644 --- a/clang/test/CodeGenSYCL/debug-info-srcpos-kernel.cpp +++ b/clang/test/CodeGenSYCL/debug-info-srcpos-kernel.cpp @@ -1,5 +1,4 @@ -// RUN: DISABLE_INFER_AS=1 %clang --sycl %s -S -emit-llvm -g -o - | FileCheck %s --check-prefixes CHECK,CHECK-OLD -// RUN: %clang --sycl %s -S -emit-llvm -g -o - | FileCheck %s --check-prefixes CHECK,CHECK-NEW +// RUN: %clang --sycl %s -S -emit-llvm -g -o - | FileCheck %s --check-prefixes CHECK // // Verify the SYCL kernel routine is marked artificial. // @@ -22,8 +21,8 @@ int main() { return 0; } -// CHECK-OLD: define{{.*}} spir_kernel {{.*}}void @_ZTSZ4mainE15kernel_function(i32*{{.*}}){{.*}} !dbg [[KERNEL:![0-9]+]] {{.*}}{ -// CHECK-NEW: define{{.*}} spir_kernel {{.*}}void @_ZTSZ4mainE15kernel_function(i32 addrspace(4)*{{.*}}){{.*}} !dbg [[KERNEL:![0-9]+]] {{.*}}{ + +// CHECK: define{{.*}} spir_kernel {{.*}}void @_ZTSZ4mainE15kernel_function(i32 addrspace(1)*{{.*}}){{.*}} !dbg [[KERNEL:![0-9]+]] {{.*}}{ // CHECK: [[FILE:![0-9]+]] = !DIFile(filename: "{{.*}}debug-info-srcpos-kernel.cpp"{{.*}}) // CHECK: [[KERNEL]] = {{.*}}!DISubprogram(name: "_ZTSZ4mainE15kernel_function" // CHECK-SAME: scope: [[FILE]] From 62f2d11231c56e1a42277fa9b01613bdd8566316 Mon Sep 17 00:00:00 2001 From: James Brodman Date: Wed, 26 Jun 2019 16:55:15 -0400 Subject: [PATCH 3/5] Add kind_pointer to Integration header support in SYCL RT. Modify test to use generic address space switch in order to pass. Signed-off-by: James Brodman --- sycl/include/CL/sycl/detail/kernel_desc.hpp | 3 ++- sycl/include/CL/sycl/handler.hpp | 4 +++- sycl/test/basic_tests/handler/handler_mem_op.cpp | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/sycl/include/CL/sycl/detail/kernel_desc.hpp b/sycl/include/CL/sycl/detail/kernel_desc.hpp index f5cecb8dfaa2a..ebf71ac8561c7 100644 --- a/sycl/include/CL/sycl/detail/kernel_desc.hpp +++ b/sycl/include/CL/sycl/detail/kernel_desc.hpp @@ -33,7 +33,8 @@ class half; enum class kernel_param_kind_t { kind_accessor, kind_std_layout, // standard layout object parameters - kind_sampler + kind_sampler, + kind_pointer }; // describes a kernel parameter diff --git a/sycl/include/CL/sycl/handler.hpp b/sycl/include/CL/sycl/handler.hpp index 85bd0db65131c..4218e8239b80e 100644 --- a/sycl/include/CL/sycl/handler.hpp +++ b/sycl/include/CL/sycl/handler.hpp @@ -239,9 +239,11 @@ class handler { const auto kind_std_layout = detail::kernel_param_kind_t::kind_std_layout; const auto kind_accessor = detail::kernel_param_kind_t::kind_accessor; const auto kind_sampler = detail::kernel_param_kind_t::kind_sampler; + const auto kind_pointer = detail::kernel_param_kind_t::kind_pointer; switch (Kind) { - case kind_std_layout: { + case kind_std_layout: + case kind_pointer: { MArgs.emplace_back(Kind, Ptr, Size, Index + IndexShift); break; } diff --git a/sycl/test/basic_tests/handler/handler_mem_op.cpp b/sycl/test/basic_tests/handler/handler_mem_op.cpp index 3ed7421d85219..a4b95002b82b2 100644 --- a/sycl/test/basic_tests/handler/handler_mem_op.cpp +++ b/sycl/test/basic_tests/handler/handler_mem_op.cpp @@ -1,4 +1,4 @@ -// RUN: %clang -std=c++11 -fsycl %s -o %t.out -lstdc++ -lOpenCL -lsycl +// RUN: env ENABLE_INFER_AS=1 %clang -std=c++11 -fsycl %s -o %t.out -lstdc++ -lOpenCL -lsycl // RUN: env SYCL_DEVICE_TYPE=HOST %t.out // RUN: %CPU_RUN_PLACEHOLDER %t.out // RUN: %GPU_RUN_PLACEHOLDER %t.out From ae2fc0bec2c542bd9440d52ada3958a371e360b9 Mon Sep 17 00:00:00 2001 From: James Brodman Date: Thu, 27 Jun 2019 09:52:30 -0400 Subject: [PATCH 4/5] Fuse tests into one. Signed-off-by: James Brodman --- clang/test/CodeGenSYCL/usm-int-header.cpp | 11 ++++---- clang/test/SemaSYCL/usm-params.cpp | 32 ----------------------- 2 files changed, 6 insertions(+), 37 deletions(-) delete mode 100644 clang/test/SemaSYCL/usm-params.cpp diff --git a/clang/test/CodeGenSYCL/usm-int-header.cpp b/clang/test/CodeGenSYCL/usm-int-header.cpp index c3f9a0440da58..2f49b58b5ee32 100644 --- a/clang/test/CodeGenSYCL/usm-int-header.cpp +++ b/clang/test/CodeGenSYCL/usm-int-header.cpp @@ -1,10 +1,11 @@ +// RUN: %clang_cc1 -std=c++11 -I %S/Inputs -fsycl-is-device -ast-dump %s | FileCheck %s // RUN: %clang -I %S/Inputs --sycl -Xclang -fsycl-int-header=%t.h %s -c -o kernel.spv -// RUN: FileCheck -input-file=%t.h %s +// RUN: FileCheck -input-file=%t.h %s --check-prefix=INT-HEADER -// CHECK:{ kernel_param_kind_t::kind_pointer, 8, 0 }, -// CHECK:{ kernel_param_kind_t::kind_pointer, 8, 8 }, +// INT-HEADER:{ kernel_param_kind_t::kind_pointer, 8, 0 }, +// INT-HEADER:{ kernel_param_kind_t::kind_pointer, 8, 8 }, -//==-------------------usm-params.cpp - USM kernel param aspace test -------==// +//==--usm-int-header.cpp - USM kernel param aspace and int header test -----==// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -30,4 +31,4 @@ int main() { }); } - +// CHECK: FunctionDecl {{.*}}usm_test 'void (__global int *, __global float *)' diff --git a/clang/test/SemaSYCL/usm-params.cpp b/clang/test/SemaSYCL/usm-params.cpp deleted file mode 100644 index 2068653f70692..0000000000000 --- a/clang/test/SemaSYCL/usm-params.cpp +++ /dev/null @@ -1,32 +0,0 @@ -// RUN: %clang_cc1 -std=c++11 -I %S/Inputs -fsycl-is-device -ast-dump %s | FileCheck %s - -//==-------------------usm-params.cpp - USM kernel param aspace test -------==// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - - -#include - -using namespace cl::sycl; - -template -__attribute__((sycl_kernel)) void kernel(Func kernelFunc) { - kernelFunc(); -} - -int main() { - int* x; - float* y; - - kernel([=]() { - *x = 42; - *y = 3.14; - }); -} - - -// CHECK: FunctionDecl {{.*}}usm_test 'void (__global int *, __global float *)' From 955618aaad21eaf752dacc12d9205de5e81ff6ec Mon Sep 17 00:00:00 2001 From: James Brodman Date: Thu, 27 Jun 2019 09:54:09 -0400 Subject: [PATCH 5/5] Remove env var due to new default. Signed-off-by: James Brodman --- sycl/test/basic_tests/handler/handler_mem_op.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sycl/test/basic_tests/handler/handler_mem_op.cpp b/sycl/test/basic_tests/handler/handler_mem_op.cpp index a4b95002b82b2..3ed7421d85219 100644 --- a/sycl/test/basic_tests/handler/handler_mem_op.cpp +++ b/sycl/test/basic_tests/handler/handler_mem_op.cpp @@ -1,4 +1,4 @@ -// RUN: env ENABLE_INFER_AS=1 %clang -std=c++11 -fsycl %s -o %t.out -lstdc++ -lOpenCL -lsycl +// RUN: %clang -std=c++11 -fsycl %s -o %t.out -lstdc++ -lOpenCL -lsycl // RUN: env SYCL_DEVICE_TYPE=HOST %t.out // RUN: %CPU_RUN_PLACEHOLDER %t.out // RUN: %GPU_RUN_PLACEHOLDER %t.out