From ca0e6827db16c01e4d90ca982fcbbe2e8144d37d Mon Sep 17 00:00:00 2001 From: Premanand M Rao Date: Tue, 22 Mar 2022 14:28:07 -0700 Subject: [PATCH 1/8] Store the kernel object size in the integration header --- clang/include/clang/Sema/Sema.h | 12 +++++++--- clang/lib/Sema/SemaSYCL.cpp | 15 +++++++++--- .../CodeGenSYCL/int_header_kernelobjsize.cpp | 23 +++++++++++++++++++ 3 files changed, 44 insertions(+), 6 deletions(-) create mode 100644 clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 3c2e0a863d5ad..6e40891294842 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -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, + unsigned long ObjSize); /// Adds a kernel parameter descriptor to current kernel invocation /// descriptor. @@ -402,10 +403,15 @@ class SYCLIntegrationHeader { // hasn't provided an explicit name for. bool IsUnnamedKernel; + /// Size of the kernel object. + unsigned long ObjSize = 0; + KernelDesc(const FunctionDecl *SyclKernel, QualType NameType, - SourceLocation KernelLoc, bool IsESIMD, bool IsUnnamedKernel) + SourceLocation KernelLoc, bool IsESIMD, bool IsUnnamedKernel, + unsigned long 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(); diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index fac53fcfbf94a..497ea291b01c6 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -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. + unsigned long 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, @@ -4777,6 +4782,9 @@ void SYCLIntegrationHeader::emit(raw_ostream &O) { O << " return 0;\n"; O << "#endif\n"; O << " }\n"; + O << " __SYCL_DLL_LOCAL\n"; + O << " static constexpr unsigned long getSize() { return " + << K.ObjSize << "; }\n"; O << "};\n"; CurStart += N; } @@ -4807,9 +4815,10 @@ void SYCLIntegrationHeader::startKernel(const FunctionDecl *SyclKernel, QualType KernelNameType, SourceLocation KernelLocation, bool IsESIMDKernel, - bool IsUnnamedKernel) { + bool IsUnnamedKernel, + unsigned long ObjSize) { KernelDescs.emplace_back(SyclKernel, KernelNameType, KernelLocation, - IsESIMDKernel, IsUnnamedKernel); + IsESIMDKernel, IsUnnamedKernel, ObjSize); } void SYCLIntegrationHeader::addParamDesc(kernel_param_kind_t Kind, int Info, diff --git a/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp b/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp new file mode 100644 index 0000000000000..7485b5e2d8525 --- /dev/null +++ b/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp @@ -0,0 +1,23 @@ +// 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 getSize() member function is generated +// into the integration header and that it returns the size of the +// kernel object. + +#include "sycl.hpp" + +using namespace cl::sycl; + +void testA() { + queue q; + constexpr int N = 256; + int A[N] = { 10 }; + kernel_single_task([=]() { + for (int k = 0; k < N; ++k) { + (void)A[k]; + } + }); +} +// CHECK: template <> struct KernelInfo { +// CHECK: static constexpr unsigned long getSize() { return 1024; } From b9f758ceea60c8483bf4dc0752a956c7479e190b Mon Sep 17 00:00:00 2001 From: Premanand M Rao Date: Tue, 22 Mar 2022 15:08:55 -0700 Subject: [PATCH 2/8] Fix clang-format errors --- clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp b/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp index 7485b5e2d8525..802e7cb68c567 100644 --- a/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp +++ b/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp @@ -12,7 +12,7 @@ using namespace cl::sycl; void testA() { queue q; constexpr int N = 256; - int A[N] = { 10 }; + int A[N] = {10}; kernel_single_task([=]() { for (int k = 0; k < N; ++k) { (void)A[k]; From 83631c2b4dfc99ca7d777ec058f1f468ec8fe2c2 Mon Sep 17 00:00:00 2001 From: Premanand M Rao Date: Tue, 22 Mar 2022 15:10:44 -0700 Subject: [PATCH 3/8] Fix clang-format errors (2) --- clang/lib/Sema/SemaSYCL.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 497ea291b01c6..2a79b4f448c8d 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -3125,9 +3125,9 @@ class SyclKernelIntHeaderCreator : public SyclKernelFieldHandler { : SyclKernelFieldHandler(S), Header(H) { bool IsSIMDKernel = isESIMDKernelType(KernelObj); // The header needs to access the kernel object size. - unsigned long ObjSize = SemaRef.getASTContext(). - getTypeSizeInChars(KernelObj->getTypeForDecl()). - getQuantity(); + unsigned long ObjSize = SemaRef.getASTContext() + .getTypeSizeInChars(KernelObj->getTypeForDecl()) + .getQuantity(); Header.startKernel(KernelFunc, NameType, KernelObj->getLocation(), IsSIMDKernel, IsSYCLUnnamedKernel(S, KernelFunc), ObjSize); @@ -4783,8 +4783,8 @@ void SYCLIntegrationHeader::emit(raw_ostream &O) { O << "#endif\n"; O << " }\n"; O << " __SYCL_DLL_LOCAL\n"; - O << " static constexpr unsigned long getSize() { return " - << K.ObjSize << "; }\n"; + O << " static constexpr unsigned long getSize() { return " << K.ObjSize + << "; }\n"; O << "};\n"; CurStart += N; } From 8c275732b534344b11412da67fc690ba2b5dda6f Mon Sep 17 00:00:00 2001 From: Premanand M Rao Date: Wed, 23 Mar 2022 07:38:26 -0700 Subject: [PATCH 4/8] Address code-review comments --- clang/lib/Sema/SemaSYCL.cpp | 5 +++-- .../CodeGenSYCL/int_header_kernelobjsize.cpp | 19 +++++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 2a79b4f448c8d..de490cd79ea81 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -4782,9 +4782,10 @@ void SYCLIntegrationHeader::emit(raw_ostream &O) { O << " return 0;\n"; O << "#endif\n"; O << " }\n"; + O << " // Returns the size of the kernel object in bytes.\n"; O << " __SYCL_DLL_LOCAL\n"; - O << " static constexpr unsigned long getSize() { return " << K.ObjSize - << "; }\n"; + O << " static constexpr unsigned long getKernelObjectSize() { return " + << K.ObjSize << "; }\n"; O << "};\n"; CurStart += N; } diff --git a/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp b/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp index 802e7cb68c567..9f6c8e7c1c90d 100644 --- a/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp +++ b/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp @@ -1,9 +1,9 @@ // 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 getSize() member function is generated -// into the integration header and that it returns the size of the -// kernel object. +// This test checks that the getKernelObjectSize() member function +// is generated into the integration header and that it returns the +// size of the kernel object in bytes. #include "sycl.hpp" @@ -13,11 +13,14 @@ void testA() { queue q; constexpr int N = 256; int A[N] = {10}; - kernel_single_task([=]() { - for (int k = 0; k < N; ++k) { - (void)A[k]; - } + q.submit([&](handler &h) { + h.single_task([=]() { + for (int k = 0; k < N; ++k) { + (void)A[k]; + } + }); }); } // CHECK: template <> struct KernelInfo { -// CHECK: static constexpr unsigned long getSize() { return 1024; } +// CHECK: // Returns the size of the kernel object in bytes. +// CHECK: static constexpr unsigned long getKernelObjectSize() { return 1024; } From d728d2615f87da0396257afc3fbe8df5d24b37fa Mon Sep 17 00:00:00 2001 From: Premanand M Rao Date: Wed, 23 Mar 2022 07:56:54 -0700 Subject: [PATCH 5/8] Address more code review comments --- clang/lib/Sema/SemaSYCL.cpp | 10 +++++----- clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index de490cd79ea81..c9a83098a48b2 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -3125,9 +3125,9 @@ class SyclKernelIntHeaderCreator : public SyclKernelFieldHandler { : SyclKernelFieldHandler(S), Header(H) { bool IsSIMDKernel = isESIMDKernelType(KernelObj); // The header needs to access the kernel object size. - unsigned long ObjSize = SemaRef.getASTContext() - .getTypeSizeInChars(KernelObj->getTypeForDecl()) - .getQuantity(); + int64_t ObjSize = SemaRef.getASTContext() + .getTypeSizeInChars(KernelObj->getTypeForDecl()) + .getQuantity(); Header.startKernel(KernelFunc, NameType, KernelObj->getLocation(), IsSIMDKernel, IsSYCLUnnamedKernel(S, KernelFunc), ObjSize); @@ -4784,7 +4784,7 @@ void SYCLIntegrationHeader::emit(raw_ostream &O) { O << " }\n"; O << " // Returns the size of the kernel object in bytes.\n"; O << " __SYCL_DLL_LOCAL\n"; - O << " static constexpr unsigned long getKernelObjectSize() { return " + O << " static constexpr int64_t getKernelSizeof() { return " << K.ObjSize << "; }\n"; O << "};\n"; CurStart += N; @@ -4817,7 +4817,7 @@ void SYCLIntegrationHeader::startKernel(const FunctionDecl *SyclKernel, SourceLocation KernelLocation, bool IsESIMDKernel, bool IsUnnamedKernel, - unsigned long ObjSize) { + int64_t ObjSize) { KernelDescs.emplace_back(SyclKernel, KernelNameType, KernelLocation, IsESIMDKernel, IsUnnamedKernel, ObjSize); } diff --git a/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp b/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp index 9f6c8e7c1c90d..63ba7c8252328 100644 --- a/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp +++ b/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp @@ -1,8 +1,8 @@ // 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 getKernelObjectSize() member function -// is generated into the integration header and that it returns the +// This test checks that the getKernelSizeof() member function is +// generated into the integration header and that it returns the // size of the kernel object in bytes. #include "sycl.hpp" @@ -23,4 +23,4 @@ void testA() { } // CHECK: template <> struct KernelInfo { // CHECK: // Returns the size of the kernel object in bytes. -// CHECK: static constexpr unsigned long getKernelObjectSize() { return 1024; } +// CHECK: static constexpr int64_t getKernelSizeof() { return 1024; } From c95df69f31975dbddfbe75a1193eebe9d883cec7 Mon Sep 17 00:00:00 2001 From: Premanand M Rao Date: Wed, 23 Mar 2022 08:02:03 -0700 Subject: [PATCH 6/8] Missed one file --- clang/include/clang/Sema/Sema.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 6e40891294842..d9ae70b86d99a 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -336,7 +336,7 @@ class SYCLIntegrationHeader { /// the kernel with given name. Starts new kernel invocation descriptor. void startKernel(const FunctionDecl *SyclKernel, QualType KernelNameType, SourceLocation Loc, bool IsESIMD, bool IsUnnamedKernel, - unsigned long ObjSize); + int64_t ObjSize); /// Adds a kernel parameter descriptor to current kernel invocation /// descriptor. @@ -404,11 +404,11 @@ class SYCLIntegrationHeader { bool IsUnnamedKernel; /// Size of the kernel object. - unsigned long ObjSize = 0; + int64_t ObjSize = 0; KernelDesc(const FunctionDecl *SyclKernel, QualType NameType, SourceLocation KernelLoc, bool IsESIMD, bool IsUnnamedKernel, - unsigned long ObjSize) + int64_t ObjSize) : SyclKernel(SyclKernel), NameType(NameType), KernelLocation(KernelLoc), IsESIMDKernel(IsESIMD), IsUnnamedKernel(IsUnnamedKernel), ObjSize(ObjSize) {} From fd7b9d37568d8c7befe790d0007c7bf0d57382e1 Mon Sep 17 00:00:00 2001 From: Premanand M Rao Date: Wed, 23 Mar 2022 12:05:37 -0700 Subject: [PATCH 7/8] Avoid int64_t and fix function name --- clang/lib/Sema/SemaSYCL.cpp | 5 ++++- clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index c9a83098a48b2..40d5684aff4b5 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -4782,9 +4782,12 @@ void SYCLIntegrationHeader::emit(raw_ostream &O) { O << " return 0;\n"; O << "#endif\n"; O << " }\n"; + StringRef ReturnType = + (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 int64_t getKernelSizeof() { return " + O << " static constexpr " << ReturnType << " getKernelSize() { return " << K.ObjSize << "; }\n"; O << "};\n"; CurStart += N; diff --git a/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp b/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp index 63ba7c8252328..a055d26b7062e 100644 --- a/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp +++ b/clang/test/CodeGenSYCL/int_header_kernelobjsize.cpp @@ -1,7 +1,7 @@ // 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 getKernelSizeof() member function is +// 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. @@ -23,4 +23,4 @@ void testA() { } // CHECK: template <> struct KernelInfo { // CHECK: // Returns the size of the kernel object in bytes. -// CHECK: static constexpr int64_t getKernelSizeof() { return 1024; } +// CHECK: static constexpr long{{.*}} getKernelSize() { return 1024; } From e1a845ae4a9b1a8c87779db0767314b98be80e2f Mon Sep 17 00:00:00 2001 From: Premanand M Rao Date: Wed, 23 Mar 2022 14:18:50 -0700 Subject: [PATCH 8/8] clang-format again --- clang/lib/Sema/SemaSYCL.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 40d5684aff4b5..c9e23e30bb7ed 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -4783,8 +4783,9 @@ void SYCLIntegrationHeader::emit(raw_ostream &O) { O << "#endif\n"; O << " }\n"; StringRef ReturnType = - (S.Context.getTargetInfo().getInt64Type() == TargetInfo::SignedLong) ? - "long" : "long long"; + (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 " @@ -4819,8 +4820,7 @@ void SYCLIntegrationHeader::startKernel(const FunctionDecl *SyclKernel, QualType KernelNameType, SourceLocation KernelLocation, bool IsESIMDKernel, - bool IsUnnamedKernel, - int64_t ObjSize) { + bool IsUnnamedKernel, int64_t ObjSize) { KernelDescs.emplace_back(SyclKernel, KernelNameType, KernelLocation, IsESIMDKernel, IsUnnamedKernel, ObjSize); }