Skip to content

Commit d02879c

Browse files
smanna12smanna12
authored
[SYCL][FPGA] Refactor of [[intel::force_pow2_depth()]] attribute (#3387)
This patch 1. refactors FPGA Decl attribute [[intel::force_pow2_depth()]] to better fit for community standards. 2. refactors the way we handle duplicate attributes with same and different argument values and mutually exclusive attributes logic when present on a given declaration. 3. handles redeclarations or template instantiations properly. 4. adds tests 5. removes AddOneConstantValueAttr() function. 6. adds tests to check for global constant variable (which allows the redeclaration) since IntelFPGAConstVar is one of the subjects listed for [[intel::force_pow2_depth()]] attribute. 7. removes arbitrary upper bound check for attribute argument. Signed-off-by: soumi.manna <[email protected]>
1 parent 8e40116 commit d02879c

File tree

7 files changed

+132
-65
lines changed

7 files changed

+132
-65
lines changed

‎clang/include/clang/Basic/Attr.td

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2101,22 +2101,14 @@ def IntelFPGABankBits : Attr {
21012101
let Documentation = [IntelFPGABankBitsDocs];
21022102
}
21032103

2104-
def IntelFPGAForcePow2Depth : Attr {
2104+
def IntelFPGAForcePow2Depth : InheritableAttr {
21052105
let Spellings = [CXX11<"intelfpga","force_pow2_depth">,
21062106
CXX11<"intel","force_pow2_depth">];
21072107
let Args = [ExprArgument<"Value">];
21082108
let Subjects = SubjectList<[IntelFPGAConstVar, IntelFPGALocalStaticSlaveMemVar,
21092109
Field], ErrorDiag>;
21102110
let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
21112111
let Documentation = [IntelFPGAForcePow2DepthAttrDocs];
2112-
let AdditionalMembers = [{
2113-
static unsigned getMinValue() {
2114-
return 0;
2115-
}
2116-
static unsigned getMaxValue() {
2117-
return 1;
2118-
}
2119-
}];
21202112
}
21212113

21222114
def Naked : InheritableAttr {

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

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10197,11 +10197,6 @@ class Sema final {
1019710197
/// attribute to be added (usually because of a pragma).
1019810198
void AddOptnoneAttributeIfNoConflicts(FunctionDecl *FD, SourceLocation Loc);
1019910199

10200-
template <typename AttrType>
10201-
bool checkRangedIntegralArgument(Expr *E, const AttrType *TmpAttr,
10202-
ExprResult &Result);
10203-
template <typename AttrType>
10204-
void AddOneConstantValueAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E);
1020510200
template <typename AttrType>
1020610201
void AddOneConstantPowerTwoValueAttr(Decl *D, const AttributeCommonInfo &CI,
1020710202
Expr *E);
@@ -10241,6 +10236,11 @@ class Sema final {
1024110236
Expr *E);
1024210237
IntelFPGAMaxReplicatesAttr *
1024310238
MergeIntelFPGAMaxReplicatesAttr(Decl *D, const IntelFPGAMaxReplicatesAttr &A);
10239+
void AddIntelFPGAForcePow2DepthAttr(Decl *D, const AttributeCommonInfo &CI,
10240+
Expr *E);
10241+
IntelFPGAForcePow2DepthAttr *
10242+
MergeIntelFPGAForcePow2DepthAttr(Decl *D,
10243+
const IntelFPGAForcePow2DepthAttr &A);
1024410244

1024510245
/// AddAlignedAttr - Adds an aligned attribute to a particular declaration.
1024610246
void AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E,
@@ -13176,20 +13176,6 @@ void Sema::addIntelTripleArgAttr(Decl *D, const AttributeCommonInfo &CI,
1317613176
WorkGroupAttrType(Context, CI, XDimExpr, YDimExpr, ZDimExpr));
1317713177
}
1317813178

13179-
template <typename AttrType>
13180-
void Sema::AddOneConstantValueAttr(Decl *D, const AttributeCommonInfo &CI,
13181-
Expr *E) {
13182-
AttrType TmpAttr(Context, CI, E);
13183-
13184-
if (!E->isValueDependent()) {
13185-
ExprResult ICE;
13186-
if (checkRangedIntegralArgument<AttrType>(E, &TmpAttr, ICE))
13187-
return;
13188-
E = ICE.get();
13189-
}
13190-
D->addAttr(::new (Context) AttrType(Context, CI, E));
13191-
}
13192-
1319313179
template <typename AttrType>
1319413180
void Sema::AddOneConstantPowerTwoValueAttr(Decl *D,
1319513181
const AttributeCommonInfo &CI,

‎clang/lib/Sema/SemaDecl.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2628,6 +2628,8 @@ static bool mergeDeclAttribute(Sema &S, NamedDecl *D,
26282628
NewAttr = S.MergeSYCLIntelNoGlobalWorkOffsetAttr(D, *A);
26292629
else if (const auto *A = dyn_cast<IntelFPGAMaxReplicatesAttr>(Attr))
26302630
NewAttr = S.MergeIntelFPGAMaxReplicatesAttr(D, *A);
2631+
else if (const auto *A = dyn_cast<IntelFPGAForcePow2DepthAttr>(Attr))
2632+
NewAttr = S.MergeIntelFPGAForcePow2DepthAttr(D, *A);
26312633
else if (Attr->shouldInheritEvenIfAlreadyPresent() || !DeclHasAttr(D, Attr))
26322634
NewAttr = cast<InheritableAttr>(Attr->clone(S.Context));
26332635

‎clang/lib/Sema/SemaDeclAttr.cpp

Lines changed: 74 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4452,24 +4452,6 @@ static void handleAlignedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
44524452
S.AddAlignedAttr(D, AL, E, AL.isPackExpansion());
44534453
}
44544454

4455-
template <typename AttrType>
4456-
bool Sema::checkRangedIntegralArgument(Expr *E, const AttrType *TmpAttr,
4457-
ExprResult &Result) {
4458-
llvm::APSInt Value;
4459-
Result = VerifyIntegerConstantExpression(E, &Value);
4460-
if (Result.isInvalid())
4461-
return true;
4462-
4463-
if (Value < AttrType::getMinValue() || Value > AttrType::getMaxValue()) {
4464-
Diag(TmpAttr->getRange().getBegin(),
4465-
diag::err_attribute_argument_out_of_range)
4466-
<< TmpAttr << AttrType::getMinValue() << AttrType::getMaxValue()
4467-
<< E->getSourceRange();
4468-
return true;
4469-
}
4470-
return false;
4471-
}
4472-
44734455
void Sema::AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E,
44744456
bool IsPackExpansion) {
44754457
AlignedAttr TmpAttr(Context, CI, true, E);
@@ -6212,21 +6194,88 @@ static void handleIntelFPGAPrivateCopiesAttr(Sema &S, Decl *D,
62126194
S.AddIntelFPGAPrivateCopiesAttr(D, A, A.getArgAsExpr(0));
62136195
}
62146196

6215-
static void handleIntelFPGAForcePow2DepthAttr(Sema &S, Decl *D,
6216-
const ParsedAttr &A) {
6217-
checkForDuplicateAttribute<IntelFPGAForcePow2DepthAttr>(S, D, A);
6197+
void Sema::AddIntelFPGAForcePow2DepthAttr(Decl *D,
6198+
const AttributeCommonInfo &CI,
6199+
Expr *E) {
6200+
if (!E->isValueDependent()) {
6201+
// Validate that we have an integer constant expression and then store the
6202+
// converted constant expression into the semantic attribute so that we
6203+
// don't have to evaluate it again later.
6204+
llvm::APSInt ArgVal;
6205+
ExprResult Res = VerifyIntegerConstantExpression(E, &ArgVal);
6206+
if (Res.isInvalid())
6207+
return;
6208+
E = Res.get();
62186209

6219-
if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(S, D, A))
6210+
// This attribute requires a range of values.
6211+
if (ArgVal < 0 || ArgVal > 1) {
6212+
Diag(E->getBeginLoc(), diag::err_attribute_argument_out_of_range)
6213+
<< CI << 0 << 1 << E->getSourceRange();
6214+
return;
6215+
}
6216+
6217+
// Check to see if there's a duplicate attribute with different values
6218+
// already applied to the declaration.
6219+
if (const auto *DeclAttr = D->getAttr<IntelFPGAForcePow2DepthAttr>()) {
6220+
// If the other attribute argument is instantiation dependent, we won't
6221+
// have converted it to a constant expression yet and thus we test
6222+
// whether this is a null pointer.
6223+
if (const auto *DeclExpr = dyn_cast<ConstantExpr>(DeclAttr->getValue())) {
6224+
if (ArgVal != DeclExpr->getResultAsAPSInt()) {
6225+
Diag(CI.getLoc(), diag::warn_duplicate_attribute) << CI;
6226+
Diag(DeclAttr->getLoc(), diag::note_previous_attribute);
6227+
}
6228+
// If there is no mismatch, drop any duplicate attributes.
6229+
return;
6230+
}
6231+
}
6232+
}
6233+
6234+
// [[intel::fpga_register]] and [[intel::force_pow2_depth()]]
6235+
// attributes are incompatible.
6236+
if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(*this, D, CI))
62206237
return;
62216238

6239+
// If the declaration does not have an [[intel::fpga_memory]]
6240+
// attribute, this creates one as an implicit attribute.
62226241
if (!D->hasAttr<IntelFPGAMemoryAttr>())
62236242
D->addAttr(IntelFPGAMemoryAttr::CreateImplicit(
6224-
S.Context, IntelFPGAMemoryAttr::Default));
6243+
Context, IntelFPGAMemoryAttr::Default));
6244+
6245+
D->addAttr(::new (Context) IntelFPGAForcePow2DepthAttr(Context, CI, E));
6246+
}
6247+
6248+
IntelFPGAForcePow2DepthAttr *
6249+
Sema::MergeIntelFPGAForcePow2DepthAttr(Decl *D,
6250+
const IntelFPGAForcePow2DepthAttr &A) {
6251+
// Check to see if there's a duplicate attribute with different values
6252+
// already applied to the declaration.
6253+
if (const auto *DeclAttr = D->getAttr<IntelFPGAForcePow2DepthAttr>()) {
6254+
if (const auto *DeclExpr = dyn_cast<ConstantExpr>(DeclAttr->getValue())) {
6255+
if (const auto *MergeExpr = dyn_cast<ConstantExpr>(A.getValue())) {
6256+
if (DeclExpr->getResultAsAPSInt() != MergeExpr->getResultAsAPSInt()) {
6257+
Diag(DeclAttr->getLoc(), diag::warn_duplicate_attribute) << &A;
6258+
Diag(A.getLoc(), diag::note_previous_attribute);
6259+
}
6260+
// If there is no mismatch, drop any duplicate attributes.
6261+
return nullptr;
6262+
}
6263+
}
6264+
}
62256265

6266+
// [[intel::fpga_register]] and [[intel::force_pow2_depth()]]
6267+
// attributes are incompatible.
6268+
if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(*this, D, A))
6269+
return nullptr;
6270+
6271+
return ::new (Context) IntelFPGAForcePow2DepthAttr(Context, A, A.getValue());
6272+
}
6273+
6274+
static void handleIntelFPGAForcePow2DepthAttr(Sema &S, Decl *D,
6275+
const ParsedAttr &A) {
62266276
S.CheckDeprecatedSYCLAttributeSpelling(A);
62276277

6228-
S.AddOneConstantValueAttr<IntelFPGAForcePow2DepthAttr>(D, A,
6229-
A.getArgAsExpr(0));
6278+
S.AddIntelFPGAForcePow2DepthAttr(D, A, A.getArgAsExpr(0));
62306279
}
62316280

62326281
static void handleXRayLogArgsAttr(Sema &S, Decl *D, const ParsedAttr &AL) {

‎clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -581,8 +581,7 @@ static void instantiateIntelFPGAForcePow2DepthAttr(
581581
S, Sema::ExpressionEvaluationContext::ConstantEvaluated);
582582
ExprResult Result = S.SubstExpr(Attr->getValue(), TemplateArgs);
583583
if (!Result.isInvalid())
584-
return S.AddOneConstantValueAttr<IntelFPGAForcePow2DepthAttr>(
585-
New, *Attr, Result.getAs<Expr>());
584+
return S.AddIntelFPGAForcePow2DepthAttr(New, *Attr, Result.getAs<Expr>());
586585
}
587586

588587
static void instantiateIntelFPGABankWidthAttr(

‎clang/test/SemaSYCL/intel-fpga-global-const.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,36 @@
3232
//expected-note@+2{{conflicting attribute is here}}
3333
[[intel::max_replicates(12)]] extern const int var_max_replicates_2;
3434
[[intel::fpga_register]] const int var_max_replicates_2 =0;
35+
36+
// Test that checks global constant variable (which allows the redeclaration) since
37+
// IntelFPGAConstVar is one of the subjects listed for [[intel::force_pow2_depth()]] attribute.
38+
39+
// Checking of duplicate argument values.
40+
//CHECK: VarDecl{{.*}}force_pow2_depth
41+
//CHECK: IntelFPGAMemoryAttr{{.*}}Implicit
42+
//CHECK: IntelFPGAForcePow2DepthAttr
43+
//CHECK-NEXT: ConstantExpr
44+
//CHECK-NEXT: value:{{.*}}1
45+
//CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
46+
//CHECK: IntelFPGAForcePow2DepthAttr
47+
//CHECK-NEXT: ConstantExpr
48+
//CHECK-NEXT: value:{{.*}}1
49+
//CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
50+
[[intel::force_pow2_depth(1)]] extern const int var_force_pow2_depth;
51+
[[intel::force_pow2_depth(1)]] const int var_force_pow2_depth = 0; // OK
52+
53+
// Merging of different arg values.
54+
//expected-warning@+2{{attribute 'force_pow2_depth' is already applied with different arguments}}
55+
[[intel::force_pow2_depth(1)]] extern const int var_force_pow2_depth_1;
56+
[[intel::force_pow2_depth(0)]] const int var_force_pow2_depth_1 = 0;
57+
//expected-note@-2{{previous attribute is here}}
58+
59+
// Merging of incompatible attributes.
60+
// FIXME: Diagnostic order isn't correct, this isn't what we'd want here but
61+
// this is an upstream issue. Merge function is calling here
62+
// checkAttrMutualExclusion() function that has backwards diagnostic behavior.
63+
// This should be fixed into upstream.
64+
//expected-error@+2{{'force_pow2_depth' and 'fpga_register' attributes are not compatible}}
65+
//expected-note@+2{{conflicting attribute is here}}
66+
[[intel::force_pow2_depth(1)]] extern const int var_force_pow2_depth_2;
67+
[[intel::fpga_register]] const int var_force_pow2_depth_2 =0;

‎clang/test/SemaSYCL/intel-fpga-local.cpp

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,16 @@ void check_ast()
175175
//CHECK-NEXT: IntegerLiteral{{.*}}12{{$}}
176176
[[intel::private_copies(12)]]
177177
[[intel::private_copies(12)]] int var_private_copies; // OK
178+
179+
// Checking of duplicate argument values.
180+
//CHECK: VarDecl{{.*}}var_forcep2d
181+
//CHECK: IntelFPGAMemoryAttr{{.*}}Implicit
182+
//CHECK: IntelFPGAForcePow2DepthAttr
183+
//CHECK-NEXT: ConstantExpr
184+
//CHECK-NEXT: value:{{.*}}1
185+
//CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
186+
[[intel::force_pow2_depth(1)]]
187+
[[intel::force_pow2_depth(1)]] int var_forcep2d; // OK
178188
}
179189

180190
//CHECK: FunctionDecl{{.*}}diagnostics
@@ -613,17 +623,15 @@ void diagnostics()
613623
//expected-error@+1{{'force_pow2_depth' attribute takes one argument}}
614624
[[intel::force_pow2_depth(0, 1)]] unsigned int force_p2d_2_args[64];
615625

626+
// Checking of different argument values.
616627
//CHECK: VarDecl{{.*}}force_p2d_dup
617628
//CHECK: IntelFPGAMemoryAttr{{.*}}Implicit
618629
//CHECK: IntelFPGAForcePow2DepthAttr
619630
//CHECK-NEXT: ConstantExpr
620631
//CHECK-NEXT: value:{{.*}}1
621632
//CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
622-
//CHECK: IntelFPGAForcePow2DepthAttr
623-
//CHECK-NEXT: ConstantExpr
624-
//CHECK-NEXT: value:{{.*}}0
625-
//CHECK-NEXT: IntegerLiteral{{.*}}0{{$}}
626-
//expected-warning@+1{{attribute 'force_pow2_depth' is already applied}}
633+
//expected-note@+2{{previous attribute is here}}
634+
//expected-warning@+1{{attribute 'force_pow2_depth' is already applied with different arguments}}
627635
[[intel::force_pow2_depth(1), intel::force_pow2_depth(0)]] unsigned int force_p2d_dup[64];
628636
}
629637

@@ -864,6 +872,7 @@ void check_template_parameters() {
864872
//expected-note@-1{{conflicting attribute is here}}
865873
[[intel::force_pow2_depth(E)]] unsigned int reg_force_p2d[64];
866874

875+
// Test that checks template instantiations for different arg values.
867876
//CHECK: VarDecl{{.*}}force_p2d_dup
868877
//CHECK: IntelFPGAMemoryAttr{{.*}}Implicit
869878
//CHECK: IntelFPGAForcePow2DepthAttr
@@ -872,11 +881,8 @@ void check_template_parameters() {
872881
//CHECK-NEXT: SubstNonTypeTemplateParmExpr
873882
//CHECK-NEXT: NonTypeTemplateParmDecl
874883
//CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
875-
//CHECK: IntelFPGAForcePow2DepthAttr
876-
//CHECK: ConstantExpr
877-
//CHECK-NEXT: value:{{.*}}0
878-
//CHECK-NEXT: IntegerLiteral{{.*}}0{{$}}
879-
//expected-warning@+1{{attribute 'force_pow2_depth' is already applied}}
884+
//expected-note@+2{{previous attribute is here}}
885+
//expected-warning@+1{{attribute 'force_pow2_depth' is already applied with different arguments}}
880886
[[intel::force_pow2_depth(E), intel::force_pow2_depth(0)]] unsigned int force_p2d_dup[64];
881887
}
882888

0 commit comments

Comments
 (0)