Skip to content

Commit 628bc0c

Browse files
smanna12smanna12
authored
[SYCL] Refactor of FPGA loop fusion function attributes (#3298)
This patch 1. refactors two function attributes using #3224 : [[intel::loop_fuse()]] and [[intel::loop_fuse_independent()]] 2. store expression as ConstantExpr in Semantic Attributes 3. handles template instantiations properly for duplicate attributes on a given declaration. 4. adds test 5. updates codegen codes Signed-off-by: Soumi Manna <soumi.manna@intel.com>
1 parent 64f326f commit 628bc0c

File tree

7 files changed

+167
-98
lines changed

7 files changed

+167
-98
lines changed

‎clang/include/clang/Sema/Sema.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3471,8 +3471,6 @@ class Sema final {
34713471
EnforceTCBLeafAttr *mergeEnforceTCBLeafAttr(Decl *D,
34723472
const EnforceTCBLeafAttr &AL);
34733473

3474-
SYCLIntelLoopFuseAttr *
3475-
mergeSYCLIntelLoopFuseAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E);
34763474
void mergeDeclAttributes(NamedDecl *New, Decl *Old,
34773475
AvailabilityMergeKind AMK = AMK_Redeclaration);
34783476
void MergeTypedefNameDecl(Scope *S, TypedefNameDecl *New,
@@ -10232,6 +10230,10 @@ class Sema final {
1023210230
Expr *E);
1023310231
SYCLIntelNoGlobalWorkOffsetAttr *MergeSYCLIntelNoGlobalWorkOffsetAttr(
1023410232
Decl *D, const SYCLIntelNoGlobalWorkOffsetAttr &A);
10233+
void AddSYCLIntelLoopFuseAttr(Decl *D, const AttributeCommonInfo &CI,
10234+
Expr *E);
10235+
SYCLIntelLoopFuseAttr *
10236+
MergeSYCLIntelLoopFuseAttr(Decl *D, const SYCLIntelLoopFuseAttr &A);
1023510237

1023610238
/// AddAlignedAttr - Adds an aligned attribute to a particular declaration.
1023710239
void AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E,
@@ -10286,8 +10288,6 @@ class Sema final {
1028610288
/// addSYCLIntelPipeIOAttr - Adds a pipe I/O attribute to a particular
1028710289
/// declaration.
1028810290
void addSYCLIntelPipeIOAttr(Decl *D, const AttributeCommonInfo &CI, Expr *ID);
10289-
void addSYCLIntelLoopFuseAttr(Decl *D, const AttributeCommonInfo &CI,
10290-
Expr *E);
1029110291

1029210292
bool checkNSReturnsRetainedReturnType(SourceLocation loc, QualType type);
1029310293
bool checkAllowedSYCLInitializer(VarDecl *VD,

‎clang/lib/CodeGen/CodeGenFunction.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -983,10 +983,11 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
983983

984984
if (getLangOpts().SYCLIsDevice && D) {
985985
if (const auto *A = D->getAttr<SYCLIntelLoopFuseAttr>()) {
986-
Expr *E = A->getValue();
986+
const auto *CE = cast<ConstantExpr>(A->getValue());
987+
Optional<llvm::APSInt> ArgVal = CE->getResultAsAPSInt();
987988
llvm::Metadata *AttrMDArgs[] = {
988-
llvm::ConstantAsMetadata::get(Builder.getInt32(
989-
E->getIntegerConstantExpr(D->getASTContext())->getZExtValue())),
989+
llvm::ConstantAsMetadata::get(
990+
Builder.getInt32(ArgVal->getZExtValue())),
990991
llvm::ConstantAsMetadata::get(
991992
A->isIndependent() ? Builder.getInt32(1) : Builder.getInt32(0))};
992993
Fn->setMetadata("loop_fuse",

‎clang/lib/Sema/SemaDecl.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2612,8 +2612,8 @@ static bool mergeDeclAttribute(Sema &S, NamedDecl *D,
26122612
NewAttr = S.mergeImportModuleAttr(D, *IMA);
26132613
else if (const auto *INA = dyn_cast<WebAssemblyImportNameAttr>(Attr))
26142614
NewAttr = S.mergeImportNameAttr(D, *INA);
2615-
else if (const auto *LFA = dyn_cast<SYCLIntelLoopFuseAttr>(Attr))
2616-
NewAttr = S.mergeSYCLIntelLoopFuseAttr(D, *LFA, LFA->getValue());
2615+
else if (const auto *A = dyn_cast<SYCLIntelLoopFuseAttr>(Attr))
2616+
NewAttr = S.MergeSYCLIntelLoopFuseAttr(D, *A);
26172617
else if (const auto *TCBA = dyn_cast<EnforceTCBAttr>(Attr))
26182618
NewAttr = S.mergeEnforceTCBAttr(D, *TCBA);
26192619
else if (const auto *TCBLA = dyn_cast<EnforceTCBLeafAttr>(Attr))

‎clang/lib/Sema/SemaDeclAttr.cpp

Lines changed: 74 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -3449,87 +3449,93 @@ static void handleMaxGlobalWorkDimAttr(Sema &S, Decl *D, const ParsedAttr &A) {
34493449
S.addIntelSingleArgAttr<SYCLIntelMaxGlobalWorkDimAttr>(D, A, E);
34503450
}
34513451

3452-
SYCLIntelLoopFuseAttr *
3453-
Sema::mergeSYCLIntelLoopFuseAttr(Decl *D, const AttributeCommonInfo &CI,
3454-
Expr *E) {
3452+
// Handles [[intel::loop_fuse]] and [[intel::loop_fuse_independent]].
3453+
void Sema::AddSYCLIntelLoopFuseAttr(Decl *D, const AttributeCommonInfo &CI,
3454+
Expr *E) {
3455+
if (!E->isValueDependent()) {
3456+
// Validate that we have an integer constant expression and then store the
3457+
// converted constant expression into the semantic attribute so that we
3458+
// don't have to evaluate it again later.
3459+
llvm::APSInt ArgVal;
3460+
ExprResult Res = VerifyIntegerConstantExpression(E, &ArgVal);
3461+
if (Res.isInvalid())
3462+
return;
3463+
E = Res.get();
34553464

3456-
if (const auto ExistingAttr = D->getAttr<SYCLIntelLoopFuseAttr>()) {
3465+
// This attribute requires a non-negative value.
3466+
if (ArgVal < 0) {
3467+
Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
3468+
<< CI << /*non-negative*/ 1;
3469+
return;
3470+
}
3471+
// Check to see if there's a duplicate attribute with different values
3472+
// already applied to the declaration.
3473+
if (const auto *DeclAttr = D->getAttr<SYCLIntelLoopFuseAttr>()) {
3474+
// If the other attribute argument is instantiation dependent, we won't
3475+
// have converted it to a constant expression yet and thus we test
3476+
// whether this is a null pointer.
3477+
const auto *DeclExpr = dyn_cast<ConstantExpr>(DeclAttr->getValue());
3478+
if (DeclExpr && ArgVal != DeclExpr->getResultAsAPSInt()) {
3479+
Diag(CI.getLoc(), diag::warn_duplicate_attribute) << CI;
3480+
Diag(DeclAttr->getLoc(), diag::note_previous_attribute);
3481+
return;
3482+
}
3483+
// [[intel::loop_fuse]] and [[intel::loop_fuse_independent]] are
3484+
// incompatible.
3485+
// FIXME: If additional spellings are provided for this attribute,
3486+
// this code will do the wrong thing.
3487+
if (DeclAttr->getAttributeSpellingListIndex() !=
3488+
CI.getAttributeSpellingListIndex()) {
3489+
Diag(CI.getLoc(), diag::err_attributes_are_not_compatible)
3490+
<< CI << DeclAttr;
3491+
Diag(DeclAttr->getLocation(), diag::note_conflicting_attribute);
3492+
return;
3493+
}
3494+
}
3495+
}
3496+
3497+
D->addAttr(::new (Context) SYCLIntelLoopFuseAttr(Context, CI, E));
3498+
}
3499+
3500+
SYCLIntelLoopFuseAttr *
3501+
Sema::MergeSYCLIntelLoopFuseAttr(Decl *D, const SYCLIntelLoopFuseAttr &A) {
3502+
// Check to see if there's a duplicate attribute with different values
3503+
// already applied to the declaration.
3504+
if (const auto *DeclAttr = D->getAttr<SYCLIntelLoopFuseAttr>()) {
3505+
const auto *DeclExpr = dyn_cast<ConstantExpr>(DeclAttr->getValue());
3506+
const auto *MergeExpr = dyn_cast<ConstantExpr>(A.getValue());
3507+
if (DeclExpr && MergeExpr &&
3508+
DeclExpr->getResultAsAPSInt() != MergeExpr->getResultAsAPSInt()) {
3509+
Diag(DeclAttr->getLoc(), diag::warn_duplicate_attribute) << &A;
3510+
Diag(A.getLoc(), diag::note_previous_attribute);
3511+
return nullptr;
3512+
}
34573513
// [[intel::loop_fuse]] and [[intel::loop_fuse_independent]] are
34583514
// incompatible.
34593515
// FIXME: If additional spellings are provided for this attribute,
34603516
// this code will do the wrong thing.
3461-
if (ExistingAttr->getAttributeSpellingListIndex() !=
3462-
CI.getAttributeSpellingListIndex()) {
3463-
Diag(CI.getLoc(), diag::err_attributes_are_not_compatible)
3464-
<< CI << ExistingAttr;
3465-
Diag(ExistingAttr->getLocation(), diag::note_conflicting_attribute);
3517+
if (DeclAttr->getAttributeSpellingListIndex() !=
3518+
A.getAttributeSpellingListIndex()) {
3519+
Diag(A.getLoc(), diag::err_attributes_are_not_compatible)
3520+
<< &A << DeclAttr;
3521+
Diag(DeclAttr->getLoc(), diag::note_conflicting_attribute);
34663522
return nullptr;
34673523
}
3468-
3469-
if (!E->isValueDependent()) {
3470-
Optional<llvm::APSInt> ArgVal = E->getIntegerConstantExpr(Context);
3471-
Optional<llvm::APSInt> ExistingArgVal =
3472-
ExistingAttr->getValue()->getIntegerConstantExpr(Context);
3473-
3474-
assert(ArgVal && ExistingArgVal &&
3475-
"Argument should be an integer constant expression");
3476-
// Compare attribute argument value and warn if there is a mismatch.
3477-
if (ArgVal->getExtValue() != ExistingArgVal->getExtValue())
3478-
Diag(ExistingAttr->getLoc(), diag::warn_duplicate_attribute)
3479-
<< ExistingAttr;
3480-
}
3481-
3482-
// If there is no mismatch, silently ignore duplicate attribute.
3483-
return nullptr;
3484-
}
3485-
return ::new (Context) SYCLIntelLoopFuseAttr(Context, CI, E);
3486-
}
3487-
3488-
static bool checkSYCLIntelLoopFuseArgument(Sema &S,
3489-
const AttributeCommonInfo &CI,
3490-
Expr *E) {
3491-
// Dependent expressions are checked when instantiated.
3492-
if (E->isValueDependent())
3493-
return false;
3494-
3495-
Optional<llvm::APSInt> ArgVal = E->getIntegerConstantExpr(S.Context);
3496-
if (!ArgVal) {
3497-
S.Diag(E->getExprLoc(), diag::err_attribute_argument_type)
3498-
<< CI << AANT_ArgumentIntegerConstant << E->getSourceRange();
3499-
return true;
3500-
}
3501-
3502-
if (!ArgVal->isNonNegative()) {
3503-
S.Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
3504-
<< CI << /*non-negative*/ 1;
3505-
return true;
35063524
}
35073525

3508-
return false;
3526+
return ::new (Context) SYCLIntelLoopFuseAttr(Context, A, A.getValue());
35093527
}
35103528

3511-
void Sema::addSYCLIntelLoopFuseAttr(Decl *D, const AttributeCommonInfo &CI,
3512-
Expr *E) {
3513-
assert(E && "argument has unexpected null value");
3514-
3515-
if (checkSYCLIntelLoopFuseArgument(*this, CI, E))
3516-
return;
3517-
3518-
SYCLIntelLoopFuseAttr *NewAttr = mergeSYCLIntelLoopFuseAttr(D, CI, E);
3519-
3520-
if (NewAttr)
3521-
D->addAttr(NewAttr);
3522-
}
3529+
static void handleSYCLIntelLoopFuseAttr(Sema &S, Decl *D, const ParsedAttr &A) {
3530+
S.CheckDeprecatedSYCLAttributeSpelling(A);
35233531

3524-
// Handles [[intel::loop_fuse]] and [[intel::loop_fuse_independent]].
3525-
static void handleLoopFuseAttr(Sema &S, Decl *D, const ParsedAttr &Attr) {
3526-
// Default argument value is set to 1.
3527-
Expr *E = Attr.isArgExpr(0)
3528-
? Attr.getArgAsExpr(0)
3532+
// If no attribute argument is specified, set to default value '1'.
3533+
Expr *E = A.isArgExpr(0)
3534+
? A.getArgAsExpr(0)
35293535
: IntegerLiteral::Create(S.Context, llvm::APInt(32, 1),
3530-
S.Context.IntTy, Attr.getLoc());
3536+
S.Context.IntTy, A.getLoc());
35313537

3532-
S.addSYCLIntelLoopFuseAttr(D, Attr, E);
3538+
S.AddSYCLIntelLoopFuseAttr(D, A, E);
35333539
}
35343540

35353541
static void handleVecTypeHint(Sema &S, Decl *D, const ParsedAttr &AL) {
@@ -9132,7 +9138,7 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
91329138
handleUseStallEnableClustersAttr(S, D, AL);
91339139
break;
91349140
case ParsedAttr::AT_SYCLIntelLoopFuse:
9135-
handleLoopFuseAttr(S, D, AL);
9141+
handleSYCLIntelLoopFuseAttr(S, D, AL);
91369142
break;
91379143
case ParsedAttr::AT_VecTypeHint:
91389144
handleVecTypeHint(S, D, AL);

‎clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,7 @@ static void instantiateSYCLIntelLoopFuseAttr(
640640
S, Sema::ExpressionEvaluationContext::ConstantEvaluated);
641641
ExprResult Result = S.SubstExpr(Attr->getValue(), TemplateArgs);
642642
if (!Result.isInvalid())
643-
S.addSYCLIntelLoopFuseAttr(New, *Attr, Result.getAs<Expr>());
643+
S.AddSYCLIntelLoopFuseAttr(New, *Attr, Result.getAs<Expr>());
644644
}
645645

646646
static void instantiateIntelReqdSubGroupSize(

‎clang/test/SemaSYCL/loop_fusion.cpp

Lines changed: 65 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,63 +1,108 @@
11
// RUN: %clang_cc1 -fsycl -fsycl-is-device -verify %s
22

3+
// Tests for incorrect argument values for Intel FPGA loop fusion function attributes
34
[[intel::loop_fuse(5)]] int a; // expected-error{{'loop_fuse' attribute only applies to functions}}
45

5-
[[intel::loop_fuse("foo")]] void func() {} // expected-error{{'loop_fuse' attribute requires an integer constant}}
6+
[[intel::loop_fuse("foo")]] void func() {} // expected-error{{integral constant expression must have integral or unscoped enumeration type, not 'const char [4]'}}
67

78
[[intel::loop_fuse(1048577)]] void func1() {} // OK
89
[[intel::loop_fuse_independent(-1)]] void func2() {} // expected-error{{'loop_fuse_independent' attribute requires a non-negative integral compile time constant expression}}
910

1011
[[intel::loop_fuse(0, 1)]] void func3() {} // expected-error{{'loop_fuse' attribute takes no more than 1 argument}}
1112
[[intel::loop_fuse_independent(2, 3)]] void func4() {} // expected-error{{'loop_fuse_independent' attribute takes no more than 1 argument}}
1213

14+
// Tests for Intel FPGA loop attributes duplication
1315
// No diagnostic is thrown since arguments match. Duplicate attribute is silently ignored.
1416
[[intel::loop_fuse]] [[intel::loop_fuse]] void func5() {}
1517
[[intel::loop_fuse_independent(10)]] [[intel::loop_fuse_independent(10)]] void func6() {}
1618

17-
[[intel::loop_fuse]] [[intel::loop_fuse(10)]] void func7() {} // expected-warning {{attribute 'loop_fuse' is already applied with different arguments}}
18-
[[intel::loop_fuse_independent(5)]] [[intel::loop_fuse_independent(10)]] void func8() {} // expected-warning {{attribute 'loop_fuse_independent' is already applied with different arguments}}
19+
// Tests for merging of different argument values for Intel FPGA loop fusion function attributes
20+
[[intel::loop_fuse]] // expected-note {{previous attribute is here}}
21+
[[intel::loop_fuse(10)]] void func7() {} // expected-warning {{attribute 'loop_fuse' is already applied with different arguments}}
22+
23+
[[intel::loop_fuse_independent(5)]] // expected-note {{previous attribute is here}}
24+
[[intel::loop_fuse_independent(10)]] void func8() {} // expected-warning {{attribute 'loop_fuse_independent' is already applied with different arguments}}
1925

2026
[[intel::loop_fuse]] void func9();
21-
[[intel::loop_fuse]] void func9();
27+
[[intel::loop_fuse]] void func9(); // OK
2228

2329
[[intel::loop_fuse_independent(10)]] void func10();
24-
[[intel::loop_fuse_independent(10)]] void func10();
30+
[[intel::loop_fuse_independent(10)]] void func10(); // OK
2531

26-
[[intel::loop_fuse(1)]] void func11();
32+
[[intel::loop_fuse(1)]] void func11(); // expected-note {{previous attribute is here}}
2733
[[intel::loop_fuse(3)]] void func11(); // expected-warning {{attribute 'loop_fuse' is already applied with different arguments}}
2834

29-
[[intel::loop_fuse_independent(1)]] void func12();
35+
[[intel::loop_fuse_independent(1)]] void func12(); // expected-note {{previous attribute is here}}
3036
[[intel::loop_fuse_independent(3)]] void func12(); // expected-warning {{attribute 'loop_fuse_independent' is already applied with different arguments}}
3137

38+
[[intel::loop_fuse_independent]]
39+
[[intel::loop_fuse_independent]] void func13() {} // OK
40+
41+
[[intel::loop_fuse_independent]] // expected-note {{previous attribute is here}}
42+
[[intel::loop_fuse_independent(10)]] void func14() {} // expected-warning {{attribute 'loop_fuse_independent' is already applied with different arguments}}
43+
44+
// Tests for Intel FPGA loop fusion function attributes compatibility
3245
// expected-error@+2 {{'loop_fuse_independent' and 'loop_fuse' attributes are not compatible}}
3346
// expected-note@+1 {{conflicting attribute is here}}
34-
[[intel::loop_fuse]] [[intel::loop_fuse_independent]] void func13();
47+
[[intel::loop_fuse]] [[intel::loop_fuse_independent]] void func15();
3548

3649
// expected-error@+2 {{'loop_fuse' and 'loop_fuse_independent' attributes are not compatible}}
3750
// expected-note@+1 {{conflicting attribute is here}}
38-
[[intel::loop_fuse_independent]] [[intel::loop_fuse]] void func14();
51+
[[intel::loop_fuse_independent]] [[intel::loop_fuse]] void func16();
3952

4053
// expected-error@+2 {{'loop_fuse' and 'loop_fuse_independent' attributes are not compatible}}
4154
// expected-note@+2 {{conflicting attribute is here}}
42-
[[intel::loop_fuse]] void func15();
43-
[[intel::loop_fuse_independent]] void func15();
55+
[[intel::loop_fuse]] void func17();
56+
[[intel::loop_fuse_independent]] void func17();
4457

4558
// expected-error@+2 {{'loop_fuse_independent' and 'loop_fuse' attributes are not compatible}}
4659
// expected-note@+2 {{conflicting attribute is here}}
47-
[[intel::loop_fuse_independent]] void func16();
48-
[[intel::loop_fuse]] void func16();
60+
[[intel::loop_fuse_independent]] void func18();
61+
[[intel::loop_fuse]] void func18();
4962

63+
// Tests that check template parameter support for Intel FPGA loop fusion function attributes
5064
template <int N>
51-
[[intel::loop_fuse(N)]] void func17(); // expected-error{{'loop_fuse' attribute requires a non-negative integral compile time constant expression}}
65+
[[intel::loop_fuse(N)]] void func19(); // expected-error{{'loop_fuse' attribute requires a non-negative integral compile time constant expression}}
5266

53-
template <typename Ty>
54-
[[intel::loop_fuse(Ty{})]] void func18() {} // expected-error{{'loop_fuse' attribute requires an integer constant}}
67+
template <int size>
68+
[[intel::loop_fuse(12)]] void func20(); // expected-note {{previous attribute is here}}
69+
template <int size>
70+
[[intel::loop_fuse(size)]] void func20() {} // expected-warning {{attribute 'loop_fuse' is already applied with different arguments}}
71+
72+
template <int size>
73+
[[intel::loop_fuse_independent(5)]] void func21(); // expected-note {{previous attribute is here}}
74+
template <int size>
75+
[[intel::loop_fuse_independent(size)]] void func21() {} // expected-warning {{attribute 'loop_fuse_independent' is already applied with different arguments}}
5576

5677
void checkTemplates() {
57-
func17<-1>(); // expected-note{{in instantiation of}}
58-
func17<0>(); // OK
59-
func18<float>(); // expected-note{{in instantiation of}}
78+
func19<-1>(); // expected-note{{in instantiation of}}
79+
func19<0>(); // OK
80+
func20<20>(); // expected-note {{in instantiation of function template specialization 'func20<20>' requested here}}
81+
func21<14>(); // expected-note {{in instantiation of function template specialization 'func21<14>' requested here}}
6082
}
6183

84+
// Test that checks expression is not a constant expression.
85+
// expected-note@+1{{declared here}}
6286
int baz();
63-
[[intel::loop_fuse(baz())]] void func19(); // expected-error{{'loop_fuse' attribute requires an integer constant}}
87+
// expected-error@+2{{expression is not an integral constant expression}}
88+
// expected-note@+1{{non-constexpr function 'baz' cannot be used in a constant expression}}
89+
[[intel::loop_fuse(baz() + 1)]] void func22();
90+
91+
// Test that checks expression is a constant expression.
92+
constexpr int bar() { return 0; }
93+
[[intel::loop_fuse(bar() + 2)]] void func23(); // OK
94+
95+
// Test that checks wrong function template instantiation and ensures that the type
96+
// is checked properly when instantiating from the template definition.
97+
template <typename Ty>
98+
// expected-error@+2 {{integral constant expression must have integral or unscoped enumeration type, not 'S'}}
99+
// expected-error@+1 {{integral constant expression must have integral or unscoped enumeration type, not 'float'}}
100+
[[intel::loop_fuse(Ty{})]] void func24() {}
101+
102+
struct S {};
103+
void test() {
104+
//expected-note@+1{{in instantiation of function template specialization 'func24<S>' requested here}}
105+
func24<S>();
106+
//expected-note@+1{{in instantiation of function template specialization 'func24<float>' requested here}}
107+
func24<float>();
108+
}

0 commit comments

Comments
 (0)