Skip to content

[SYCL][FPGA] Refactor of [[intel::force_pow2_depth()]] attribute #3387

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -2083,22 +2083,14 @@ def IntelFPGABankBits : Attr {
let Documentation = [IntelFPGABankBitsDocs];
}

def IntelFPGAForcePow2Depth : Attr {
def IntelFPGAForcePow2Depth : InheritableAttr {
let Spellings = [CXX11<"intelfpga","force_pow2_depth">,
CXX11<"intel","force_pow2_depth">];
let Args = [ExprArgument<"Value">];
let Subjects = SubjectList<[IntelFPGAConstVar, IntelFPGALocalStaticSlaveMemVar,
Field], ErrorDiag>;
let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
let Documentation = [IntelFPGAForcePow2DepthAttrDocs];
let AdditionalMembers = [{
static unsigned getMinValue() {
return 0;
}
static unsigned getMaxValue() {
return 1;
}
}];
}

def Naked : InheritableAttr {
Expand Down
24 changes: 5 additions & 19 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -10196,11 +10196,6 @@ class Sema final {
/// attribute to be added (usually because of a pragma).
void AddOptnoneAttributeIfNoConflicts(FunctionDecl *FD, SourceLocation Loc);

template <typename AttrType>
bool checkRangedIntegralArgument(Expr *E, const AttrType *TmpAttr,
ExprResult &Result);
template <typename AttrType>
void AddOneConstantValueAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E);
template <typename AttrType>
void AddOneConstantPowerTwoValueAttr(Decl *D, const AttributeCommonInfo &CI,
Expr *E);
Expand Down Expand Up @@ -10240,6 +10235,11 @@ class Sema final {
Expr *E);
IntelFPGAMaxReplicatesAttr *
MergeIntelFPGAMaxReplicatesAttr(Decl *D, const IntelFPGAMaxReplicatesAttr &A);
void AddIntelFPGAForcePow2DepthAttr(Decl *D, const AttributeCommonInfo &CI,
Expr *E);
IntelFPGAForcePow2DepthAttr *
MergeIntelFPGAForcePow2DepthAttr(Decl *D,
const IntelFPGAForcePow2DepthAttr &A);

/// AddAlignedAttr - Adds an aligned attribute to a particular declaration.
void AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E,
Expand Down Expand Up @@ -13175,20 +13175,6 @@ void Sema::addIntelTripleArgAttr(Decl *D, const AttributeCommonInfo &CI,
WorkGroupAttrType(Context, CI, XDimExpr, YDimExpr, ZDimExpr));
}

template <typename AttrType>
void Sema::AddOneConstantValueAttr(Decl *D, const AttributeCommonInfo &CI,
Expr *E) {
AttrType TmpAttr(Context, CI, E);

if (!E->isValueDependent()) {
ExprResult ICE;
if (checkRangedIntegralArgument<AttrType>(E, &TmpAttr, ICE))
return;
E = ICE.get();
}
D->addAttr(::new (Context) AttrType(Context, CI, E));
}

template <typename AttrType>
void Sema::AddOneConstantPowerTwoValueAttr(Decl *D,
const AttributeCommonInfo &CI,
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2628,6 +2628,8 @@ static bool mergeDeclAttribute(Sema &S, NamedDecl *D,
NewAttr = S.MergeSYCLIntelNoGlobalWorkOffsetAttr(D, *A);
else if (const auto *A = dyn_cast<IntelFPGAMaxReplicatesAttr>(Attr))
NewAttr = S.MergeIntelFPGAMaxReplicatesAttr(D, *A);
else if (const auto *A = dyn_cast<IntelFPGAForcePow2DepthAttr>(Attr))
NewAttr = S.MergeIntelFPGAForcePow2DepthAttr(D, *A);
else if (Attr->shouldInheritEvenIfAlreadyPresent() || !DeclHasAttr(D, Attr))
NewAttr = cast<InheritableAttr>(Attr->clone(S.Context));

Expand Down
99 changes: 74 additions & 25 deletions clang/lib/Sema/SemaDeclAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4452,24 +4452,6 @@ static void handleAlignedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
S.AddAlignedAttr(D, AL, E, AL.isPackExpansion());
}

template <typename AttrType>
bool Sema::checkRangedIntegralArgument(Expr *E, const AttrType *TmpAttr,
ExprResult &Result) {
llvm::APSInt Value;
Result = VerifyIntegerConstantExpression(E, &Value);
if (Result.isInvalid())
return true;

if (Value < AttrType::getMinValue() || Value > AttrType::getMaxValue()) {
Diag(TmpAttr->getRange().getBegin(),
diag::err_attribute_argument_out_of_range)
<< TmpAttr << AttrType::getMinValue() << AttrType::getMaxValue()
<< E->getSourceRange();
return true;
}
return false;
}

void Sema::AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E,
bool IsPackExpansion) {
AlignedAttr TmpAttr(Context, CI, true, E);
Expand Down Expand Up @@ -6205,21 +6187,88 @@ static void handleIntelFPGAPrivateCopiesAttr(Sema &S, Decl *D,
S.AddIntelFPGAPrivateCopiesAttr(D, A, A.getArgAsExpr(0));
}

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

if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(S, D, A))
// This attribute requires a range of values.
if (ArgVal < 0 || ArgVal > 1) {
Diag(E->getBeginLoc(), diag::err_attribute_argument_out_of_range)
<< CI << 0 << 1 << E->getSourceRange();
return;
}

// Check to see if there's a duplicate attribute with different values
// already applied to the declaration.
if (const auto *DeclAttr = D->getAttr<IntelFPGAForcePow2DepthAttr>()) {
// 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->getValue())) {
if (ArgVal != DeclExpr->getResultAsAPSInt()) {
Diag(CI.getLoc(), diag::warn_duplicate_attribute) << CI;
Diag(DeclAttr->getLoc(), diag::note_previous_attribute);
}
// If there is no mismatch, drop any duplicate attributes.
return;
}
}
}

// [[intel::fpga_register]] and [[intel::force_pow2_depth()]]
// attributes are incompatible.
if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(*this, D, CI))
return;

// If the declaration does not have an [[intel::fpga_memory]]
// attribute, this creates one as an implicit attribute.
if (!D->hasAttr<IntelFPGAMemoryAttr>())
D->addAttr(IntelFPGAMemoryAttr::CreateImplicit(
S.Context, IntelFPGAMemoryAttr::Default));
Context, IntelFPGAMemoryAttr::Default));

D->addAttr(::new (Context) IntelFPGAForcePow2DepthAttr(Context, CI, E));
}

IntelFPGAForcePow2DepthAttr *
Sema::MergeIntelFPGAForcePow2DepthAttr(Decl *D,
const IntelFPGAForcePow2DepthAttr &A) {
// Check to see if there's a duplicate attribute with different values
// already applied to the declaration.
if (const auto *DeclAttr = D->getAttr<IntelFPGAForcePow2DepthAttr>()) {
if (const auto *DeclExpr = dyn_cast<ConstantExpr>(DeclAttr->getValue())) {
if (const auto *MergeExpr = dyn_cast<ConstantExpr>(A.getValue())) {
if (DeclExpr->getResultAsAPSInt() != MergeExpr->getResultAsAPSInt()) {
Diag(DeclAttr->getLoc(), diag::warn_duplicate_attribute) << &A;
Diag(A.getLoc(), diag::note_previous_attribute);
}
// If there is no mismatch, drop any duplicate attributes.
return nullptr;
}
}
}

// [[intel::fpga_register]] and [[intel::force_pow2_depth()]]
// attributes are incompatible.
if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(*this, D, A))
return nullptr;

return ::new (Context) IntelFPGAForcePow2DepthAttr(Context, A, A.getValue());
}

static void handleIntelFPGAForcePow2DepthAttr(Sema &S, Decl *D,
const ParsedAttr &A) {
S.CheckDeprecatedSYCLAttributeSpelling(A);

S.AddOneConstantValueAttr<IntelFPGAForcePow2DepthAttr>(D, A,
A.getArgAsExpr(0));
S.AddIntelFPGAForcePow2DepthAttr(D, A, A.getArgAsExpr(0));
}

static void handleXRayLogArgsAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
Expand Down
3 changes: 1 addition & 2 deletions clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,7 @@ static void instantiateIntelFPGAForcePow2DepthAttr(
S, Sema::ExpressionEvaluationContext::ConstantEvaluated);
ExprResult Result = S.SubstExpr(Attr->getValue(), TemplateArgs);
if (!Result.isInvalid())
return S.AddOneConstantValueAttr<IntelFPGAForcePow2DepthAttr>(
New, *Attr, Result.getAs<Expr>());
return S.AddIntelFPGAForcePow2DepthAttr(New, *Attr, Result.getAs<Expr>());
}

static void instantiateIntelFPGABankWidthAttr(
Expand Down
33 changes: 33 additions & 0 deletions clang/test/SemaSYCL/intel-fpga-global-const.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,36 @@
//expected-note@+2{{conflicting attribute is here}}
[[intel::max_replicates(12)]] extern const int var_max_replicates_2;
[[intel::fpga_register]] const int var_max_replicates_2 =0;

// Test that checks global constant variable (which allows the redeclaration) since
// IntelFPGAConstVar is one of the subjects listed for [[intel::force_pow2_depth()]] attribute.

// Checking of duplicate argument values.
//CHECK: VarDecl{{.*}}force_pow2_depth
//CHECK: IntelFPGAMemoryAttr{{.*}}Implicit
//CHECK: IntelFPGAForcePow2DepthAttr
//CHECK-NEXT: ConstantExpr
//CHECK-NEXT: value:{{.*}}1
//CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
//CHECK: IntelFPGAForcePow2DepthAttr
//CHECK-NEXT: ConstantExpr
//CHECK-NEXT: value:{{.*}}1
//CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
[[intel::force_pow2_depth(1)]] extern const int var_force_pow2_depth;
[[intel::force_pow2_depth(1)]] const int var_force_pow2_depth = 0; // OK

// Merging of different arg values.
//expected-warning@+2{{attribute 'force_pow2_depth' is already applied with different arguments}}
[[intel::force_pow2_depth(1)]] extern const int var_force_pow2_depth_1;
[[intel::force_pow2_depth(0)]] const int var_force_pow2_depth_1 = 0;
//expected-note@-2{{previous attribute is here}}

// Merging of incompatible attributes.
// FIXME: Diagnostic order isn't correct, this isn't what we'd want here but
// this is an upstream issue. Merge function is calling here
// checkAttrMutualExclusion() function that has backwards diagnostic behavior.
// This should be fixed into upstream.
//expected-error@+2{{'force_pow2_depth' and 'fpga_register' attributes are not compatible}}
//expected-note@+2{{conflicting attribute is here}}
[[intel::force_pow2_depth(1)]] extern const int var_force_pow2_depth_2;
[[intel::fpga_register]] const int var_force_pow2_depth_2 =0;
26 changes: 16 additions & 10 deletions clang/test/SemaSYCL/intel-fpga-local.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,16 @@ void check_ast()
//CHECK-NEXT: IntegerLiteral{{.*}}12{{$}}
[[intel::private_copies(12)]]
[[intel::private_copies(12)]] int var_private_copies; // OK

// Checking of duplicate argument values.
//CHECK: VarDecl{{.*}}var_forcep2d
//CHECK: IntelFPGAMemoryAttr{{.*}}Implicit
//CHECK: IntelFPGAForcePow2DepthAttr
//CHECK-NEXT: ConstantExpr
//CHECK-NEXT: value:{{.*}}1
//CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
[[intel::force_pow2_depth(1)]]
[[intel::force_pow2_depth(1)]] int var_forcep2d; // OK
}

//CHECK: FunctionDecl{{.*}}diagnostics
Expand Down Expand Up @@ -608,17 +618,15 @@ void diagnostics()
//expected-error@+1{{'force_pow2_depth' attribute takes one argument}}
[[intel::force_pow2_depth(0, 1)]] unsigned int force_p2d_2_args[64];

// Checking of different argument values.
//CHECK: VarDecl{{.*}}force_p2d_dup
//CHECK: IntelFPGAMemoryAttr{{.*}}Implicit
//CHECK: IntelFPGAForcePow2DepthAttr
//CHECK-NEXT: ConstantExpr
//CHECK-NEXT: value:{{.*}}1
//CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
//CHECK: IntelFPGAForcePow2DepthAttr
//CHECK-NEXT: ConstantExpr
//CHECK-NEXT: value:{{.*}}0
//CHECK-NEXT: IntegerLiteral{{.*}}0{{$}}
//expected-warning@+1{{attribute 'force_pow2_depth' is already applied}}
//expected-note@+2{{previous attribute is here}}
//expected-warning@+1{{attribute 'force_pow2_depth' is already applied with different arguments}}
[[intel::force_pow2_depth(1), intel::force_pow2_depth(0)]] unsigned int force_p2d_dup[64];
}

Expand Down Expand Up @@ -857,6 +865,7 @@ void check_template_parameters() {
//expected-note@-1{{conflicting attribute is here}}
[[intel::force_pow2_depth(E)]] unsigned int reg_force_p2d[64];

// Test that checks template instantiations for different arg values.
//CHECK: VarDecl{{.*}}force_p2d_dup
//CHECK: IntelFPGAMemoryAttr{{.*}}Implicit
//CHECK: IntelFPGAForcePow2DepthAttr
Expand All @@ -865,11 +874,8 @@ void check_template_parameters() {
//CHECK-NEXT: SubstNonTypeTemplateParmExpr
//CHECK-NEXT: NonTypeTemplateParmDecl
//CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
//CHECK: IntelFPGAForcePow2DepthAttr
//CHECK: ConstantExpr
//CHECK-NEXT: value:{{.*}}0
//CHECK-NEXT: IntegerLiteral{{.*}}0{{$}}
//expected-warning@+1{{attribute 'force_pow2_depth' is already applied}}
//expected-note@+2{{previous attribute is here}}
//expected-warning@+1{{attribute 'force_pow2_depth' is already applied with different arguments}}
[[intel::force_pow2_depth(E), intel::force_pow2_depth(0)]] unsigned int force_p2d_dup[64];
}

Expand Down