Skip to content

[SYCL] Start fully-qualifying our inline kernel names in forward decl… #4220

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.

Sign up for GitHub

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 4 commits into from
Aug 3, 2021
Merged

[SYCL] Start fully-qualifying our inline kernel names in forward decl… #4220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b986c30
[SYCL] Start fully-qualifying our inline kernel names in forward decl…
Jul 29, 2021
1d658c4
Add test
Jul 30, 2021
8e19b88
Update clang/lib/Sema/SemaSYCL.cpp
Aug 2, 2021
0d25e9d
Update clang/lib/Sema/SemaSYCL.cpp
Aug 2, 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
65 changes: 48 additions & 17 deletions clang/lib/Sema/SemaSYCL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4383,17 +4383,32 @@ class SYCLFwdDeclEmitter
const DeclContext *DC = D->getDeclContext();

while (DC) {
const auto *NS = dyn_cast_or_null<NamespaceDecl>(DC);

if (!NS)
break;

++NamespaceCnt;
const StringRef NSInlinePrefix = NS->isInline() ? "inline " : "";
NSStr.insert(
0,
Twine(NSInlinePrefix + "namespace " + NS->getName() + " { ").str());
DC = NS->getDeclContext();
if (const auto *NS = dyn_cast<NamespaceDecl>(DC)) {
++NamespaceCnt;
StringRef NSInlinePrefix = NS->isInline() ? "inline " : "";
NSStr.insert(
0,
Twine(NSInlinePrefix + "namespace " + NS->getName() + " { ").str());
DC = NS->getDeclContext();
} else {
// We should be able to handle a subset of the decl-context types to
// make our namespaces for forward declarations as specific as possible,
// so just skip them here. We can't use their names, since they would
// not be forward declarable, but we can try to make them as specific as
// possible.
// This permits things such as:
// namespace N1 { void foo() { kernel<class K>(...); }}
// and
// namespace N2 { void foo() { kernel<class K>(...); }}
// to co-exist, despite technically being against the SYCL rules.
// See SYCLKernelNameTypePrinter for the corresponding part that prints
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you point me to where this 'technically violates SYCL rules'? Also, what happens when the declarations are in the same namespace but the context is technically different? For e.g.

    // namespace N1 { void foo() { kernel<class K>(...); }}
    // and
    // namespace N1 { void bar() { kernel<class K>(...); }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is supposed to be part of this line:

The kernel name must be forward declarable at namespace scope (including global namespace scope) and may not be forward declared other than at namespace scope. If it isn’t forward declared but is specified as a template argument in a kernel invoking interface, as described in Section 4.9.4.2, then it may not conflict with a name in any enclosing namespace scope.

here: https://www.khronos.org/registry/SYCL/specs/sycl-2020/html/sycl-2020.html#sec:naming.kernels

That at least prohibits your example (since they are the same name in the same enclosing namespace), but I don't know what prevents something like:

namespace N1 { void foo() { kernel<class K>(...); }}
namespace N2 { void foo() { kernel<class K>(...); }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AlexeySachkov : Can you help with the above standards interpretation there?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. Thank you for clarification

// the kernel information for this type. These two must match.
if (isa<FunctionDecl, RecordDecl, LinkageSpecDecl>(DC)) {
DC = cast<Decl>(DC)->getDeclContext();
} else {
break;
}
}
}
OS << NSStr;
if (NamespaceCnt > 0)
Expand Down Expand Up @@ -4568,6 +4583,18 @@ class SYCLKernelNameTypePrinter
Quals.print(OS, Policy, /*appendSpaceIfNotEmpty*/ true);
}

// Use recursion to print the namespace-qualified name for the purposes of the
// canonical sycl example of a type being created in the kernel call.
void PrintNamespaceScopes(const DeclContext *DC) {
if (isa<NamespaceDecl, FunctionDecl, RecordDecl, LinkageSpecDecl>(DC)) {
PrintNamespaceScopes(DC->getParent());

const auto *NS = dyn_cast<NamespaceDecl>(DC);
if (NS && !NS->isAnonymousNamespace())
OS << NS->getName() << "::";
}
}

public:
SYCLKernelNameTypePrinter(raw_ostream &OS, PrintingPolicy &Policy)
: OS(OS), Policy(Policy) {}
Expand Down Expand Up @@ -4606,12 +4633,16 @@ class SYCLKernelNameTypePrinter

return;
}
// TODO: Next part of code results in printing of "class" keyword before
// class name in case if kernel name doesn't belong to some namespace. It
// seems if we don't print it, the integration header still represents valid
// c++ code. Probably we don't need to print it at all.
if (RD->getDeclContext()->isFunctionOrMethod()) {
OS << QualType::getAsString(T, Qualifiers(), Policy);

// Handle the canonical sycl example where the type is created for the first
// time in the kernel naming. We want to qualify this as fully as we can,
// but not in a way that won't be forward declarable. See
// SYCLFwdDeclEmitter::printForwardDecl for the corresponding list for
// printing the forward declaration, these two must match.
DeclContext *DC = RD->getDeclContext();
if (isa<FunctionDecl, RecordDecl, LinkageSpecDecl>(DC)) {
PrintNamespaceScopes(DC);
RD->printName(OS);
return;
}

Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGenSYCL/int_header1.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// CHECK-NEXT:struct IsThisValid;
// CHECK-NEXT:}}

// CHECK:template <> struct KernelInfo<class KernelName> {
// CHECK:template <> struct KernelInfo<KernelName> {
// CHECK:template <> struct KernelInfo<::nm1::nm2::KernelName0> {
// CHECK:template <> struct KernelInfo<::nm1::KernelName1> {
// CHECK:template <> struct KernelInfo<::nm1::KernelName3<::nm1::nm2::KernelName0>> {
Expand Down
4 changes: 2 additions & 2 deletions clang/test/CodeGenSYCL/int_header_esimd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ void testA() {
h.single_task<class KernelA>([=]() __attribute__((sycl_explicit_simd)){});
});
}
// CHECK-LABEL: template <> struct KernelInfo<class KernelA> {
// CHECK-LABEL: template <> struct KernelInfo<KernelA> {
// CHECK: static constexpr bool isESIMD() { return 1; }

// -- ESIMD Functor object kernel.
Expand All @@ -46,7 +46,7 @@ void testNA() {
h.single_task<class KernelNA>([=]() {});
});
}
// CHECK-LABEL: template <> struct KernelInfo<class KernelNA> {
// CHECK-LABEL: template <> struct KernelInfo<KernelNA> {
// CHECK: static constexpr bool isESIMD() { return 0; }

// -- Non-ESIMD Functor object kernel.
Expand Down
124 changes: 124 additions & 0 deletions clang/test/CodeGenSYCL/int_header_namespaced_inline_decls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
// RUN: %clang_cc1 -fsycl-is-device -fsycl-int-header=%t.h %s -o %t.out
// RUN: FileCheck -input-file=%t.h %s

// This test validates the behavior of inline-kernel-names to try to put them in
// the 'closest' possible namespace.

#include "Inputs/sycl.hpp"

using namespace sycl;

// Forward declarations of templated kernel function types:

namespace TopLevel {
void use() {
kernel_single_task<class DirectTopLevel>([]() {});
// CHECK: namespace TopLevel {
// CHECK-NEXT: class DirectTopLevel;
// CHECK-NEXT: }
}

struct TypeName {
void member_func() {
kernel_single_task<class DirectTopLevelMemFunc>([]() {});
// CHECK: namespace TopLevel {
// CHECK-NEXT: class DirectTopLevelMemFunc;
// CHECK-NEXT: }
}
};

extern "C" {
void use1() {
kernel_single_task<class DirectTopLevelLinkage>([]() {});
// CHECK: namespace TopLevel {
// CHECK-NEXT: class DirectTopLevelLinkage;
// CHECK-NEXT: }
}
struct LinkageTypeName {
void member_func() {
kernel_single_task<class DirectTopLevelLinkageMemFunc>([]() {});
// CHECK: namespace TopLevel {
// CHECK-NEXT: class DirectTopLevelLinkageMemFunc;
// CHECK-NEXT: }
}
};
}
} // namespace TopLevel

namespace {
void use2() {
kernel_single_task<class TopLevelAnonNS>([]() {});
// CHECK: namespace {
// CHECK-NEXT: class TopLevelAnonNS;
// CHECK-NEXT: }
}

struct LinkageTypeName {
void member_func() {
kernel_single_task<class AnonNSMemFunc>([]() {});
// CHECK: namespace {
// CHECK-NEXT: class AnonNSMemFunc;
// CHECK-NEXT: }
}
};
} // namespace

inline namespace InlineTopLevel {
void use3() {
kernel_single_task<class InlineDirectTopLevel>([]() {});
// CHECK: inline namespace InlineTopLevel {
// CHECK-NEXT: class InlineDirectTopLevel;
// CHECK-NEXT: }
}
struct LinkageTypeName {
void member_func() {
kernel_single_task<class InlineNSMemFunc>([]() {});
// CHECK: inline namespace InlineTopLevel {
// CHECK-NEXT: class InlineNSMemFunc;
// CHECK-NEXT: }
}
};

inline namespace {
void use4() {
kernel_single_task<class AnonNS>([]() {});
// CHECK: inline namespace {
// CHECK-NEXT: class AnonNS;
// CHECK-NEXT: }
}

extern "C" {
void use5() {
kernel_single_task<class AnonNSLinkage>([]() {});
// CHECK: inline namespace {
// CHECK-NEXT: class AnonNSLinkage;
// CHECK-NEXT: }
}
}
struct LinkageTypeName {
void member_func() {
kernel_single_task<class InlineAnonNSMemFunc>([]() {});
// CHECK: inline namespace {
// CHECK-NEXT: class InlineAnonNSMemFunc;
// CHECK-NEXT: }
}
};
} // namespace
} // namespace TopLevel

namespace A {
namespace B {
namespace {
namespace C::D {
struct DeepStruct {
void member_func() {
kernel_single_task<class WoahDeep>([]() {});
// CHECK: namespace A { namespace B { namespace { namespace C { namespace D {
// CHECK-NEXT: class WoahDeep;
// CHECK-NEXT: }}}}}
}
};
} // namespace C::D
} // namespace
} // namespace B
} // namespace A
2 changes: 1 addition & 1 deletion clang/test/CodeGenSYCL/integration_header.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
// CHECK-EMPTY:
// CHECK-NEXT: };
//
// CHECK: template <> struct KernelInfo<class first_kernel> {
// CHECK: template <> struct KernelInfo<first_kernel> {
// CHECK: template <> struct KernelInfo<::second_namespace::second_kernel<char>> {
// CHECK: template <> struct KernelInfo<::fourth_kernel<::template_arg_ns::namespaced_arg<1>>> {

Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGenSYCL/kernel-param-acc-array-ih.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
// CHECK-EMPTY:
// CHECK-NEXT: };

// CHECK: template <> struct KernelInfo<class kernel_A> {
// CHECK: template <> struct KernelInfo<kernel_A> {

#include "Inputs/sycl.hpp"

Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGenSYCL/kernel-param-member-acc-array-ih.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
// CHECK-EMPTY:
// CHECK-NEXT: };

// CHECK: template <> struct KernelInfo<class kernel_C> {
// CHECK: template <> struct KernelInfo<kernel_C> {

#include "Inputs/sycl.hpp"

Expand Down
6 changes: 3 additions & 3 deletions clang/test/CodeGenSYCL/kernel-param-pod-array-ih.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
// CHECK-EMPTY:
// CHECK-NEXT: };

// CHECK: template <> struct KernelInfo<class kernel_B> {
// CHECK: template <> struct KernelInfo<class kernel_C> {
// CHECK: template <> struct KernelInfo<class kernel_D> {
// CHECK: template <> struct KernelInfo<kernel_B> {
// CHECK: template <> struct KernelInfo<kernel_C> {
// CHECK: template <> struct KernelInfo<kernel_D> {

#include "Inputs/sycl.hpp"

Expand Down
12 changes: 6 additions & 6 deletions clang/test/CodeGenSYCL/parallel_for_this_item.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
// CHECK-NEXT: "_ZTSZZ4mainENKUlRN2cl4sycl7handlerEE_clES2_E3BEE"
// CHECK-NEXT: };

// CHECK:template <> struct KernelInfo<class GNU> {
// CHECK:template <> struct KernelInfo<GNU> {
// CHECK-NEXT: __SYCL_DLL_LOCAL
// CHECK-NEXT: static constexpr const char* getName() { return "_ZTSZZ4mainENKUlRN2cl4sycl7handlerEE_clES2_E3GNU"; }
// CHECK-NEXT: __SYCL_DLL_LOCAL
Expand All @@ -34,7 +34,7 @@
// CHECK-NEXT: __SYCL_DLL_LOCAL
// CHECK-NEXT: static constexpr bool callsAnyThisFreeFunction() { return 0; }
// CHECK-NEXT:};
// CHECK-NEXT:template <> struct KernelInfo<class EMU> {
// CHECK-NEXT:template <> struct KernelInfo<EMU> {
// CHECK-NEXT: __SYCL_DLL_LOCAL
// CHECK-NEXT: static constexpr const char* getName() { return "_ZTSZZ4mainENKUlRN2cl4sycl7handlerEE_clES2_E3EMU"; }
// CHECK-NEXT: __SYCL_DLL_LOCAL
Expand All @@ -50,7 +50,7 @@
// CHECK-NEXT: __SYCL_DLL_LOCAL
// CHECK-NEXT: static constexpr bool callsAnyThisFreeFunction() { return 1; }
// CHECK-NEXT:};
// CHECK-NEXT:template <> struct KernelInfo<class OWL> {
// CHECK-NEXT:template <> struct KernelInfo<OWL> {
// CHECK-NEXT: __SYCL_DLL_LOCAL
// CHECK-NEXT: static constexpr const char* getName() { return "_ZTSZZ4mainENKUlRN2cl4sycl7handlerEE_clES2_E3OWL"; }
// CHECK-NEXT: __SYCL_DLL_LOCAL
Expand All @@ -66,7 +66,7 @@
// CHECK-NEXT: __SYCL_DLL_LOCAL
// CHECK-NEXT: static constexpr bool callsAnyThisFreeFunction() { return 0; }
// CHECK-NEXT:};
// CHECK-NEXT:template <> struct KernelInfo<class RAT> {
// CHECK-NEXT:template <> struct KernelInfo<RAT> {
// CHECK-NEXT: __SYCL_DLL_LOCAL
// CHECK-NEXT: static constexpr const char* getName() { return "_ZTSZZ4mainENKUlRN2cl4sycl7handlerEE_clES2_E3RAT"; }
// CHECK-NEXT: __SYCL_DLL_LOCAL
Expand All @@ -82,7 +82,7 @@
// CHECK-NEXT: __SYCL_DLL_LOCAL
// CHECK-NEXT: static constexpr bool callsAnyThisFreeFunction() { return 1; }
// CHECK-NEXT:};
// CHECK-NEXT:template <> struct KernelInfo<class FOX> {
// CHECK-NEXT:template <> struct KernelInfo<FOX> {
// CHECK-NEXT: __SYCL_DLL_LOCAL
// CHECK-NEXT: static constexpr const char* getName() { return "_ZTSZZ4mainENKUlRN2cl4sycl7handlerEE_clES2_E3FOX"; }
// CHECK-NEXT: __SYCL_DLL_LOCAL
Expand All @@ -98,7 +98,7 @@
// CHECK-NEXT: __SYCL_DLL_LOCAL
// CHECK-NEXT: static constexpr bool callsAnyThisFreeFunction() { return 1; }
// CHECK-NEXT:};
// CHECK-NEXT:template <> struct KernelInfo<class BEE> {
// CHECK-NEXT:template <> struct KernelInfo<BEE> {
// CHECK-NEXT: __SYCL_DLL_LOCAL
// CHECK-NEXT: static constexpr const char* getName() { return "_ZTSZZ4mainENKUlRN2cl4sycl7handlerEE_clES2_E3BEE"; }
// CHECK-NEXT: __SYCL_DLL_LOCAL
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGenSYCL/union-kernel-param-ih.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
// CHECK-EMPTY:
// CHECK-NEXT:};

// CHECK: template <> struct KernelInfo<class kernel_A> {
// CHECK: template <> struct KernelInfo<kernel_A> {

union MyUnion {
int FldInt;
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGenSYCL/wrapped-accessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
// CHECK-EMPTY:
// CHECK-NEXT: };

// CHECK: template <> struct KernelInfo<class wrapped_access> {
// CHECK: template <> struct KernelInfo<wrapped_access> {

#include "Inputs/sycl.hpp"

Expand Down