Skip to content

[SYCL][USM] Initial Support for USM #256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 1, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
35 changes: 30 additions & 5 deletions clang/lib/Sema/SemaSYCL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,11 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {

private:
bool CheckSYCLType(QualType Ty, SourceRange Loc) {
llvm::DenseSet<QualType> visited;
return CheckSYCLType(Ty, Loc, visited);
}

bool CheckSYCLType(QualType Ty, SourceRange Loc, llvm::DenseSet<QualType> &Visited) {
if (Ty->isVariableArrayType()) {
SemaRef.Diag(Loc.getBegin(), diag::err_vla_unsupported);
return false;
Expand All @@ -339,6 +344,11 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
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);
Expand All @@ -347,25 +357,25 @@ class MarkDeviceFunction : public RecursiveASTVisitor<MarkDeviceFunction> {
}

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<FunctionProtoType>(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<FunctionType>(Ty)) {
return CheckSYCLType(FTy->getReturnType(), Loc);
return CheckSYCLType(FTy->getReturnType(), Loc, Visited);
}
return true;
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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<unsigned>(Sz), static_cast<unsigned>(Offset));
} else if (ArgTy->isPointerType()) {
uint64_t Sz = Ctx.getTypeSizeInChars(Fld->getType()).getQuantity();
H.addParamDesc(SYCLIntegrationHeader::kind_pointer,
static_cast<unsigned>(Sz), static_cast<unsigned>(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
Expand Down Expand Up @@ -1006,6 +1030,7 @@ static const char *paramKind2Str(KernelParamKind K) {
CASE(accessor);
CASE(std_layout);
CASE(sampler);
CASE(pointer);
default:
return "<ERROR>";
}
Expand Down
7 changes: 3 additions & 4 deletions clang/test/CodeGenSYCL/debug-info-srcpos-kernel.cpp
Original file line number Diff line number Diff line change
@@ -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.
//
Expand All @@ -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]+]] {{.*}}{
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting.
@asavonic, this looks like a bug with address space handling.

  1. Note, clang emit OpenCL kernel with "generic pointer" argument.
  2. Clang does not emit warnings about captured pointers.

I think both these issues should be fixed for the cases when USM feature is not available.

// CHECK: [[FILE:![0-9]+]] = !DIFile(filename: "{{.*}}debug-info-srcpos-kernel.cpp"{{.*}})
// CHECK: [[KERNEL]] = {{.*}}!DISubprogram(name: "_ZTSZ4mainE15kernel_function"
// CHECK-SAME: scope: [[FILE]]
Expand Down
33 changes: 33 additions & 0 deletions clang/test/CodeGenSYCL/usm-int-header.cpp
Original file line number Diff line number Diff line change
@@ -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 <sycl.hpp>

template <typename name, typename Func>
__attribute__((sycl_kernel)) void kernel(Func kernelFunc) {
kernelFunc();
}

int main() {
int* x;
float* y;

kernel<class usm_test>([=]() {
*x = 42;
*y = 3.14;
});
}


32 changes: 32 additions & 0 deletions clang/test/SemaSYCL/usm-params.cpp
Original file line number Diff line number Diff line change
@@ -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 <sycl.hpp>

using namespace cl::sycl;

template <typename name, typename Func>
__attribute__((sycl_kernel)) void kernel(Func kernelFunc) {
kernelFunc();
}

int main() {
int* x;
float* y;

kernel<class usm_test>([=]() {
*x = 42;
*y = 3.14;
});
}


// CHECK: FunctionDecl {{.*}}usm_test 'void (__global int *, __global float *)'