From 7fdfe12dd777a4c498e0b897a7e00769af269919 Mon Sep 17 00:00:00 2001 From: Soumi Manna Date: Mon, 21 Sep 2020 22:28:55 -0700 Subject: [PATCH 1/7] [SYCL] Add template parameter support for num_simd_work_items attribute This patch 1. adds support for template parameter on [[intelfpga::num_simd_work_items()]] attribute 2. splits test/SemaSYCL/num_simd_work_items.cpp to separate files for host compilation (test/SemaSYCL/num_simd_work_items_host.cpp) and device compilation (test/SemaSYCL/num_simd_work_items_device.cpp) 3. makes changes to intel_reqd_sub_group_size attribute source codes to avoid duplication and reuse for the template parameter support. Signed-off-by: Soumi Manna --- clang/include/clang/Basic/Attr.td | 4 +- clang/include/clang/Sema/Sema.h | 32 ++++++++++++-- clang/lib/CodeGen/CodeGenFunction.cpp | 10 +++-- clang/lib/Sema/SemaDeclAttr.cpp | 42 ++----------------- clang/lib/Sema/SemaSYCL.cpp | 6 +-- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 20 +++++---- .../test/CodeGenSYCL/num-simd-work-items.cpp | 12 +++++- ...ems.cpp => num_simd_work_items_device.cpp} | 15 +++---- .../SemaSYCL/num_simd_work_items_host.cpp | 8 ++++ ...cl-device-num_simd_work_items-template.cpp | 25 +++++++++++ 10 files changed, 108 insertions(+), 66 deletions(-) rename clang/test/SemaSYCL/{num_simd_work_items.cpp => num_simd_work_items_device.cpp} (78%) create mode 100644 clang/test/SemaSYCL/num_simd_work_items_host.cpp create mode 100644 clang/test/SemaSYCL/sycl-device-num_simd_work_items-template.cpp diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 6e1d15bed74e6..c0a5f90e79491 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1209,7 +1209,7 @@ def SYCLIntelKernelArgsRestrict : InheritableAttr { def SYCLIntelNumSimdWorkItems : InheritableAttr { let Spellings = [CXX11<"intelfpga","num_simd_work_items">]; - let Args = [UnsignedArgument<"Number">]; + let Args = [ExprArgument<"Value">]; let LangOpts = [SYCLIsDevice, SYCLIsHost]; let Subjects = SubjectList<[Function], ErrorDiag>; let Documentation = [SYCLIntelNumSimdWorkItemsAttrDocs]; @@ -1300,7 +1300,7 @@ def LoopUnrollHint : InheritableAttr { def IntelReqdSubGroupSize: InheritableAttr { let Spellings = [GNU<"intel_reqd_sub_group_size">, CXX11<"intel", "reqd_sub_group_size">]; - let Args = [ExprArgument<"SubGroupSize">]; + let Args = [ExprArgument<"Value">]; let Subjects = SubjectList<[Function, CXXMethod], ErrorDiag>; let Documentation = [IntelReqdSubGroupSizeDocs]; let LangOpts = [OpenCL, SYCLIsDevice, SYCLIsHost]; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 2632a92f91764..00114c84a6a05 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -9986,6 +9986,8 @@ class Sema final { Expr *E); void AddIntelFPGABankBitsAttr(Decl *D, const AttributeCommonInfo &CI, Expr **Exprs, unsigned Size); + template + void addIntelSYCLFunctionAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E); /// AddAlignedAttr - Adds an aligned attribute to a particular declaration. void AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E, @@ -10041,10 +10043,6 @@ class Sema final { bool checkAllowedSYCLInitializer(VarDecl *VD, bool CheckValueDependent = false); - // Adds an intel_reqd_sub_group_size attribute to a particular declaration. - void addIntelReqdSubGroupSizeAttr(Decl *D, const AttributeCommonInfo &CI, - Expr *E); - //===--------------------------------------------------------------------===// // C++ Coroutines TS // @@ -12836,6 +12834,32 @@ class Sema final { } }; +template +void Sema::addIntelSYCLFunctionAttr(Decl *D, + const AttributeCommonInfo &Attr, + Expr *E) { + if (!E) + return; + + if (!E->isInstantiationDependent()) { + Optional ArgVal = E->getIntegerConstantExpr(getASTContext()); + if (!ArgVal) { + Diag(E->getExprLoc(), diag::err_attribute_argument_type) + << Attr.getAttrName() << AANT_ArgumentIntegerConstant + << E->getSourceRange(); + return; + } + int32_t ArgInt = ArgVal->getSExtValue(); + if (ArgInt <= 0) { + Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer) + << Attr.getAttrName() << /*positive*/ 0; + return; + } + } + + D->addAttr(::new (Context) AttrType(Context, Attr, E)); +} + template void Sema::AddOneConstantValueAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E) { diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 5c9f60ae5f0df..cd42d8a64f9c5 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -611,7 +611,7 @@ void CodeGenFunction::EmitOpenCLKernelMetadata(const FunctionDecl *FD, FD->getAttr()) { llvm::LLVMContext &Context = getLLVMContext(); Optional ArgVal = - A->getSubGroupSize()->getIntegerConstantExpr(FD->getASTContext()); + A->getValue()->getIntegerConstantExpr(FD->getASTContext()); assert(ArgVal.hasValue() && "Not an integer constant expression"); llvm::Metadata *AttrMDArgs[] = {llvm::ConstantAsMetadata::get( Builder.getInt32(ArgVal->getSExtValue()))}; @@ -629,8 +629,12 @@ void CodeGenFunction::EmitOpenCLKernelMetadata(const FunctionDecl *FD, if (const SYCLIntelNumSimdWorkItemsAttr *A = FD->getAttr()) { - llvm::Metadata *AttrMDArgs[] = { - llvm::ConstantAsMetadata::get(Builder.getInt32(A->getNumber()))}; + llvm::LLVMContext &Context = getLLVMContext(); + Optional ArgVal = + A->getValue()->getIntegerConstantExpr(FD->getASTContext()); + assert(ArgVal.hasValue() && "Not an integer constant expression"); + llvm::Metadata *AttrMDArgs[] = {llvm::ConstantAsMetadata::get( + Builder.getInt32(ArgVal->getSExtValue()))}; Fn->setMetadata("num_simd_work_items", llvm::MDNode::get(Context, AttrMDArgs)); } diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 9aabf4aae0b4f..fe5222dc17e00 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -2979,31 +2979,6 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) { } // Handles intel_reqd_sub_group_size. -void Sema::addIntelReqdSubGroupSizeAttr(Decl *D, - const AttributeCommonInfo &Attr, - Expr *E) { - if (!E) - return; - - if (!E->isInstantiationDependent()) { - Optional ArgVal = E->getIntegerConstantExpr(getASTContext()); - if (!ArgVal) { - Diag(E->getExprLoc(), diag::err_attribute_argument_type) - << Attr.getAttrName() << AANT_ArgumentIntegerConstant - << E->getSourceRange(); - return; - } - int32_t ArgInt = ArgVal->getSExtValue(); - if (ArgInt <= 0) { - Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer) - << Attr.getAttrName() << /*positive*/ 0; - return; - } - } - - D->addAttr(::new (Context) IntelReqdSubGroupSizeAttr(Context, Attr, E)); -} - static void handleSubGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) { if (S.LangOpts.SYCLIsHost) return; @@ -3013,7 +2988,7 @@ static void handleSubGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) { if (D->getAttr()) S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL; - S.addIntelReqdSubGroupSizeAttr(D, AL, E); + S.addIntelSYCLFunctionAttr(D, AL, E); } // Handles num_simd_work_items. @@ -3022,23 +2997,12 @@ static void handleNumSimdWorkItemsAttr(Sema &S, Decl *D, if (D->isInvalidDecl()) return; - uint32_t NumSimdWorkItems = 0; - const Expr *E = Attr.getArgAsExpr(0); - if (!checkUInt32Argument(S, Attr, E, NumSimdWorkItems, 0, - /*StrictlyUnsigned=*/true)) - return; - - if (NumSimdWorkItems == 0) { - S.Diag(Attr.getLoc(), diag::err_attribute_argument_is_zero) - << Attr << E->getSourceRange(); - return; - } + Expr *E = Attr.getArgAsExpr(0); if (D->getAttr()) S.Diag(Attr.getLoc(), diag::warn_duplicate_attribute) << Attr; - D->addAttr(::new (S.Context) SYCLIntelNumSimdWorkItemsAttr( - S.Context, Attr, NumSimdWorkItems)); + S.addIntelSYCLFunctionAttr(D, Attr, E); } // Handles max_global_work_dim. diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 11efd7577a2e9..a956e0b62d3df 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -2740,15 +2740,15 @@ void Sema::MarkDevice(void) { KernelBody ? KernelBody->getAttr() : nullptr; if (auto *Existing = SYCLKernel->getAttr()) { - if (getIntExprValue(Existing->getSubGroupSize(), getASTContext()) != - getIntExprValue(Attr->getSubGroupSize(), getASTContext())) { + if (getIntExprValue(Existing->getValue(), getASTContext()) != + getIntExprValue(Attr->getValue(), getASTContext())) { Diag(SYCLKernel->getLocation(), diag::err_conflicting_sycl_kernel_attributes); Diag(Existing->getLocation(), diag::note_conflicting_attribute); Diag(Attr->getLocation(), diag::note_conflicting_attribute); SYCLKernel->setInvalidDecl(); } - } else if (KBSimdAttr && (getIntExprValue(Attr->getSubGroupSize(), + } else if (KBSimdAttr && (getIntExprValue(Attr->getValue(), getASTContext()) != 1)) { reportConflictingAttrs(*this, KernelBody, KBSimdAttr, Attr); } else { diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index a9526671fd5a3..7fa7b6edef2c5 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -541,15 +541,15 @@ static void instantiateSYCLIntelPipeIOAttr( S.addSYCLIntelPipeIOAttr(New, *Attr, Result.getAs()); } -static void instantiateIntelReqdSubGroupSizeAttr( +template +static void instantiateIntelSYCLFunctionAttr( Sema &S, const MultiLevelTemplateArgumentList &TemplateArgs, - const IntelReqdSubGroupSizeAttr *Attr, Decl *New) { - // The SubGroupSize expression is a constant expression. + const AttrName *Attr, Decl *New) { EnterExpressionEvaluationContext Unevaluated( S, Sema::ExpressionEvaluationContext::ConstantEvaluated); - ExprResult Result = S.SubstExpr(Attr->getSubGroupSize(), TemplateArgs); + ExprResult Result = S.SubstExpr(Attr->getValue(), TemplateArgs); if (!Result.isInvalid()) - S.addIntelReqdSubGroupSizeAttr(New, *Attr, Result.getAs()); + S.addIntelSYCLFunctionAttr(New, *Attr, Result.getAs()); } void Sema::InstantiateAttrsForDecl( @@ -697,8 +697,14 @@ void Sema::InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs, } if (const auto *IntelReqdSubGroupSize = dyn_cast(TmplAttr)) { - instantiateIntelReqdSubGroupSizeAttr(*this, TemplateArgs, - IntelReqdSubGroupSize, New); + instantiateIntelSYCLFunctionAttr( + *this, TemplateArgs, IntelReqdSubGroupSize, New); + continue; + } + if (const auto *SYCLIntelNumSimdWorkItems = + dyn_cast(TmplAttr)) { + instantiateIntelSYCLFunctionAttr( + *this, TemplateArgs, SYCLIntelNumSimdWorkItems, New); continue; } // Existing DLL attribute on the instantiation takes precedence. diff --git a/clang/test/CodeGenSYCL/num-simd-work-items.cpp b/clang/test/CodeGenSYCL/num-simd-work-items.cpp index 5613969b88a91..1667ad877ba51 100644 --- a/clang/test/CodeGenSYCL/num-simd-work-items.cpp +++ b/clang/test/CodeGenSYCL/num-simd-work-items.cpp @@ -5,6 +5,12 @@ class Foo { [[intelfpga::num_simd_work_items(1)]] void operator()() const {} }; +template +class Functor { +public: + [[intelfpga::num_simd_work_items(SIZE)]] void operator()() const {} +}; + template __attribute__((sycl_kernel)) void kernel(const Func &kernelFunc) { kernelFunc(); @@ -17,10 +23,14 @@ void bar() { kernel( []() [[intelfpga::num_simd_work_items(42)]] {}); + Functor<2> f; + kernel(f); + } // CHECK: define spir_kernel void @{{.*}}kernel_name1() {{.*}} !num_simd_work_items ![[NUM1:[0-9]+]] // CHECK: define spir_kernel void @{{.*}}kernel_name2() {{.*}} !num_simd_work_items ![[NUM42:[0-9]+]] +// CHECK: define spir_kernel void @{{.*}}kernel_name3() {{.*}} !num_simd_work_items ![[NUM2:[0-9]+]] // CHECK: ![[NUM1]] = !{i32 1} // CHECK: ![[NUM42]] = !{i32 42} - +// CHECK: ![[NUM2]] = !{i32 2} diff --git a/clang/test/SemaSYCL/num_simd_work_items.cpp b/clang/test/SemaSYCL/num_simd_work_items_device.cpp similarity index 78% rename from clang/test/SemaSYCL/num_simd_work_items.cpp rename to clang/test/SemaSYCL/num_simd_work_items_device.cpp index d6528ec4e665d..8cfcac9247aab 100644 --- a/clang/test/SemaSYCL/num_simd_work_items.cpp +++ b/clang/test/SemaSYCL/num_simd_work_items_device.cpp @@ -1,6 +1,5 @@ // RUN: %clang_cc1 %s -fsycl -fsycl-is-device -triple spir64 -fsyntax-only -Wno-sycl-2017-compat -DTRIGGER_ERROR -verify // RUN: %clang_cc1 %s -fsycl -fsycl-is-device -triple spir64 -fsyntax-only -Wno-sycl-2017-compat -ast-dump | FileCheck %s -// RUN: %clang_cc1 -fsycl -fsycl-is-host -fsyntax-only -Wno-sycl-2017-compat -verify %s #ifndef __SYCL_DEVICE_ONLY__ struct FuncObj { @@ -20,7 +19,6 @@ void foo() { } #else // __SYCL_DEVICE_ONLY__ - [[intelfpga::num_simd_work_items(2)]] void func_do_not_ignore() {} struct FuncObj { @@ -34,17 +32,20 @@ __attribute__((sycl_kernel)) void kernel(const Func &kernelFunc) { int main() { // CHECK-LABEL: FunctionDecl {{.*}}test_kernel1 - // CHECK: SYCLIntelNumSimdWorkItemsAttr {{.*}} 42 + // CHECK: SYCLIntelNumSimdWorkItemsAttr {{.*}} + // CHECK-NEXT: IntegerLiteral{{.*}}42{{$}} kernel( FuncObj()); // CHECK-LABEL: FunctionDecl {{.*}}test_kernel2 - // CHECK: SYCLIntelNumSimdWorkItemsAttr {{.*}} 8 + // CHECK: SYCLIntelNumSimdWorkItemsAttr {{.*}} + // CHECK-NEXT: IntegerLiteral{{.*}}8{{$}} kernel( []() [[intelfpga::num_simd_work_items(8)]] {}); // CHECK-LABEL: FunctionDecl {{.*}}test_kernel3 - // CHECK: SYCLIntelNumSimdWorkItemsAttr {{.*}} 2 + // CHECK: SYCLIntelNumSimdWorkItemsAttr {{.*}} + // CHECK-NEXT: IntegerLiteral{{.*}}2{{$}} kernel( []() { func_do_not_ignore(); }); @@ -52,10 +53,10 @@ int main() { [[intelfpga::num_simd_work_items(0)]] int Var = 0; // expected-error{{'num_simd_work_items' attribute only applies to functions}} kernel( - []() [[intelfpga::num_simd_work_items(0)]] {}); // expected-error{{'num_simd_work_items' attribute must be greater than 0}} + []() [[intelfpga::num_simd_work_items(0)]] {}); // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}} kernel( - []() [[intelfpga::num_simd_work_items(-42)]] {}); // expected-error{{'num_simd_work_items' attribute requires a non-negative integral compile time constant expression}} + []() [[intelfpga::num_simd_work_items(-42)]] {}); // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}} kernel( []() [[intelfpga::num_simd_work_items(1), intelfpga::num_simd_work_items(2)]] {}); // expected-warning{{attribute 'num_simd_work_items' is already applied with different parameters}} diff --git a/clang/test/SemaSYCL/num_simd_work_items_host.cpp b/clang/test/SemaSYCL/num_simd_work_items_host.cpp new file mode 100644 index 0000000000000..c447cc1d6a0c4 --- /dev/null +++ b/clang/test/SemaSYCL/num_simd_work_items_host.cpp @@ -0,0 +1,8 @@ +// RUN: %clang_cc1 -fsycl -fsycl-is-host -fsyntax-only -Wno-sycl-2017-compat -verify %s +// expected-no-diagnostics + +[[intelfpga::num_simd_work_items(2)]] void func_do_not_ignore() {} + +struct FuncObj { + [[intelfpga::num_simd_work_items(42)]] void operator()() const {} +}; diff --git a/clang/test/SemaSYCL/sycl-device-num_simd_work_items-template.cpp b/clang/test/SemaSYCL/sycl-device-num_simd_work_items-template.cpp new file mode 100644 index 0000000000000..6e100636cbcb2 --- /dev/null +++ b/clang/test/SemaSYCL/sycl-device-num_simd_work_items-template.cpp @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -fsycl -fsycl-is-device -fsyntax-only -ast-dump -verify -pedantic %s | FileCheck %s + +// Test that checkes template parameter support for 'num_simd_work_items' attribute on sycl device. + +template +class KernelFunctor { +public: + // expected-error@+1{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}} + [[intelfpga::num_simd_work_items(SIZE)]] void operator()() {} +}; + +int main() { + //expected-note@+1{{in instantiation of template class 'KernelFunctor<-1>' requested here}} + KernelFunctor<-1>(); + // no error expected + KernelFunctor<10>(); +} + +// CHECK: ClassTemplateDecl {{.*}} {{.*}} KernelFunctor +// CHECK: ClassTemplateSpecializationDecl {{.*}} {{.*}} class KernelFunctor definition +// CHECK: CXXRecordDecl {{.*}} {{.*}} implicit class KernelFunctor +// CHECK: SYCLIntelNumSimdWorkItemsAttr {{.*}} +// CHECK: SubstNonTypeTemplateParmExpr {{.*}} +// CHECK-NEXT: NonTypeTemplateParmDecl {{.*}} +// CHECK-NEXT: IntegerLiteral{{.*}}10{{$}} From eff12586abb0f70ba02a222fe1a67d2c6d1c9bbe Mon Sep 17 00:00:00 2001 From: Soumi Manna Date: Mon, 21 Sep 2020 22:48:14 -0700 Subject: [PATCH 2/7] Fix Clang-format errors Signed-off-by: Soumi Manna --- clang/include/clang/Sema/Sema.h | 7 +++---- clang/lib/Sema/SemaDeclAttr.cpp | 2 +- clang/test/CodeGenSYCL/num-simd-work-items.cpp | 1 - clang/test/SemaSYCL/num_simd_work_items_device.cpp | 4 ++-- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 00114c84a6a05..6bccc7b8689e0 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -9987,8 +9987,8 @@ class Sema final { void AddIntelFPGABankBitsAttr(Decl *D, const AttributeCommonInfo &CI, Expr **Exprs, unsigned Size); template - void addIntelSYCLFunctionAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E); - + void addIntelSYCLFunctionAttr(Decl *D, const AttributeCommonInfo &CI, + Expr *E); /// AddAlignedAttr - Adds an aligned attribute to a particular declaration. void AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E, bool IsPackExpansion); @@ -12835,8 +12835,7 @@ class Sema final { }; template -void Sema::addIntelSYCLFunctionAttr(Decl *D, - const AttributeCommonInfo &Attr, +void Sema::addIntelSYCLFunctionAttr(Decl *D, const AttributeCommonInfo &Attr, Expr *E) { if (!E) return; diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index fe5222dc17e00..3bd6bdc3fbb32 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3002,7 +3002,7 @@ static void handleNumSimdWorkItemsAttr(Sema &S, Decl *D, if (D->getAttr()) S.Diag(Attr.getLoc(), diag::warn_duplicate_attribute) << Attr; - S.addIntelSYCLFunctionAttr(D, Attr, E); + S.addIntelSYCLFunctionAttr(D, Attr, E); } // Handles max_global_work_dim. diff --git a/clang/test/CodeGenSYCL/num-simd-work-items.cpp b/clang/test/CodeGenSYCL/num-simd-work-items.cpp index 1667ad877ba51..b88cfbf5ff917 100644 --- a/clang/test/CodeGenSYCL/num-simd-work-items.cpp +++ b/clang/test/CodeGenSYCL/num-simd-work-items.cpp @@ -25,7 +25,6 @@ void bar() { Functor<2> f; kernel(f); - } // CHECK: define spir_kernel void @{{.*}}kernel_name1() {{.*}} !num_simd_work_items ![[NUM1:[0-9]+]] diff --git a/clang/test/SemaSYCL/num_simd_work_items_device.cpp b/clang/test/SemaSYCL/num_simd_work_items_device.cpp index 8cfcac9247aab..4659183cb73e7 100644 --- a/clang/test/SemaSYCL/num_simd_work_items_device.cpp +++ b/clang/test/SemaSYCL/num_simd_work_items_device.cpp @@ -53,10 +53,10 @@ int main() { [[intelfpga::num_simd_work_items(0)]] int Var = 0; // expected-error{{'num_simd_work_items' attribute only applies to functions}} kernel( - []() [[intelfpga::num_simd_work_items(0)]] {}); // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}} + []() [[intelfpga::num_simd_work_items(0)]]{}); // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}} kernel( - []() [[intelfpga::num_simd_work_items(-42)]] {}); // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}} + []() [[intelfpga::num_simd_work_items(-42)]]{}); // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}} kernel( []() [[intelfpga::num_simd_work_items(1), intelfpga::num_simd_work_items(2)]] {}); // expected-warning{{attribute 'num_simd_work_items' is already applied with different parameters}} From cbab06d1b7a6f796b4bcf55dffae1a2be302a619 Mon Sep 17 00:00:00 2001 From: Soumi Manna Date: Tue, 22 Sep 2020 05:46:22 -0700 Subject: [PATCH 3/7] address review comments Signed-off-by: Soumi Manna --- clang/include/clang/Sema/Sema.h | 15 ++++++++------- clang/lib/Sema/SemaDeclAttr.cpp | 4 ++-- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 3 ++- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 6bccc7b8689e0..4cb43cb8d3f5d 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -9987,8 +9987,8 @@ class Sema final { void AddIntelFPGABankBitsAttr(Decl *D, const AttributeCommonInfo &CI, Expr **Exprs, unsigned Size); template - void addIntelSYCLFunctionAttr(Decl *D, const AttributeCommonInfo &CI, - Expr *E); + void addIntelSYCLSingleArgFunctionAttr(Decl *D, + const AttributeCommonInfo &CI, Expr *E); /// AddAlignedAttr - Adds an aligned attribute to a particular declaration. void AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E, bool IsPackExpansion); @@ -12835,8 +12835,9 @@ class Sema final { }; template -void Sema::addIntelSYCLFunctionAttr(Decl *D, const AttributeCommonInfo &Attr, - Expr *E) { +void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D, + const AttributeCommonInfo &CI, + Expr *E) { if (!E) return; @@ -12844,19 +12845,19 @@ void Sema::addIntelSYCLFunctionAttr(Decl *D, const AttributeCommonInfo &Attr, Optional ArgVal = E->getIntegerConstantExpr(getASTContext()); if (!ArgVal) { Diag(E->getExprLoc(), diag::err_attribute_argument_type) - << Attr.getAttrName() << AANT_ArgumentIntegerConstant + << CI.getAttrName() << AANT_ArgumentIntegerConstant << E->getSourceRange(); return; } int32_t ArgInt = ArgVal->getSExtValue(); if (ArgInt <= 0) { Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer) - << Attr.getAttrName() << /*positive*/ 0; + << CI.getAttrName() << /*positive*/ 0; return; } } - D->addAttr(::new (Context) AttrType(Context, Attr, E)); + D->addAttr(::new (Context) AttrType(Context, CI, E)); } template diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 3bd6bdc3fbb32..fa72e1a086f9f 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -2988,7 +2988,7 @@ static void handleSubGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) { if (D->getAttr()) S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL; - S.addIntelSYCLFunctionAttr(D, AL, E); + S.addIntelSYCLSingleArgFunctionAttr(D, AL, E); } // Handles num_simd_work_items. @@ -3002,7 +3002,7 @@ static void handleNumSimdWorkItemsAttr(Sema &S, Decl *D, if (D->getAttr()) S.Diag(Attr.getLoc(), diag::warn_duplicate_attribute) << Attr; - S.addIntelSYCLFunctionAttr(D, Attr, E); + S.addIntelSYCLSingleArgFunctionAttr(D, Attr, E); } // Handles max_global_work_dim. diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 7fa7b6edef2c5..fd1b49cb4dc0d 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -549,7 +549,8 @@ static void instantiateIntelSYCLFunctionAttr( S, Sema::ExpressionEvaluationContext::ConstantEvaluated); ExprResult Result = S.SubstExpr(Attr->getValue(), TemplateArgs); if (!Result.isInvalid()) - S.addIntelSYCLFunctionAttr(New, *Attr, Result.getAs()); + S.addIntelSYCLSingleArgFunctionAttr(New, *Attr, + Result.getAs()); } void Sema::InstantiateAttrsForDecl( From bb70e24df5044dc77f9dec321d33836f9654d983 Mon Sep 17 00:00:00 2001 From: Soumi Manna Date: Tue, 22 Sep 2020 05:52:48 -0700 Subject: [PATCH 4/7] Fix clang-format errors Signed-off-by: Soumi Manna --- clang/include/clang/Sema/Sema.h | 4 ++-- clang/lib/Sema/SemaDeclAttr.cpp | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 4cb43cb8d3f5d..9bee54a208974 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -9987,8 +9987,8 @@ class Sema final { void AddIntelFPGABankBitsAttr(Decl *D, const AttributeCommonInfo &CI, Expr **Exprs, unsigned Size); template - void addIntelSYCLSingleArgFunctionAttr(Decl *D, - const AttributeCommonInfo &CI, Expr *E); + void addIntelSYCLSingleArgFunctionAttr(Decl *D, const AttributeCommonInfo &CI, + Expr *E); /// AddAlignedAttr - Adds an aligned attribute to a particular declaration. void AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E, bool IsPackExpansion); diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index fa72e1a086f9f..f3b9fb4023b87 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3002,7 +3002,8 @@ static void handleNumSimdWorkItemsAttr(Sema &S, Decl *D, if (D->getAttr()) S.Diag(Attr.getLoc(), diag::warn_duplicate_attribute) << Attr; - S.addIntelSYCLSingleArgFunctionAttr(D, Attr, E); + S.addIntelSYCLSingleArgFunctionAttr(D, Attr, + E); } // Handles max_global_work_dim. From 533d9a1389a2683f8c0627ad32c621f008de56e2 Mon Sep 17 00:00:00 2001 From: Soumi Manna Date: Tue, 22 Sep 2020 09:20:06 -0700 Subject: [PATCH 5/7] Add assert based on review comment Signed-off-by: Soumi Manna --- clang/include/clang/Sema/Sema.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 9bee54a208974..86a0e025a2c07 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -12838,8 +12838,7 @@ template void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E) { - if (!E) - return; + assert (!E); if (!E->isInstantiationDependent()) { Optional ArgVal = E->getIntegerConstantExpr(getASTContext()); From bf8f3d9a5507748607a83497218db53908a9832f Mon Sep 17 00:00:00 2001 From: Soumi Manna Date: Tue, 22 Sep 2020 09:26:16 -0700 Subject: [PATCH 6/7] Fix clang-format error Signed-off-by: Soumi Manna --- clang/include/clang/Sema/Sema.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 86a0e025a2c07..05cecf3f5da1d 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -12838,7 +12838,7 @@ template void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E) { - assert (!E); + assert(!E); if (!E->isInstantiationDependent()) { Optional ArgVal = E->getIntegerConstantExpr(getASTContext()); From 19a354218446032bad34f5272b23568900875389 Mon Sep 17 00:00:00 2001 From: Soumi Manna Date: Tue, 22 Sep 2020 10:04:16 -0700 Subject: [PATCH 7/7] Fix assertion Signed-off-by: Soumi Manna --- clang/include/clang/Sema/Sema.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 05cecf3f5da1d..af8ddf40ff12a 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -12838,7 +12838,7 @@ template void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E) { - assert(!E); + assert(E && "Attribute must have an argument."); if (!E->isInstantiationDependent()) { Optional ArgVal = E->getIntegerConstantExpr(getASTContext());