From b0ecb56b97b077ff11b4f0f55d61dc056753e717 Mon Sep 17 00:00:00 2001 From: smanna12 Date: Wed, 27 Mar 2024 09:12:51 -0500 Subject: [PATCH] [SYCL][FPGA] Refactor statments attributes to align with community the way we handle conflicting vs duplicate values (#13146) This patch updates ExprArgument<> to use same routine CheckForDuplicateAttrs() that was added on https://github.com/intel/llvm/pull/12243 to diagnose non-identical duplicates as a 'conflicting' loop attributes and suppresse duplicate errors in cases where the two match for FPGA SYCL attributes: [[intel::max_concurrency()]] and [[intel::initiation_interval()]] to align with clang community change (Ref: https://github.com/llvm/llvm-project/pull/70762). --------- Signed-off-by: Soumi Manna --- clang/include/clang/Basic/Attr.td | 4 +-- clang/lib/CodeGen/CGLoopInfo.cpp | 5 ++- clang/lib/CodeGen/CodeGenFunction.cpp | 4 +-- clang/lib/Sema/SemaDeclAttr.cpp | 21 +++++-------- clang/lib/Sema/SemaStmtAttr.cpp | 12 +++---- clang/lib/Sema/SemaTemplateInstantiate.cpp | 6 ++-- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 4 +-- clang/test/SemaSYCL/intel-fpga-loops.cpp | 31 +++++++++++++++---- 8 files changed, 49 insertions(+), 38 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 7d309d4ddd472..f4fc106c766a5 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -2549,7 +2549,7 @@ def SYCLIntelInitiationInterval : DeclOrStmtAttr { let Subjects = SubjectList<[ForStmt, CXXForRangeStmt, WhileStmt, DoStmt, Function], ErrorDiag, "'for', 'while', 'do' statements, and functions">; - let Args = [ExprArgument<"IntervalExpr">]; + let Args = [ExprArgument<"NExpr">]; let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost]; let Documentation = [SYCLIntelInitiationIntervalAttrDocs]; let SupportsNonconformingLambdaSyntax = 1; @@ -2560,7 +2560,7 @@ def SYCLIntelMaxConcurrency : DeclOrStmtAttr { let Subjects = SubjectList<[ForStmt, CXXForRangeStmt, WhileStmt, DoStmt, Function], ErrorDiag, "'for', 'while', 'do' statements, and functions">; - let Args = [ExprArgument<"NThreadsExpr">]; + let Args = [ExprArgument<"NExpr">]; let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost]; let Documentation = [SYCLIntelMaxConcurrencyAttrDocs]; let SupportsNonconformingLambdaSyntax = 1; diff --git a/clang/lib/CodeGen/CGLoopInfo.cpp b/clang/lib/CodeGen/CGLoopInfo.cpp index 7febf0653a7fa..247a4141f2a09 100644 --- a/clang/lib/CodeGen/CGLoopInfo.cpp +++ b/clang/lib/CodeGen/CGLoopInfo.cpp @@ -1056,15 +1056,14 @@ void LoopInfoStack::push(BasicBlock *Header, clang::ASTContext &Ctx, if (const auto *SYCLIntelII = dyn_cast(A)) { - const auto *CE = cast(SYCLIntelII->getIntervalExpr()); + const auto *CE = cast(SYCLIntelII->getNExpr()); llvm::APSInt ArgVal = CE->getResultAsAPSInt(); setSYCLIInterval(ArgVal.getSExtValue()); } if (const auto *SYCLIntelMaxConcurrency = dyn_cast(A)) { - const auto *CE = - cast(SYCLIntelMaxConcurrency->getNThreadsExpr()); + const auto *CE = cast(SYCLIntelMaxConcurrency->getNExpr()); llvm::APSInt ArgVal = CE->getResultAsAPSInt(); setSYCLMaxConcurrencyNThreads(ArgVal.getSExtValue()); } diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 49ca152f35d66..f6becc3b66bcf 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -803,7 +803,7 @@ void CodeGenFunction::EmitKernelMetadata(const FunctionDecl *FD, } if (const auto *A = FD->getAttr()) { - const auto *CE = cast(A->getNThreadsExpr()); + const auto *CE = cast(A->getNExpr()); llvm::APSInt ArgVal = CE->getResultAsAPSInt(); llvm::Metadata *AttrMDArgs[] = { llvm::ConstantAsMetadata::get(Builder.getInt32(ArgVal.getSExtValue()))}; @@ -817,7 +817,7 @@ void CodeGenFunction::EmitKernelMetadata(const FunctionDecl *FD, } if (const auto *A = FD->getAttr()) { - const auto *CE = cast(A->getIntervalExpr()); + const auto *CE = cast(A->getNExpr()); llvm::APSInt ArgVal = CE->getResultAsAPSInt(); llvm::Metadata *AttrMDArgs[] = { llvm::ConstantAsMetadata::get(Builder.getInt32(ArgVal.getSExtValue()))}; diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 49af476dc529e..a701f6844e4a9 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -4250,8 +4250,7 @@ void Sema::AddSYCLIntelInitiationIntervalAttr(Decl *D, // If the other attribute argument is instantiation dependent, we won't // have converted it to a constant expression yet and thus we test // whether this is a null pointer. - if (const auto *DeclExpr = - dyn_cast(DeclAttr->getIntervalExpr())) { + if (const auto *DeclExpr = dyn_cast(DeclAttr->getNExpr())) { if (ArgVal != DeclExpr->getResultAsAPSInt()) { Diag(CI.getLoc(), diag::warn_duplicate_attribute) << CI; Diag(DeclAttr->getLoc(), diag::note_previous_attribute); @@ -4273,9 +4272,8 @@ Sema::MergeSYCLIntelInitiationIntervalAttr( // already applied to the declaration. if (const auto *DeclAttr = D->getAttr()) { - if (const auto *DeclExpr = - dyn_cast(DeclAttr->getIntervalExpr())) { - if (const auto *MergeExpr = dyn_cast(A.getIntervalExpr())) { + if (const auto *DeclExpr = dyn_cast(DeclAttr->getNExpr())) { + if (const auto *MergeExpr = dyn_cast(A.getNExpr())) { if (DeclExpr->getResultAsAPSInt() != MergeExpr->getResultAsAPSInt()) { Diag(DeclAttr->getLoc(), diag::warn_duplicate_attribute) << &A; Diag(A.getLoc(), diag::note_previous_attribute); @@ -4287,7 +4285,7 @@ Sema::MergeSYCLIntelInitiationIntervalAttr( } return ::new (Context) - SYCLIntelInitiationIntervalAttr(Context, A, A.getIntervalExpr()); + SYCLIntelInitiationIntervalAttr(Context, A, A.getNExpr()); } static void handleSYCLIntelInitiationIntervalAttr(Sema &S, Decl *D, @@ -8418,9 +8416,8 @@ SYCLIntelMaxConcurrencyAttr *Sema::MergeSYCLIntelMaxConcurrencyAttr( // Check to see if there's a duplicate attribute with different values // already applied to the declaration. if (const auto *DeclAttr = D->getAttr()) { - if (const auto *DeclExpr = - dyn_cast(DeclAttr->getNThreadsExpr())) { - if (const auto *MergeExpr = dyn_cast(A.getNThreadsExpr())) { + if (const auto *DeclExpr = dyn_cast(DeclAttr->getNExpr())) { + if (const auto *MergeExpr = dyn_cast(A.getNExpr())) { if (DeclExpr->getResultAsAPSInt() != MergeExpr->getResultAsAPSInt()) { Diag(DeclAttr->getLoc(), diag::warn_duplicate_attribute) << &A; Diag(A.getLoc(), diag::note_previous_attribute); @@ -8431,8 +8428,7 @@ SYCLIntelMaxConcurrencyAttr *Sema::MergeSYCLIntelMaxConcurrencyAttr( } } - return ::new (Context) - SYCLIntelMaxConcurrencyAttr(Context, A, A.getNThreadsExpr()); + return ::new (Context) SYCLIntelMaxConcurrencyAttr(Context, A, A.getNExpr()); } void Sema::AddSYCLIntelMaxConcurrencyAttr(Decl *D, @@ -8458,8 +8454,7 @@ void Sema::AddSYCLIntelMaxConcurrencyAttr(Decl *D, // If the other attribute argument is instantiation dependent, we won't // have converted it to a constant expression yet and thus we test // whether this is a null pointer. - if (const auto *DeclExpr = - dyn_cast(DeclAttr->getNThreadsExpr())) { + if (const auto *DeclExpr = dyn_cast(DeclAttr->getNExpr())) { if (ArgVal != DeclExpr->getResultAsAPSInt()) { Diag(CI.getLoc(), diag::warn_duplicate_attribute) << CI; Diag(DeclAttr->getLoc(), diag::note_previous_attribute); diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp index e67a923ca239f..fdae227010efb 100644 --- a/clang/lib/Sema/SemaStmtAttr.cpp +++ b/clang/lib/Sema/SemaStmtAttr.cpp @@ -980,8 +980,8 @@ CheckForDuplicationSYCLLoopAttribute(Sema &S, // Diagnose non-identical duplicates as a 'conflicting' loop attributes // and suppress duplicate errors in cases where the two match for // FPGA attributes: 'SYCLIntelMaxInterleavingAttr', -// 'SYCLIntelSpeculatedIterationsAttr', and -// 'SYCLIntelMaxReinvocationDelayAttr'. +// 'SYCLIntelSpeculatedIterationsAttr', 'SYCLIntelMaxReinvocationDelayAttr', +// 'SYCLIntelInitiationIntervalAttr', and 'SYCLIntelMaxConcurrencyAttr' template static void CheckForDuplicateAttrs(Sema &S, ArrayRef Attrs) { auto FindFunc = [](const Attr *A) { return isa(A); }; @@ -1023,10 +1023,8 @@ static void CheckForDuplicateAttrs(Sema &S, ArrayRef Attrs) { static void CheckForIncompatibleSYCLLoopAttributes( Sema &S, const SmallVectorImpl &Attrs) { - CheckForDuplicationSYCLLoopAttribute( - S, Attrs); - CheckForDuplicationSYCLLoopAttribute(S, - Attrs); + CheckForDuplicateAttrs(S, Attrs); + CheckForDuplicateAttrs(S, Attrs); CheckForDuplicationSYCLLoopAttribute(S, Attrs); CheckForDuplicationSYCLLoopAttribute( S, Attrs); @@ -1224,6 +1222,8 @@ bool Sema::CheckRebuiltAttributedStmtAttributes(ArrayRef Attrs) { CheckForDuplicateAttrs(*this, Attrs); CheckForDuplicateAttrs(*this, Attrs); CheckForDuplicateAttrs(*this, Attrs); + CheckForDuplicateAttrs(*this, Attrs); + CheckForDuplicateAttrs(*this, Attrs); return false; } diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index de39df731f6bc..25e9d50bd28dc 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -2067,8 +2067,7 @@ TemplateInstantiator::TransformSYCLIntelIVDepAttr( const SYCLIntelInitiationIntervalAttr * TemplateInstantiator::TransformSYCLIntelInitiationIntervalAttr( const SYCLIntelInitiationIntervalAttr *II) { - Expr *TransformedExpr = - getDerived().TransformExpr(II->getIntervalExpr()).get(); + Expr *TransformedExpr = getDerived().TransformExpr(II->getNExpr()).get(); return getSema().BuildSYCLIntelInitiationIntervalAttr(*II, TransformedExpr); } @@ -2076,8 +2075,7 @@ TemplateInstantiator::TransformSYCLIntelInitiationIntervalAttr( const SYCLIntelMaxConcurrencyAttr * TemplateInstantiator::TransformSYCLIntelMaxConcurrencyAttr( const SYCLIntelMaxConcurrencyAttr *MC) { - Expr *TransformedExpr = - getDerived().TransformExpr(MC->getNThreadsExpr()).get(); + Expr *TransformedExpr = getDerived().TransformExpr(MC->getNExpr()).get(); return getSema().BuildSYCLIntelMaxConcurrencyAttr(*MC, TransformedExpr); } diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 5d742e3292873..b0e8f836d2b5d 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -802,7 +802,7 @@ static void instantiateSYCLIntelMaxConcurrencyAttr( const SYCLIntelMaxConcurrencyAttr *A, Decl *New) { EnterExpressionEvaluationContext Unevaluated( S, Sema::ExpressionEvaluationContext::ConstantEvaluated); - ExprResult Result = S.SubstExpr(A->getNThreadsExpr(), TemplateArgs); + ExprResult Result = S.SubstExpr(A->getNExpr(), TemplateArgs); if (!Result.isInvalid()) S.AddSYCLIntelMaxConcurrencyAttr(New, *A, Result.getAs()); } @@ -832,7 +832,7 @@ static void instantiateSYCLIntelInitiationIntervalAttr( const SYCLIntelInitiationIntervalAttr *A, Decl *New) { EnterExpressionEvaluationContext Unevaluated( S, Sema::ExpressionEvaluationContext::ConstantEvaluated); - ExprResult Result = S.SubstExpr(A->getIntervalExpr(), TemplateArgs); + ExprResult Result = S.SubstExpr(A->getNExpr(), TemplateArgs); if (!Result.isInvalid()) S.AddSYCLIntelInitiationIntervalAttr(New, *A, Result.getAs()); } diff --git a/clang/test/SemaSYCL/intel-fpga-loops.cpp b/clang/test/SemaSYCL/intel-fpga-loops.cpp index a26f9a0244f07..2b1a6d5043c1a 100644 --- a/clang/test/SemaSYCL/intel-fpga-loops.cpp +++ b/clang/test/SemaSYCL/intel-fpga-loops.cpp @@ -264,18 +264,26 @@ void zoo() { [[intel::ivdep(4)]] for (int i = 0; i != 10; ++i) a[i] = 0; [[intel::max_concurrency(2)]] - // expected-error@+1 {{duplicate Intel FPGA loop attribute 'max_concurrency'}} [[intel::max_concurrency(2)]] for (int i = 0; i != 10; ++i) a[i] = 0; [[intel::initiation_interval(2)]] - // expected-error@+1 {{duplicate Intel FPGA loop attribute 'initiation_interval'}} [[intel::initiation_interval(2)]] for (int i = 0; i != 10; ++i) a[i] = 0; [[intel::initiation_interval(2)]] - // expected-error@+2 {{duplicate Intel FPGA loop attribute 'initiation_interval'}} [[intel::max_concurrency(2)]] [[intel::initiation_interval(2)]] for (int i = 0; i != 10; ++i) a[i] = 0; + + [[intel::max_concurrency(10)]] // expected-note {{previous attribute is here}} + [[intel::max_concurrency(10)]] // OK + // expected-error@+1 {{conflicting loop attribute 'max_concurrency'}} + [[intel::max_concurrency(20)]] for (int i = 0; i != 10; ++i) + a[i] = 0; + [[intel::initiation_interval(10)]] // expected-note {{previous attribute is here}} + [[intel::initiation_interval(10)]] // OK + // expected-error@+1 {{conflicting loop attribute 'initiation_interval'}} + [[intel::initiation_interval(20)]] for (int i = 0; i != 10; ++i) + a[i] = 0; [[intel::disable_loop_pipelining]] // expected-error@+1 {{duplicate Intel FPGA loop attribute 'disable_loop_pipelining'}} [[intel::disable_loop_pipelining]] for (int i = 0; i != 10; ++i) @@ -475,8 +483,12 @@ void ii_dependent() { [[intel::initiation_interval(C)]] for (int i = 0; i != 10; ++i) a[i] = 0; - // expected-error@+2 {{duplicate Intel FPGA loop attribute 'initiation_interval'}} [[intel::initiation_interval(A)]] + [[intel::initiation_interval(A)]] for (int i = 0; i != 10; ++i) + a[i] = 0; + + // expected-error@+2 {{conflicting loop attribute 'initiation_interval'}} + [[intel::initiation_interval(A)]] // expected-note {{previous attribute is here}} [[intel::initiation_interval(B)]] for (int i = 0; i != 10; ++i) a[i] = 0; } @@ -488,11 +500,18 @@ void max_concurrency_dependent() { [[intel::max_concurrency(C)]] for (int i = 0; i != 10; ++i) a[i] = 0; - // expected-error@+2 {{duplicate Intel FPGA loop attribute 'max_concurrency'}} - [[intel::max_concurrency(A)]] + // expected-error@+2 {{conflicting loop attribute 'max_concurrency'}} + [[intel::max_concurrency(A)]] // expected-note {{previous attribute is here}} [[intel::max_concurrency(B)]] for (int i = 0; i != 10; ++i) a[i] = 0; + + [[intel::max_concurrency(A)]] + [[intel::max_concurrency(A)]] for (int i = 0; i != 10; ++i) + a[i] = 0; + + + [[intel::max_concurrency(D)]] // max_concurrency attribute accepts value 0. [[intel::max_concurrency(D)]] for (int i = 0; i != 10; ++i) a[i] = 0;