Skip to content

[SYCL] Store the kernel object size in the integration header #5862

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 8 commits into from
Mar 31, 2022
Merged

[SYCL] Store the kernel object size in the integration header #5862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ca0e682
Store the kernel object size in the integration header
premanandrao Mar 22, 2022
b9f758c
Fix clang-format errors
premanandrao Mar 22, 2022
83631c2
Fix clang-format errors (2)
premanandrao Mar 22, 2022
8c27573
Address code-review comments
premanandrao Mar 23, 2022
d728d26
Address more code review comments
premanandrao Mar 23, 2022
c95df69
Missed one file
premanandrao Mar 23, 2022
fd7b9d3
Avoid int64_t and fix function name
premanandrao Mar 23, 2022
e1a845a
clang-format again
premanandrao Mar 23, 2022
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
12 changes: 9 additions & 3 deletions clang/include/clang/Sema/Sema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,8 @@ class SYCLIntegrationHeader {
/// Signals that subsequent parameter descriptor additions will go to
/// the kernel with given name. Starts new kernel invocation descriptor.
void startKernel(const FunctionDecl *SyclKernel, QualType KernelNameType,
SourceLocation Loc, bool IsESIMD, bool IsUnnamedKernel);
SourceLocation Loc, bool IsESIMD, bool IsUnnamedKernel,
int64_t ObjSize);

/// Adds a kernel parameter descriptor to current kernel invocation
/// descriptor.
Expand Down Expand Up @@ -402,10 +403,15 @@ class SYCLIntegrationHeader {
// hasn't provided an explicit name for.
bool IsUnnamedKernel;

/// Size of the kernel object.
int64_t ObjSize = 0;

KernelDesc(const FunctionDecl *SyclKernel, QualType NameType,
SourceLocation KernelLoc, bool IsESIMD, bool IsUnnamedKernel)
SourceLocation KernelLoc, bool IsESIMD, bool IsUnnamedKernel,
int64_t ObjSize)
: SyclKernel(SyclKernel), NameType(NameType), KernelLocation(KernelLoc),
IsESIMDKernel(IsESIMD), IsUnnamedKernel(IsUnnamedKernel) {}
IsESIMDKernel(IsESIMD), IsUnnamedKernel(IsUnnamedKernel),
ObjSize(ObjSize) {}

void updateKernelNames(StringRef Name, StringRef StableName) {
this->Name = Name.str();
Expand Down
19 changes: 16 additions & 3 deletions clang/lib/Sema/SemaSYCL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3124,8 +3124,13 @@ class SyclKernelIntHeaderCreator : public SyclKernelFieldHandler {
FunctionDecl *KernelFunc)
: SyclKernelFieldHandler(S), Header(H) {
bool IsSIMDKernel = isESIMDKernelType(KernelObj);
// The header needs to access the kernel object size.
int64_t ObjSize = SemaRef.getASTContext()
.getTypeSizeInChars(KernelObj->getTypeForDecl())
.getQuantity();
Header.startKernel(KernelFunc, NameType, KernelObj->getLocation(),
IsSIMDKernel, IsSYCLUnnamedKernel(S, KernelFunc));
IsSIMDKernel, IsSYCLUnnamedKernel(S, KernelFunc),
ObjSize);
}

bool handleSyclSpecialType(const CXXRecordDecl *RD,
Expand Down Expand Up @@ -4777,6 +4782,14 @@ void SYCLIntegrationHeader::emit(raw_ostream &O) {
O << " return 0;\n";
O << "#endif\n";
O << " }\n";
StringRef ReturnType =
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be useful to just put the typedef at the top of the int-header with the nullptr_t typedef (and a couple of others?) so that you can just use it? WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh! I see what you mean now. Sure, I can do that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually don't see where we do that, but I could have sworn Jeff was working on that at one point? IF we ended up doing some of those typedefs that is perhaps my suggestion, otherwise this LGTM.

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 thought you meant add them anew, but my recollection of Jeff's attempt is that he had to abandon it.
I will leave it as is for now.

(S.Context.getTargetInfo().getInt64Type() == TargetInfo::SignedLong)
? "long"
: "long long";
O << " // Returns the size of the kernel object in bytes.\n";
O << " __SYCL_DLL_LOCAL\n";
O << " static constexpr " << ReturnType << " getKernelSize() { return "
<< K.ObjSize << "; }\n";
O << "};\n";
CurStart += N;
}
Expand Down Expand Up @@ -4807,9 +4820,9 @@ void SYCLIntegrationHeader::startKernel(const FunctionDecl *SyclKernel,
QualType KernelNameType,
SourceLocation KernelLocation,
bool IsESIMDKernel,
bool IsUnnamedKernel) {
bool IsUnnamedKernel, int64_t ObjSize) {
KernelDescs.emplace_back(SyclKernel, KernelNameType, KernelLocation,
IsESIMDKernel, IsUnnamedKernel);
IsESIMDKernel, IsUnnamedKernel, ObjSize);
}

void SYCLIntegrationHeader::addParamDesc(kernel_param_kind_t Kind, int Info,
Expand Down
26 changes: 26 additions & 0 deletions clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// RUN: %clang_cc1 -fsycl-is-device -internal-isystem %S/Inputs -fsycl-int-header=%t.h %s
// RUN: FileCheck -input-file=%t.h %s

// This test checks that the getKernelSize() member function is
// generated into the integration header and that it returns the
// size of the kernel object in bytes.

#include "sycl.hpp"

using namespace cl::sycl;

void testA() {
queue q;
constexpr int N = 256;
int A[N] = {10};
q.submit([&](handler &h) {
h.single_task<class KernelName>([=]() {
for (int k = 0; k < N; ++k) {
(void)A[k];
}
});
});
}
// CHECK: template <> struct KernelInfo<KernelName> {
// CHECK: // Returns the size of the kernel object in bytes.
// CHECK: static constexpr long{{.*}} getKernelSize() { return 1024; }