Skip to content

Commit

Permalink
[SYCL][FPGA] Refactor statments attributes to align with community th…
Browse files Browse the repository at this point in the history
…e way we handle conflicting vs duplicate values (#13146)

This patch updates ExprArgument<> to use same routine
CheckForDuplicateAttrs() that was added on
#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:
llvm/llvm-project#70762).

---------

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
  • Loading branch information
smanna12 committed Mar 27, 2024
1 parent 451caf1 commit b0ecb56
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 38 deletions.
4 changes: 2 additions & 2 deletions clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions clang/lib/CodeGen/CGLoopInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1056,15 +1056,14 @@ void LoopInfoStack::push(BasicBlock *Header, clang::ASTContext &Ctx,

if (const auto *SYCLIntelII =
dyn_cast<SYCLIntelInitiationIntervalAttr>(A)) {
const auto *CE = cast<ConstantExpr>(SYCLIntelII->getIntervalExpr());
const auto *CE = cast<ConstantExpr>(SYCLIntelII->getNExpr());
llvm::APSInt ArgVal = CE->getResultAsAPSInt();
setSYCLIInterval(ArgVal.getSExtValue());
}

if (const auto *SYCLIntelMaxConcurrency =
dyn_cast<SYCLIntelMaxConcurrencyAttr>(A)) {
const auto *CE =
cast<ConstantExpr>(SYCLIntelMaxConcurrency->getNThreadsExpr());
const auto *CE = cast<ConstantExpr>(SYCLIntelMaxConcurrency->getNExpr());
llvm::APSInt ArgVal = CE->getResultAsAPSInt();
setSYCLMaxConcurrencyNThreads(ArgVal.getSExtValue());
}
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/CodeGen/CodeGenFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,7 @@ void CodeGenFunction::EmitKernelMetadata(const FunctionDecl *FD,
}

if (const auto *A = FD->getAttr<SYCLIntelMaxConcurrencyAttr>()) {
const auto *CE = cast<ConstantExpr>(A->getNThreadsExpr());
const auto *CE = cast<ConstantExpr>(A->getNExpr());
llvm::APSInt ArgVal = CE->getResultAsAPSInt();
llvm::Metadata *AttrMDArgs[] = {
llvm::ConstantAsMetadata::get(Builder.getInt32(ArgVal.getSExtValue()))};
Expand All @@ -817,7 +817,7 @@ void CodeGenFunction::EmitKernelMetadata(const FunctionDecl *FD,
}

if (const auto *A = FD->getAttr<SYCLIntelInitiationIntervalAttr>()) {
const auto *CE = cast<ConstantExpr>(A->getIntervalExpr());
const auto *CE = cast<ConstantExpr>(A->getNExpr());
llvm::APSInt ArgVal = CE->getResultAsAPSInt();
llvm::Metadata *AttrMDArgs[] = {
llvm::ConstantAsMetadata::get(Builder.getInt32(ArgVal.getSExtValue()))};
Expand Down
21 changes: 8 additions & 13 deletions clang/lib/Sema/SemaDeclAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<ConstantExpr>(DeclAttr->getIntervalExpr())) {
if (const auto *DeclExpr = dyn_cast<ConstantExpr>(DeclAttr->getNExpr())) {
if (ArgVal != DeclExpr->getResultAsAPSInt()) {
Diag(CI.getLoc(), diag::warn_duplicate_attribute) << CI;
Diag(DeclAttr->getLoc(), diag::note_previous_attribute);
Expand All @@ -4273,9 +4272,8 @@ Sema::MergeSYCLIntelInitiationIntervalAttr(
// already applied to the declaration.
if (const auto *DeclAttr =
D->getAttr<SYCLIntelInitiationIntervalAttr>()) {
if (const auto *DeclExpr =
dyn_cast<ConstantExpr>(DeclAttr->getIntervalExpr())) {
if (const auto *MergeExpr = dyn_cast<ConstantExpr>(A.getIntervalExpr())) {
if (const auto *DeclExpr = dyn_cast<ConstantExpr>(DeclAttr->getNExpr())) {
if (const auto *MergeExpr = dyn_cast<ConstantExpr>(A.getNExpr())) {
if (DeclExpr->getResultAsAPSInt() != MergeExpr->getResultAsAPSInt()) {
Diag(DeclAttr->getLoc(), diag::warn_duplicate_attribute) << &A;
Diag(A.getLoc(), diag::note_previous_attribute);
Expand All @@ -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,
Expand Down Expand Up @@ -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<SYCLIntelMaxConcurrencyAttr>()) {
if (const auto *DeclExpr =
dyn_cast<ConstantExpr>(DeclAttr->getNThreadsExpr())) {
if (const auto *MergeExpr = dyn_cast<ConstantExpr>(A.getNThreadsExpr())) {
if (const auto *DeclExpr = dyn_cast<ConstantExpr>(DeclAttr->getNExpr())) {
if (const auto *MergeExpr = dyn_cast<ConstantExpr>(A.getNExpr())) {
if (DeclExpr->getResultAsAPSInt() != MergeExpr->getResultAsAPSInt()) {
Diag(DeclAttr->getLoc(), diag::warn_duplicate_attribute) << &A;
Diag(A.getLoc(), diag::note_previous_attribute);
Expand All @@ -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,
Expand All @@ -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<ConstantExpr>(DeclAttr->getNThreadsExpr())) {
if (const auto *DeclExpr = dyn_cast<ConstantExpr>(DeclAttr->getNExpr())) {
if (ArgVal != DeclExpr->getResultAsAPSInt()) {
Diag(CI.getLoc(), diag::warn_duplicate_attribute) << CI;
Diag(DeclAttr->getLoc(), diag::note_previous_attribute);
Expand Down
12 changes: 6 additions & 6 deletions clang/lib/Sema/SemaStmtAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename LoopAttrT>
static void CheckForDuplicateAttrs(Sema &S, ArrayRef<const Attr *> Attrs) {
auto FindFunc = [](const Attr *A) { return isa<const LoopAttrT>(A); };
Expand Down Expand Up @@ -1023,10 +1023,8 @@ static void CheckForDuplicateAttrs(Sema &S, ArrayRef<const Attr *> Attrs) {

static void CheckForIncompatibleSYCLLoopAttributes(
Sema &S, const SmallVectorImpl<const Attr *> &Attrs) {
CheckForDuplicationSYCLLoopAttribute<SYCLIntelInitiationIntervalAttr>(
S, Attrs);
CheckForDuplicationSYCLLoopAttribute<SYCLIntelMaxConcurrencyAttr>(S,
Attrs);
CheckForDuplicateAttrs<SYCLIntelInitiationIntervalAttr>(S, Attrs);
CheckForDuplicateAttrs<SYCLIntelMaxConcurrencyAttr>(S, Attrs);
CheckForDuplicationSYCLLoopAttribute<SYCLIntelLoopCoalesceAttr>(S, Attrs);
CheckForDuplicationSYCLLoopAttribute<SYCLIntelDisableLoopPipeliningAttr>(
S, Attrs);
Expand Down Expand Up @@ -1224,6 +1222,8 @@ bool Sema::CheckRebuiltAttributedStmtAttributes(ArrayRef<const Attr *> Attrs) {
CheckForDuplicateAttrs<SYCLIntelSpeculatedIterationsAttr>(*this, Attrs);
CheckForDuplicateAttrs<SYCLIntelMaxInterleavingAttr>(*this, Attrs);
CheckForDuplicateAttrs<SYCLIntelMaxReinvocationDelayAttr>(*this, Attrs);
CheckForDuplicateAttrs<SYCLIntelInitiationIntervalAttr>(*this, Attrs);
CheckForDuplicateAttrs<SYCLIntelMaxConcurrencyAttr>(*this, Attrs);
return false;
}

Expand Down
6 changes: 2 additions & 4 deletions clang/lib/Sema/SemaTemplateInstantiate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2067,17 +2067,15 @@ 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);
}

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);
}

Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Expr>());
}
Expand Down Expand Up @@ -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<Expr>());
}
Expand Down
31 changes: 25 additions & 6 deletions clang/test/SemaSYCL/intel-fpga-loops.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down

0 comments on commit b0ecb56

Please sign in to comment.