Skip to content

[SYCL] Improve kernel name diagnostics implementation #3225

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 29 commits into from
Apr 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
ea65d90
Address review comments
srividya-sundaram Feb 24, 2021
4595b15
Address review comments
srividya-sundaram Feb 26, 2021
2d94faa
wip commit
srividya-sundaram Feb 28, 2021
a53ec6a
Add helper function
srividya-sundaram Mar 9, 2021
d0a3bce
wip test commit
srividya-sundaram Mar 10, 2021
d01e21b
Diagnose incomplete template specialization
srividya-sundaram Mar 16, 2021
9dcc71c
Merge branch 'sycl' into nested-anon-diag
srividya-sundaram Mar 16, 2021
b3afda8
remove -fsycl flag passed to clang_cc1
srividya-sundaram Mar 16, 2021
7b9076a
Merge branch 'nested-anon-diag' of https://github.com/srividya-sundar…
srividya-sundaram Mar 16, 2021
dabf9e2
Address review comments
srividya-sundaram Mar 19, 2021
b35202c
address review comments
srividya-sundaram Mar 22, 2021
50adf0b
Merge branch 'sycl' into nested-anon-diag
srividya-sundaram Mar 22, 2021
c699ba6
Update nested-anon-and-std-ns.cpp
srividya-sundaram Mar 22, 2021
50b26cf
Update unnamed-kernel.cpp
srividya-sundaram Mar 22, 2021
e1ffc63
Update implicit_kernel_type.cpp
srividya-sundaram Mar 22, 2021
1cbb657
Update stdtypes_kernel_type.cpp
srividya-sundaram Mar 22, 2021
73e5577
Fix test failures, format issues
srividya-sundaram Mar 22, 2021
9248f80
Remove unnecessary visitor calls
srividya-sundaram Mar 22, 2021
e9d2848
fix test
srividya-sundaram Mar 22, 2021
e1477d5
minor nit
srividya-sundaram Mar 23, 2021
c283d0d
Address review comments
srividya-sundaram Mar 23, 2021
8a00f0f
Update stdtypes_kernel_type.cpp
srividya-sundaram Mar 24, 2021
fe94fce
Update stdtypes_kernel_type.cpp
srividya-sundaram Mar 24, 2021
8f50407
Address review comments
srividya-sundaram Mar 24, 2021
413e9ac
Update unnamedlambda diagnostic
srividya-sundaram Mar 25, 2021
046e6fe
Add wild card to test
srividya-sundaram Mar 25, 2021
3541af0
Merge branch 'sycl' into nested-anon-diag
srividya-sundaram Mar 29, 2021
48e8162
update test
srividya-sundaram Mar 29, 2021
20526f2
Update sycl/test/functor/kernel_functor.cpp
srividya-sundaram Apr 5, 2021
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
13 changes: 8 additions & 5 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -11167,12 +11167,15 @@ def err_ext_int_max_size : Error<"%select{signed|unsigned}0 _ExtInt of bit "
def err_esimd_glob_cant_init : Error<
"SYCL explicit SIMD does not permit private global variable to have an initializer">;

def err_sycl_kernel_incorrectly_named : Error<"%0 is an invalid kernel name type">;
def note_invalid_type_in_sycl_kernel : Note<
"%select{%1 should be globally-visible"
def err_nullptr_t_type_in_sycl_kernel : Error<"%0 is an invalid kernel name, "
"'std::nullptr_t' is declared in the 'std' namespace ">;
def err_invalid_std_type_in_sycl_kernel : Error<"%0 is an invalid kernel name, "
"%q1 is declared in the 'std' namespace ">;

def err_sycl_kernel_incorrectly_named : Error<
"%select{%1 should be globally visible"
"|unscoped enum %1 requires fixed underlying type"
"|type %1 cannot be in the \"std\" namespace"
"|unnamed type used in a SYCL kernel name"
"|unnamed lambda %1 used"
"}0">;

def err_sycl_kernel_not_function_object
Expand Down
143 changes: 89 additions & 54 deletions clang/lib/Sema/SemaSYCL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3097,21 +3097,14 @@ class SYCLKernelNameTypeVisitor
void Visit(QualType T) {
if (T.isNull())
return;

const CXXRecordDecl *RD = T->getAsCXXRecordDecl();
if (!RD) {
if (T->isNullPtrType()) {
S.Diag(KernelInvocationFuncLoc, diag::err_sycl_kernel_incorrectly_named)
<< KernelNameType;
S.Diag(KernelInvocationFuncLoc, diag::note_invalid_type_in_sycl_kernel)
<< /* kernel name cannot be a type in the std namespace */ 2 << T;
IsInvalid = true;
}
return;
}
// If KernelNameType has template args visit each template arg via
// ConstTemplateArgumentVisitor
if (const auto *TSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) {
if (const auto *TSD =
dyn_cast_or_null<ClassTemplateSpecializationDecl>(RD)) {
ArrayRef<TemplateArgument> Args = TSD->getTemplateArgs().asArray();

VisitTemplateArgs(Args);
} else {
InnerTypeVisitor::Visit(T.getTypePtr());
Expand All @@ -3124,62 +3117,104 @@ class SYCLKernelNameTypeVisitor
InnerTemplArgVisitor::Visit(TA);
}

void VisitEnumType(const EnumType *T) {
const EnumDecl *ED = T->getDecl();
if (!ED->isScoped() && !ED->isFixed()) {
S.Diag(KernelInvocationFuncLoc, diag::err_sycl_kernel_incorrectly_named)
void VisitBuiltinType(const BuiltinType *TT) {
if (TT->isNullPtrType()) {
S.Diag(KernelInvocationFuncLoc, diag::err_nullptr_t_type_in_sycl_kernel)
<< KernelNameType;
S.Diag(KernelInvocationFuncLoc, diag::note_invalid_type_in_sycl_kernel)
<< /* Unscoped enum requires fixed underlying type */ 1
<< QualType(ED->getTypeForDecl(), 0);

IsInvalid = true;
}
return;
}

void VisitRecordType(const RecordType *T) {
return VisitTagDecl(T->getDecl());
}
void VisitTagType(const TagType *TT) {
return DiagnoseKernelNameType(TT->getDecl());
}

void DiagnoseKernelNameType(const NamedDecl *DeclNamed) {
/*
This is a helper function which throws an error if the kernel name
declaration is:
* declared within namespace 'std' (at any level)
e.g., namespace std { namespace literals { class Whatever; } }
h.single_task<std::literals::Whatever>([]() {});
* declared within an anonymous namespace (at any level)
e.g., namespace foo { namespace { class Whatever; } }
h.single_task<foo::Whatever>([]() {});
* declared within a function
e.g., void foo() { struct S { int i; };
h.single_task<S>([]() {}); }
* declared within another tag
e.g., struct S { struct T { int i } t; };
h.single_task<S::T>([]() {});
*/

if (const auto *ED = dyn_cast<EnumDecl>(DeclNamed)) {
if (!ED->isScoped() && !ED->isFixed()) {
S.Diag(KernelInvocationFuncLoc, diag::err_sycl_kernel_incorrectly_named)
<< /* unscoped enum requires fixed underlying type */ 1
<< DeclNamed;
IsInvalid = true;
}
}

void VisitTagDecl(const TagDecl *Tag) {
bool UnnamedLambdaEnabled =
S.getASTContext().getLangOpts().SYCLUnnamedLambda;
const DeclContext *DeclCtx = Tag->getDeclContext();
const DeclContext *DeclCtx = DeclNamed->getDeclContext();
if (DeclCtx && !UnnamedLambdaEnabled) {
auto *NameSpace = dyn_cast_or_null<NamespaceDecl>(DeclCtx);
if (NameSpace && NameSpace->isStdNamespace()) {
S.Diag(KernelInvocationFuncLoc, diag::err_sycl_kernel_incorrectly_named)
<< KernelNameType;
S.Diag(KernelInvocationFuncLoc, diag::note_invalid_type_in_sycl_kernel)
<< /* kernel name cannot be a type in the std namespace */ 2
<< QualType(Tag->getTypeForDecl(), 0);
IsInvalid = true;
return;
}
if (!DeclCtx->isTranslationUnit() && !isa<NamespaceDecl>(DeclCtx)) {
const bool KernelNameIsMissing = Tag->getName().empty();
if (KernelNameIsMissing) {
S.Diag(KernelInvocationFuncLoc,
diag::err_sycl_kernel_incorrectly_named)
<< KernelNameType;

// Check if the kernel name declaration is declared within namespace
// "std" or "anonymous" namespace (at any level).
while (!DeclCtx->isTranslationUnit() && isa<NamespaceDecl>(DeclCtx)) {
const auto *NSDecl = cast<NamespaceDecl>(DeclCtx);
if (NSDecl->isStdNamespace()) {
S.Diag(KernelInvocationFuncLoc,
diag::note_invalid_type_in_sycl_kernel)
<< /* unnamed type used in a SYCL kernel name */ 3;
diag::err_invalid_std_type_in_sycl_kernel)
<< KernelNameType << DeclNamed;
IsInvalid = true;
return;
}
if (Tag->isCompleteDefinition()) {
if (NSDecl->isAnonymousNamespace()) {
S.Diag(KernelInvocationFuncLoc,
diag::err_sycl_kernel_incorrectly_named)
<< /* kernel name should be globally visible */ 0
<< KernelNameType;
S.Diag(KernelInvocationFuncLoc,
diag::note_invalid_type_in_sycl_kernel)
<< /* kernel name is not globally-visible */ 0
<< QualType(Tag->getTypeForDecl(), 0);
IsInvalid = true;
} else {
S.Diag(KernelInvocationFuncLoc, diag::warn_sycl_implicit_decl);
S.Diag(Tag->getSourceRange().getBegin(), diag::note_previous_decl)
<< Tag->getName();
return;
}
DeclCtx = DeclCtx->getParent();
}

// Check if the kernel name is a Tag declaration
// local to a non-namespace scope (i.e. Inside a function or within
// another Tag etc).
if (!DeclCtx->isTranslationUnit() && !isa<NamespaceDecl>(DeclCtx)) {
if (const auto *Tag = dyn_cast<TagDecl>(DeclNamed)) {
bool UnnamedLambdaUsed = Tag->getIdentifier() == nullptr;

if (UnnamedLambdaUsed) {
S.Diag(KernelInvocationFuncLoc,
diag::err_sycl_kernel_incorrectly_named)
<< /* unnamed lambda used */ 2 << KernelNameType;

IsInvalid = true;
return;
}
// Check if the declaration is completely defined within a
// function or class/struct.

if (Tag->isCompleteDefinition()) {
S.Diag(KernelInvocationFuncLoc,
diag::err_sycl_kernel_incorrectly_named)
<< /* kernel name should be globally visible */ 0
<< KernelNameType;

IsInvalid = true;
} else {
S.Diag(KernelInvocationFuncLoc, diag::warn_sycl_implicit_decl);
S.Diag(DeclNamed->getLocation(), diag::note_previous_decl)
<< DeclNamed->getName();
}
}
}
}
Expand All @@ -3188,15 +3223,15 @@ class SYCLKernelNameTypeVisitor
void VisitTypeTemplateArgument(const TemplateArgument &TA) {
QualType T = TA.getAsType();
if (const auto *ET = T->getAs<EnumType>())
VisitEnumType(ET);
VisitTagType(ET);
else
Visit(T);
}

void VisitIntegralTemplateArgument(const TemplateArgument &TA) {
QualType T = TA.getIntegralType();
if (const EnumType *ET = T->getAs<EnumType>())
VisitEnumType(ET);
VisitTagType(ET);
}

void VisitTemplateTemplateArgument(const TemplateArgument &TA) {
Expand All @@ -3207,7 +3242,7 @@ class SYCLKernelNameTypeVisitor
if (NonTypeTemplateParmDecl *TemplateParam =
dyn_cast<NonTypeTemplateParmDecl>(P))
if (const EnumType *ET = TemplateParam->getType()->getAs<EnumType>())
VisitEnumType(ET);
VisitTagType(ET);
}
}

Expand Down Expand Up @@ -3268,7 +3303,7 @@ void Sema::CheckSYCLKernelCall(FunctionDecl *KernelFunc, SourceRange CallLoc,

// Emit diagnostics for SYCL device kernels only
if (LangOpts.SYCLIsDevice)
KernelNameTypeVisitor.Visit(KernelNameType);
KernelNameTypeVisitor.Visit(KernelNameType.getCanonicalType());
Visitor.VisitRecordBases(KernelObj, FieldChecker, UnionChecker, DecompMarker);
Visitor.VisitRecordFields(KernelObj, FieldChecker, UnionChecker,
DecompMarker);
Expand Down
24 changes: 2 additions & 22 deletions clang/test/CodeGenSYCL/int_header1.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %clang_cc1 -fsycl-is-device -fsycl-int-header=%t.h %s -o %t.out
// RUN: %clang_cc1 -fsycl-is-device -internal-isystem %S/Inputs -sycl-std=2020 -fsycl-int-header=%t.h %s -o %t.out
// RUN: FileCheck -input-file=%t.h %s

// CHECK:template <> struct KernelInfo<class KernelName> {
Expand All @@ -8,18 +8,15 @@
// CHECK:template <> struct KernelInfo<::nm1::KernelName3<::nm1::KernelName1>> {
// CHECK:template <> struct KernelInfo<::nm1::KernelName4<::nm1::nm2::KernelName0>> {
// CHECK:template <> struct KernelInfo<::nm1::KernelName4<::nm1::KernelName1>> {
// CHECK:template <> struct KernelInfo<::nm1::KernelName3<KernelName5>> {
// CHECK:template <> struct KernelInfo<::nm1::KernelName4<KernelName7>> {
// CHECK:template <> struct KernelInfo<::nm1::KernelName8<::nm1::nm2::C>> {
// CHECK:template <> struct KernelInfo<::TmplClassInAnonNS<ClassInAnonNS>> {
// CHECK:template <> struct KernelInfo<::nm1::KernelName9<char>> {
// CHECK:template <> struct KernelInfo<::nm1::KernelName3<const volatile ::nm1::KernelName3<const volatile char>>> {

// This test checks if the SYCL device compiler is able to generate correct
// integration header when the kernel name class is expressed in different
// forms.

#include "Inputs/sycl.hpp"
#include "sycl.hpp"

template <typename KernelName, typename KernelType>
__attribute__((sycl_kernel)) void kernel_single_task(const KernelType &kernelFunc) {
Expand Down Expand Up @@ -110,29 +107,12 @@ struct MyWrapper {
kernel_single_task<nm1::KernelName4<nm1::KernelName1>>(
[=]() { acc.use(); });

// TIPI
// an incomplete template specialization class with incomplete class as
// argument forward-declared "in-place"
kernel_single_task<nm1::KernelName3<class KernelName5>>(
[=]() { acc.use(); });

// TDPI
// a defined template specialization class with incomplete class as
// argument forward-declared "in-place"
kernel_single_task<nm1::KernelName4<class KernelName7>>(
[=]() { acc.use(); });

// TPITD
// a defined template pack specialization class with defined class
// as argument declared in a namespace at translation unit scope
kernel_single_task<nm1::KernelName8<nm1::nm2::C>>(
[=]() { acc.use(); });

// kernel name type is a templated class, both the top-level class and the
// template argument are declared in the anonymous namespace
kernel_single_task<TmplClassInAnonNS<class ClassInAnonNS>>(
[=]() { acc.use(); });

// Kernel name type is a templated specialization class with empty template pack argument
kernel_single_task<nm1::KernelName9<char>>(
[=]() { acc.use(); });
Expand Down
18 changes: 1 addition & 17 deletions clang/test/CodeGenSYCL/integration_header.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %clang_cc1 -fsycl-is-device -triple spir64-unknown-unknown-sycldevice -fsycl-int-header=%t.h %s -emit-llvm -o %t.ll
// RUN: %clang_cc1 -fsycl-is-device -triple spir64-unknown-unknown-sycldevice -sycl-std=2020 -fsycl-int-header=%t.h %s -emit-llvm -o %t.ll
// RUN: FileCheck -input-file=%t.h %s
//
// CHECK: #include <CL/sycl/detail/kernel_desc.hpp>
Expand All @@ -7,9 +7,6 @@
// CHECK-NEXT: namespace second_namespace {
// CHECK-NEXT: template <typename T> class second_kernel;
// CHECK-NEXT: }
// CHECK-NEXT: struct X;
// CHECK-NEXT: template <typename T> struct point;
// CHECK-NEXT: template <int a, typename T1, typename T2> class third_kernel;
// CHECK-NEXT: namespace template_arg_ns {
// CHECK-NEXT: template <int DimX> struct namespaced_arg;
// CHECK-NEXT: }
Expand All @@ -19,7 +16,6 @@
// CHECK-NEXT: const char* const kernel_names[] = {
// CHECK-NEXT: "_ZTSZ4mainE12first_kernel",
// CHECK-NEXT: "_ZTSN16second_namespace13second_kernelIcEE",
// CHECK-NEXT: "_ZTS12third_kernelILi1Ei5pointIZ4mainE1XEE"
// CHECK-NEXT: "_ZTS13fourth_kernelIJN15template_arg_ns14namespaced_argILi1EEEEE"
// CHECK-NEXT: "_ZTSZ4mainE16accessor_in_base"
// CHECK-NEXT: };
Expand All @@ -38,11 +34,6 @@
// CHECK-NEXT: { kernel_param_kind_t::kind_accessor, 6112, 4 },
// CHECK-NEXT: { kernel_param_kind_t::kind_sampler, 8, 16 },
// CHECK-EMPTY:
// CHECK-NEXT: //--- _ZTS12third_kernelILi1Ei5pointIZ4mainE1XEE
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 4, 0 },
// CHECK-NEXT: { kernel_param_kind_t::kind_accessor, 6112, 4 },
// CHECK-NEXT: { kernel_param_kind_t::kind_sampler, 8, 16 },
// CHECK-EMPTY:
// CHECK-NEXT: //--- _ZTS13fourth_kernelIJN15template_arg_ns14namespaced_argILi1EEEEE
// CHECK-NEXT: { kernel_param_kind_t::kind_std_layout, 4, 0 },
// CHECK-NEXT: { kernel_param_kind_t::kind_accessor, 6112, 4 },
Expand All @@ -61,7 +52,6 @@
//
// CHECK: template <> struct KernelInfo<class first_kernel> {
// CHECK: template <> struct KernelInfo<::second_namespace::second_kernel<char>> {
// CHECK: template <> struct KernelInfo<::third_kernel<1, int, ::point<X>>> {
// CHECK: template <> struct KernelInfo<::fourth_kernel<::template_arg_ns::namespaced_arg<1>>> {

#include "Inputs/sycl.hpp"
Expand Down Expand Up @@ -140,12 +130,6 @@ int main() {
smplr.use();
}
});
kernel_single_task<class third_kernel<1, int,point<struct X>>>([=]() {
if (i == 13) {
acc2.use();
smplr.use();
}
});

kernel_single_task<class fourth_kernel<template_arg_ns::namespaced_arg<1>>>([=]() {
if (i == 13) {
Expand Down
Loading