From 2457447aa4ca779bbc54ea19fc66fda0635ee3d1 Mon Sep 17 00:00:00 2001 From: Soumi Manna Date: Tue, 26 Mar 2024 06:19:42 -0700 Subject: [PATCH 1/3] [SYCL][FPGA] Allow tablegen handle mutually exclusive decl attrs (SYCLIntelRegister, SYCLIntelMemory) This patch uses MutualExclusions tablegen support to allow us to remove a custom diagnostic checking codes with FPGA attributes: [[intel:fpga_register]] and [[intel::fpga_memory]]. No test is added as we alreday have an existing LIT test (SemaSYCL/local.cpp) that shows the behavior. --- clang/include/clang/Basic/Attr.td | 1 + clang/lib/Sema/SemaDeclAttr.cpp | 21 +-------------------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 4473dde0cb50..e7af14194674 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -2779,6 +2779,7 @@ def SYCLIntelForcePow2Depth : InheritableAttr { let Documentation = [SYCLIntelForcePow2DepthAttrDocs]; } def : MutualExclusions<[SYCLIntelRegister, SYCLIntelForcePow2Depth]>; +def : MutualExclusions<[SYCLIntelRegister, SYCLIntelMemory]>; def Naked : InheritableAttr { let Spellings = [GCC<"naked">, Declspec<"naked">]; diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index c76b280e2940..8f4917c83f58 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -7617,9 +7617,6 @@ static void handleSYCLIntelDoublePumpAttr(Sema &S, Decl *D, /// Handle the [[intel::fpga_memory]] attribute. /// This is incompatible with the [[intel::fpga_register]] attribute. static void handleSYCLIntelMemoryAttr(Sema &S, Decl *D, const ParsedAttr &AL) { - if (checkAttrMutualExclusion(S, D, AL)) - return; - SYCLIntelMemoryAttr::MemoryKind Kind; if (AL.getNumArgs() == 0) Kind = SYCLIntelMemoryAttr::Default; @@ -7665,19 +7662,6 @@ static void handleSYCLIntelMemoryAttr(Sema &S, Decl *D, const ParsedAttr &AL) { D->addAttr(::new (S.Context) SYCLIntelMemoryAttr(S.Context, AL, Kind)); } -/// Check for and diagnose attributes incompatible with register. -/// return true if any incompatible attributes exist. -static bool checkIntelFPGARegisterAttrCompatibility(Sema &S, Decl *D, - const ParsedAttr &Attr) { - bool InCompat = false; - if (auto *MA = D->getAttr()) - if (!MA->isImplicit() && - checkAttrMutualExclusion(S, D, Attr)) - InCompat = true; - - return InCompat; -} - /// Handle the [[intel::fpga_register]] attribute. /// This is incompatible with most of the other memory attributes. static void handleSYCLIntelRegisterAttr(Sema &S, Decl *D, @@ -7705,10 +7689,7 @@ static void handleSYCLIntelRegisterAttr(Sema &S, Decl *D, return; } - if (checkIntelFPGARegisterAttrCompatibility(S, D, A)) - return; - - handleSimpleAttribute(S, D, A); + D->addAttr(::new (S.Context) SYCLIntelRegisterAttr(S.Context, A)); } /// Handle the [[intel::bankwidth]] and [[intel::numbanks]] attributes. From 90355649b436533651762643ddbcc489db9c2a16 Mon Sep 17 00:00:00 2001 From: Soumi Manna Date: Thu, 27 Jun 2024 19:03:55 -0700 Subject: [PATCH 2/3] [SYCL][FPGA] Fix bugs the way we handle duplicate vs conflicting values with loop attributes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch improves diagnostic supports by resolving bugs the way we handle duplicate vs conflicting values with the following SYCL FPGA loop attributes: [[intel::max_reinvocation_delay()]] [[intel::initiation_interval()]] [[intel::max_concurrency()]] [[intel::speculated_iterations()]] [[intel::max_interleaving()]] The patch addresses issues in the test case below, which previously missed diagnostics due to a discontinuation in the while loop while checking for duplicate versus conflicting attribute values in the routine CheckForDuplicateAttrs().    Example with `speculated_iterations` attribute:    Before the fix:     [[intel::speculated_iterations(1)]] // expected-note {{previous attribute is here}}     [[intel::speculated_iterations(1)]] // OK     [[intel::speculated_iterations(2)]] // expected-error {{conflicting loop attribute 'speculated_iterations'}}     [[intel::speculated_iterations(4)]] // OK     for (int i = 0; i != 10; ++i) { a[i] = 0; }     After the fix:     [[intel::speculated_iterations(1)]] // expected-note 2 {{previous attribute is here}}     [[intel::speculated_iterations(1)]] // OK     [[intel::speculated_iterations(2)]] // expected-error {{conflicting loop attribute 'speculated_iterations'}}     [[intel::speculated_iterations(4)]] // expected-error {{conflicting loop attribute 'speculated_iterations'}}     for (int i = 0; i != 10; ++i) { a[i] = 0; } Signed-off-by: Soumi Manna --- clang/lib/Sema/SemaStmtAttr.cpp | 2 +- clang/test/SemaSYCL/intel-fpga-loops.cpp | 77 ++++++++++++++++++++++-- 2 files changed, 72 insertions(+), 7 deletions(-) diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp index a7a7347cfcba..a06792c95b0e 100644 --- a/clang/lib/Sema/SemaStmtAttr.cpp +++ b/clang/lib/Sema/SemaStmtAttr.cpp @@ -1025,9 +1025,9 @@ static void CheckForDuplicateAttrs(Sema &S, ArrayRef Attrs) { S.Diag((*LastFoundItr)->getLocation(), diag::err_loop_attr_conflict) << *FirstItr; S.Diag((*FirstItr)->getLocation(), diag::note_previous_attribute); - return; } } + return; } static void CheckForIncompatibleSYCLLoopAttributes( diff --git a/clang/test/SemaSYCL/intel-fpga-loops.cpp b/clang/test/SemaSYCL/intel-fpga-loops.cpp index 2b1a6d5043c1..53d3e61f69c5 100644 --- a/clang/test/SemaSYCL/intel-fpga-loops.cpp +++ b/clang/test/SemaSYCL/intel-fpga-loops.cpp @@ -311,6 +311,30 @@ void zoo() { [[intel::speculated_iterations(2)]] for (int i = 0; i != 10; ++i) a[i] = 0; + [[intel::speculated_iterations(1)]] // expected-note 2{{previous attribute is here}} + [[intel::speculated_iterations(1)]] // OK + [[intel::speculated_iterations(2)]] // expected-error {{conflicting loop attribute 'speculated_iterations'}} + [[intel::speculated_iterations(4)]] // expected-error {{conflicting loop attribute 'speculated_iterations'}} + for (int i = 0; i != 10; ++i) { a[i] = 0; } + + [[intel::max_interleaving(0)]] // expected-note 2{{previous attribute is here}} + [[intel::max_interleaving(0)]] // OK + [[intel::max_interleaving(1)]] // expected-error {{conflicting loop attribute 'max_interleaving'}} + [[intel::max_interleaving(1)]] // expected-error {{conflicting loop attribute 'max_interleaving'}} + for (int i = 0; i != 10; ++i) { a[i] = 0; } + + [[intel::max_concurrency(10)]] // expected-note 2{{previous attribute is here}} + [[intel::max_concurrency(10)]] // OK + [[intel::max_concurrency(20)]] // expected-error {{conflicting loop attribute 'max_concurrency'}} + [[intel::max_concurrency(40)]] // expected-error {{conflicting loop attribute 'max_concurrency'}} + for (int i = 0; i != 10; ++i) { a[i] = 0; } + + [[intel::initiation_interval(10)]] // expected-note 2{{previous attribute is here}} + [[intel::initiation_interval(10)]] // OK + [[intel::initiation_interval(20)]] // expected-error {{conflicting loop attribute 'initiation_interval'}} + [[intel::initiation_interval(40)]] // expected-error {{conflicting loop attribute 'initiation_interval'}} + for (int i = 0; i != 10; ++i) { a[i] = 0; } + [[intel::ivdep]] // expected-warning@+2 {{ignoring redundant Intel FPGA loop attribute 'ivdep': safelen INF >= safelen INF}} // expected-note@-2 {{previous attribute is here}} @@ -383,6 +407,12 @@ void zoo() { [[intel::max_reinvocation_delay(20)]] for (int i = 0; i != 10; ++i) a[i] = 0; + [[intel::max_reinvocation_delay(10)]] // expected-note 2{{previous attribute is here}} + [[intel::max_reinvocation_delay(10)]] // OK + [[intel::max_reinvocation_delay(20)]] // expected-error {{conflicting loop attribute 'max_reinvocation_delay'}} + [[intel::max_reinvocation_delay(40)]] // expected-error {{conflicting loop attribute 'max_reinvocation_delay'}} + for (int i = 0; i != 10; ++i) { a[i] = 0; } + [[intel::enable_loop_pipelining]] // expected-error@+1 {{duplicate Intel FPGA loop attribute 'enable_loop_pipelining'}} [[intel::enable_loop_pipelining]] for (int i = 0; i != 10; ++i) @@ -476,7 +506,7 @@ void ivdep_dependent() { }; } -template +template void ii_dependent() { int a[10]; // expected-error@+1 {{'initiation_interval' attribute requires a positive integral compile time constant expression}} @@ -491,6 +521,13 @@ void ii_dependent() { [[intel::initiation_interval(A)]] // expected-note {{previous attribute is here}} [[intel::initiation_interval(B)]] for (int i = 0; i != 10; ++i) a[i] = 0; + + [[intel::initiation_interval(A)]] // expected-note 2{{previous attribute is here}} + [[intel::initiation_interval(A)]] // OK + [[intel::initiation_interval(B)]] // expected-error {{conflicting loop attribute 'initiation_interval'}} + [[intel::initiation_interval(D)]] // expected-error {{conflicting loop attribute 'initiation_interval'}} + for (int i = 0; i != 10; ++i) { a[i] = 0; } + } template @@ -515,6 +552,13 @@ void max_concurrency_dependent() { // max_concurrency attribute accepts value 0. [[intel::max_concurrency(D)]] for (int i = 0; i != 10; ++i) a[i] = 0; + + [[intel::max_concurrency(D)]] // expected-note 2{{previous attribute is here}} + [[intel::max_concurrency(D)]] // OK + [[intel::max_concurrency(A)]] // expected-error {{conflicting loop attribute 'max_concurrency'}} + [[intel::max_concurrency(B)]] // expected-error {{conflicting loop attribute 'max_concurrency'}} + for (int i = 0; i != 10; ++i) { a[i] = 0; } + } template @@ -540,9 +584,16 @@ void max_interleaving_dependent() { [[intel::max_interleaving(D)]] [[intel::max_interleaving(D)]] for (int i = 0; i != 10; ++i) a[i] = 0; + + [[intel::max_interleaving(D)]] // expected-note 2{{previous attribute is here}} + [[intel::max_interleaving(D)]] // OK + [[intel::max_interleaving(C)]] // expected-error {{conflicting loop attribute 'max_interleaving'}} + [[intel::max_interleaving(C)]] // expected-error {{conflicting loop attribute 'max_interleaving'}} + for (int i = 0; i != 10; ++i) { a[i] = 0; } + } -template +template void speculated_iterations_dependent() { int a[10]; // expected-error@+1 {{'speculated_iterations' attribute requires a non-negative integral compile time constant expression}} @@ -561,6 +612,13 @@ void speculated_iterations_dependent() { [[intel::speculated_iterations(B)]] [[intel::speculated_iterations(B)]] for (int i = 0; i != 10; ++i) a[i] = 0; + + [[intel::speculated_iterations(A)]] // expected-note 2{{previous attribute is here}} + [[intel::speculated_iterations(A)]] // OK + [[intel::speculated_iterations(B)]] // expected-error {{conflicting loop attribute 'speculated_iterations'}} + [[intel::speculated_iterations(E)]] // expected-error {{conflicting loop attribute 'speculated_iterations'}} + for (int i = 0; i != 10; ++i) { a[i] = 0; } + } template @@ -624,7 +682,7 @@ void loop_count_control_dependent() { a[i] = 0; } -template +template void max_reinvocation_delay_dependent() { int a[10]; // expected-error@+1 {{'max_reinvocation_delay' attribute requires a positive integral compile time constant expression}} @@ -639,6 +697,13 @@ void max_reinvocation_delay_dependent() { [[intel::max_reinvocation_delay(A)]] [[intel::max_reinvocation_delay(A)]] for (int i = 0; i != 10; ++i) a[i] = 0; + + [[intel::max_reinvocation_delay(A)]] // expected-note 2{{previous attribute is here}} + [[intel::max_reinvocation_delay(A)]] // OK + [[intel::max_reinvocation_delay(B)]] // expected-error {{conflicting loop attribute 'max_reinvocation_delay'}} + [[intel::max_reinvocation_delay(D)]] // expected-error {{conflicting loop attribute 'max_reinvocation_delay'}} + for (int i = 0; i != 10; ++i) { a[i] = 0; } + } void check_max_concurrency_expression() { @@ -815,14 +880,14 @@ int main() { //expected-note@-1 +{{in instantiation of function template specialization}} ivdep_dependent<2, 4, -1>(); //expected-note@-1 +{{in instantiation of function template specialization}} - ii_dependent<2, 4, -1>(); + ii_dependent<2, 4, -1, 8>(); //expected-note@-1 +{{in instantiation of function template specialization}} max_concurrency_dependent<1, 4, -2, 0>(); // expected-note{{in instantiation of function template specialization 'max_concurrency_dependent<1, 4, -2, 0>' requested here}} max_interleaving_dependent<-1, 4, 0, 1>(); // expected-note{{in instantiation of function template specialization 'max_interleaving_dependent<-1, 4, 0, 1>' requested here}} - speculated_iterations_dependent<1, 8, -3, 0>(); // expected-note{{in instantiation of function template specialization 'speculated_iterations_dependent<1, 8, -3, 0>' requested here}} + speculated_iterations_dependent<1, 8, -3, 0, 16>(); // expected-note{{in instantiation of function template specialization 'speculated_iterations_dependent<1, 8, -3, 0, 16>' requested here}} loop_coalesce_dependent<-1, 4, 0>(); // expected-note{{in instantiation of function template specialization 'loop_coalesce_dependent<-1, 4, 0>' requested here}} loop_count_control_dependent<3, 2, -1>(); // expected-note{{in instantiation of function template specialization 'loop_count_control_dependent<3, 2, -1>' requested here}} - max_reinvocation_delay_dependent<1, 3, 0>(); // expected-note{{in instantiation of function template specialization 'max_reinvocation_delay_dependent<1, 3, 0>' requested here}} + max_reinvocation_delay_dependent<1, 3, 0, 6>(); // expected-note{{in instantiation of function template specialization 'max_reinvocation_delay_dependent<1, 3, 0, 6>' requested here}} check_max_concurrency_expression(); check_max_interleaving_expression(); check_speculated_iterations_expression(); From 6e3789454bdd3e0f029f4e4f8243adc01ece3d01 Mon Sep 17 00:00:00 2001 From: Soumi Manna Date: Fri, 28 Jun 2024 12:20:43 -0700 Subject: [PATCH 3/3] Address review comments --- clang/lib/Sema/SemaStmtAttr.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp index a06792c95b0e..dcee4506e99f 100644 --- a/clang/lib/Sema/SemaStmtAttr.cpp +++ b/clang/lib/Sema/SemaStmtAttr.cpp @@ -1027,7 +1027,6 @@ static void CheckForDuplicateAttrs(Sema &S, ArrayRef Attrs) { S.Diag((*FirstItr)->getLocation(), diag::note_previous_attribute); } } - return; } static void CheckForIncompatibleSYCLLoopAttributes(