Skip to content

[SYCL] Remove arbitrary upper bound for attribute argument #3134

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.

Sign up for GitHub

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 11 commits into from
Feb 17, 2021
Merged

[SYCL] Remove arbitrary upper bound for attribute argument #3134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
845ff48
[SYCL] Remove arbitrary range Part 1
elizabethandrews Feb 1, 2021
224b7eb
Rename function to be more generic
elizabethandrews Feb 1, 2021
9ba170f
Remove arb range part 2
elizabethandrews Feb 1, 2021
64b4698
Revert "Rename function to be more generic"
elizabethandrews Feb 12, 2021
bbac201
Merge remote-tracking branch 'intel_llvm/sycl' into arbrangepart2
elizabethandrews Feb 12, 2021
7f8e23b
Fix tests after merge and revert to less generic function names
elizabethandrews Feb 12, 2021
22753fe
Remove dead code
elizabethandrews Feb 12, 2021
6e4e924
Implement review comments
elizabethandrews Feb 12, 2021
65e683c
Remove arbitrary range for remaining attributes
elizabethandrews Feb 12, 2021
d4e9983
Windows fail fix and implement review comments
elizabethandrews Feb 12, 2021
68570c2
Fix MSVC build fail
elizabethandrews Feb 12, 2021
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
56 changes: 0 additions & 56 deletions clang/include/clang/Basic/Attr.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1282,14 +1282,6 @@ def SYCLIntelSchedulerTargetFmaxMhz : InheritableAttr {
let LangOpts = [SYCLIsDevice, SYCLIsHost];
let Subjects = SubjectList<[Function], ErrorDiag>;
let Documentation = [SYCLIntelSchedulerTargetFmaxMhzAttrDocs];
let AdditionalMembers = [{
static unsigned getMinValue() {
return 0;
}
static unsigned getMaxValue() {
return 1024*1024;
}
}];
}

def SYCLIntelMaxWorkGroupSize : InheritableAttr {
Expand Down Expand Up @@ -1344,14 +1336,6 @@ def SYCLIntelLoopFuse : InheritableAttr {
let Accessors = [Accessor<"isIndependent",
[CXX11<"intel", "loop_fuse_independent">]>];
let Documentation = [SYCLIntelLoopFuseDocs];
let AdditionalMembers = [{
static unsigned getMinValue() {
return 0;
}
static unsigned getMaxValue() {
return 1024*1024;
}
}];
}

def C11NoReturn : InheritableAttr {
Expand Down Expand Up @@ -2014,14 +1998,6 @@ def IntelFPGABankWidth : Attr {
Field], ErrorDiag>;
let LangOpts = [SYCLIsDevice, SYCLIsHost];
let Documentation = [IntelFPGABankWidthAttrDocs];
let AdditionalMembers = [{
static unsigned getMinValue() {
return 1;
}
static unsigned getMaxValue() {
return 1024*1024;
}
}];
}

def IntelFPGANumBanks : Attr {
Expand All @@ -2032,14 +2008,6 @@ def IntelFPGANumBanks : Attr {
Field], ErrorDiag>;
let LangOpts = [SYCLIsDevice, SYCLIsHost];
let Documentation = [IntelFPGANumBanksAttrDocs];
let AdditionalMembers = [{
static unsigned getMinValue() {
return 1;
}
static unsigned getMaxValue() {
return 1024*1024;
}
}];
}

def IntelFPGAPrivateCopies : InheritableAttr {
Expand All @@ -2049,14 +2017,6 @@ def IntelFPGAPrivateCopies : InheritableAttr {
let LangOpts = [SYCLIsDevice, SYCLIsHost];
let Subjects = SubjectList<[IntelFPGALocalNonConstVar, Field], ErrorDiag>;
let Documentation = [IntelFPGAPrivateCopiesAttrDocs];
let AdditionalMembers = [{
static unsigned getMinValue() {
return 0;
}
static unsigned getMaxValue() {
return 1024*1024;
}
}];
}

// Two string arguments.
Expand All @@ -2078,14 +2038,6 @@ def IntelFPGAMaxReplicates : Attr {
Field], ErrorDiag>;
let LangOpts = [SYCLIsDevice, SYCLIsHost];
let Documentation = [IntelFPGAMaxReplicatesAttrDocs];
let AdditionalMembers = [{
static unsigned getMinValue() {
return 1;
}
static unsigned getMaxValue() {
return 1024*1024;
}
}];
}

def IntelFPGASimpleDualPort : Attr {
Expand Down Expand Up @@ -2121,14 +2073,6 @@ def IntelFPGABankBits : Attr {
Field], ErrorDiag>;
let LangOpts = [SYCLIsDevice, SYCLIsHost];
let Documentation = [IntelFPGABankBitsDocs];
let AdditionalMembers = [{
static unsigned getMinValue() {
return 0;
}
static unsigned getMaxValue() {
return 1024*1024;
}
}];
}

def IntelFPGAForcePow2Depth : Attr {
Expand Down
63 changes: 35 additions & 28 deletions clang/include/clang/Sema/Sema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10206,12 +10206,10 @@ class Sema final {
void AddIntelFPGABankBitsAttr(Decl *D, const AttributeCommonInfo &CI,
Expr **Exprs, unsigned Size);
template <typename AttrType>
void addIntelSYCLSingleArgFunctionAttr(Decl *D, const AttributeCommonInfo &CI,
Expr *E);
void addIntelSingleArgAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E);
template <typename AttrType>
void addIntelSYCLTripleArgFunctionAttr(Decl *D, const AttributeCommonInfo &CI,
Expr *XDimExpr, Expr *YDimExpr,
Expr *ZDimExpr);
void addIntelTripleArgAttr(Decl *D, const AttributeCommonInfo &CI,
Expr *XDimExpr, Expr *YDimExpr, Expr *ZDimExpr);
/// AddAlignedAttr - Adds an aligned attribute to a particular declaration.
void AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E,
bool IsPackExpansion);
Expand Down Expand Up @@ -13059,9 +13057,8 @@ class Sema final {
};

template <typename AttrType>
void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D,
const AttributeCommonInfo &CI,
Expr *E) {
void Sema::addIntelSingleArgAttr(Decl *D, const AttributeCommonInfo &CI,
Expr *E) {
assert(E && "Attribute must have an argument.");

if (!E->isInstantiationDependent()) {
Expand All @@ -13072,27 +13069,42 @@ void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D,
E = ICE.get();
int32_t ArgInt = ArgVal.getSExtValue();
if (CI.getParsedKind() == ParsedAttr::AT_SYCLIntelNumSimdWorkItems ||
CI.getParsedKind() == ParsedAttr::AT_IntelReqdSubGroupSize) {
CI.getParsedKind() == ParsedAttr::AT_IntelReqdSubGroupSize ||
CI.getParsedKind() == ParsedAttr::AT_IntelFPGAMaxReplicates) {
if (ArgInt <= 0) {
Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
<< CI.getAttrName() << /*positive*/ 0;
<< CI << /*positive*/ 0;
return;
}
}
if (CI.getParsedKind() == ParsedAttr::AT_SYCLIntelMaxGlobalWorkDim) {
if (ArgInt < 0) {
Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
<< CI.getAttrName() << /*non-negative*/ 1;
<< CI << /*non-negative*/ 1;
return;
}
if (ArgInt > 3) {
Diag(E->getBeginLoc(), diag::err_attribute_argument_out_of_range)
<< CI.getAttrName() << 0 << 3 << E->getSourceRange();
<< CI << 0 << 3 << E->getSourceRange();
return;
}
}
if (CI.getParsedKind() == ParsedAttr::AT_SYCLIntelSchedulerTargetFmaxMhz ||
CI.getParsedKind() == ParsedAttr::AT_IntelFPGAPrivateCopies) {
if (ArgInt < 0) {
Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
<< CI << /*non-negative*/ 1;
return;
}
}
}

if (CI.getParsedKind() == ParsedAttr::AT_IntelFPGAPrivateCopies) {
if (!D->hasAttr<IntelFPGAMemoryAttr>())
D->addAttr(IntelFPGAMemoryAttr::CreateImplicit(
Context, IntelFPGAMemoryAttr::Default));
}

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

Expand Down Expand Up @@ -13128,10 +13140,9 @@ static Expr *checkMaxWorkSizeAttrExpr(Sema &S, const AttributeCommonInfo &CI,
}

template <typename WorkGroupAttrType>
void Sema::addIntelSYCLTripleArgFunctionAttr(Decl *D,
const AttributeCommonInfo &CI,
Expr *XDimExpr, Expr *YDimExpr,
Expr *ZDimExpr) {
void Sema::addIntelTripleArgAttr(Decl *D, const AttributeCommonInfo &CI,
Expr *XDimExpr, Expr *YDimExpr,
Expr *ZDimExpr) {

assert((XDimExpr && YDimExpr && ZDimExpr) &&
"argument has unexpected null value");
Expand Down Expand Up @@ -13164,13 +13175,6 @@ void Sema::AddOneConstantValueAttr(Decl *D, const AttributeCommonInfo &CI,
return;
E = ICE.get();
}

if (IntelFPGAPrivateCopiesAttr::classof(&TmpAttr)) {
if (!D->hasAttr<IntelFPGAMemoryAttr>())
D->addAttr(IntelFPGAMemoryAttr::CreateImplicit(
Context, IntelFPGAMemoryAttr::Default));
}

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

Expand All @@ -13181,12 +13185,15 @@ void Sema::AddOneConstantPowerTwoValueAttr(Decl *D,
AttrType TmpAttr(Context, CI, E);

if (!E->isValueDependent()) {
ExprResult ICE;
if (checkRangedIntegralArgument<AttrType>(E, &TmpAttr, ICE))
llvm::APSInt Value;
ExprResult ICE = VerifyIntegerConstantExpression(E, &Value);
if (ICE.isInvalid())
return;
Expr::EvalResult Result;
E->EvaluateAsInt(Result, Context);
llvm::APSInt Value = Result.Val.getInt();
if (!Value.isStrictlyPositive()) {
Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
<< CI << /*positive*/ 0;
return;
}
if (!Value.isPowerOf2()) {
Diag(CI.getLoc(), diag::err_attribute_argument_not_power_of_two)
<< &TmpAttr;
Expand Down
43 changes: 22 additions & 21 deletions clang/lib/Sema/SemaDeclAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3150,8 +3150,7 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
if (!checkWorkGroupSizeValues(S, D, AL))
return;

S.addIntelSYCLTripleArgFunctionAttr<WorkGroupAttr>(D, AL, XDimExpr, YDimExpr,
ZDimExpr);
S.addIntelTripleArgAttr<WorkGroupAttr>(D, AL, XDimExpr, YDimExpr, ZDimExpr);
}

// Handles work_group_size_hint.
Expand Down Expand Up @@ -3193,7 +3192,7 @@ static void handleSubGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
if (D->getAttr<IntelReqdSubGroupSizeAttr>())
S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL;

S.addIntelSYCLSingleArgFunctionAttr<IntelReqdSubGroupSizeAttr>(D, AL, E);
S.addIntelSingleArgAttr<IntelReqdSubGroupSizeAttr>(D, AL, E);
}

// Handles num_simd_work_items.
Expand All @@ -3208,7 +3207,7 @@ static void handleNumSimdWorkItemsAttr(Sema &S, Decl *D, const ParsedAttr &A) {

S.CheckDeprecatedSYCLAttributeSpelling(A);

S.addIntelSYCLSingleArgFunctionAttr<SYCLIntelNumSimdWorkItemsAttr>(D, A, E);
S.addIntelSingleArgAttr<SYCLIntelNumSimdWorkItemsAttr>(D, A, E);
}

// Handles use_stall_enable_clusters
Expand Down Expand Up @@ -3239,7 +3238,7 @@ static void handleSchedulerTargetFmaxMhzAttr(Sema &S, Decl *D,

S.CheckDeprecatedSYCLAttributeSpelling(AL);

S.AddOneConstantValueAttr<SYCLIntelSchedulerTargetFmaxMhzAttr>(D, AL, E);
S.addIntelSingleArgAttr<SYCLIntelSchedulerTargetFmaxMhzAttr>(D, AL, E);
}

// Handles max_global_work_dim.
Expand All @@ -3259,7 +3258,7 @@ static void handleMaxGlobalWorkDimAttr(Sema &S, Decl *D, const ParsedAttr &A) {

S.CheckDeprecatedSYCLAttributeSpelling(A);

S.addIntelSYCLSingleArgFunctionAttr<SYCLIntelMaxGlobalWorkDimAttr>(D, A, E);
S.addIntelSingleArgAttr<SYCLIntelMaxGlobalWorkDimAttr>(D, A, E);
}

SYCLIntelLoopFuseAttr *
Expand Down Expand Up @@ -3312,10 +3311,13 @@ static bool checkSYCLIntelLoopFuseArgument(Sema &S,
return true;
}

SYCLIntelLoopFuseAttr TmpAttr(S.Context, CI, E);
ExprResult ICE;
if (!ArgVal->isNonNegative()) {
S.Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
<< CI << /*non-negative*/ 1;
return true;
}

return S.checkRangedIntegralArgument<SYCLIntelLoopFuseAttr>(E, &TmpAttr, ICE);
return false;
}

void Sema::addSYCLIntelLoopFuseAttr(Decl *D, const AttributeCommonInfo &CI,
Expand Down Expand Up @@ -5583,7 +5585,7 @@ static void handleNoGlobalWorkOffsetAttr(Sema &S, Decl *D,
? A.getArgAsExpr(0)
: IntegerLiteral::Create(S.Context, llvm::APInt(32, 1),
S.Context.IntTy, A.getLoc());
S.addIntelSYCLSingleArgFunctionAttr<SYCLIntelNoGlobalWorkOffsetAttr>(D, A, E);
S.addIntelSingleArgAttr<SYCLIntelNoGlobalWorkOffsetAttr>(D, A, E);
}

/// Handle the [[intelfpga::doublepump]] and [[intelfpga::singlepump]] attributes.
Expand Down Expand Up @@ -5716,9 +5718,7 @@ static void handleOneConstantPowerTwoValueAttr(Sema &S, Decl *D,
if (checkAttrMutualExclusion<IntelFPGARegisterAttr>(S, D, A))
return;

if (A.getKind() == ParsedAttr::AT_IntelFPGABankWidth ||
A.getKind() == ParsedAttr::AT_IntelFPGANumBanks)
S.CheckDeprecatedSYCLAttributeSpelling(A);
S.CheckDeprecatedSYCLAttributeSpelling(A);

S.AddOneConstantPowerTwoValueAttr<AttrType>(D, A, A.getArgAsExpr(0));
}
Expand Down Expand Up @@ -5755,8 +5755,7 @@ static void handleIntelFPGAMaxReplicatesAttr(Sema &S, Decl *D,

S.CheckDeprecatedSYCLAttributeSpelling(A);

S.AddOneConstantValueAttr<IntelFPGAMaxReplicatesAttr>(D, A,
A.getArgAsExpr(0));
S.addIntelSingleArgAttr<IntelFPGAMaxReplicatesAttr>(D, A, A.getArgAsExpr(0));
}

/// Handle the merge attribute.
Expand Down Expand Up @@ -5833,11 +5832,14 @@ void Sema::AddIntelFPGABankBitsAttr(Decl *D, const AttributeCommonInfo &CI,
Expr::EvalResult Result;
ListIsValueDep = ListIsValueDep || E->isValueDependent();
if (!E->isValueDependent()) {
ExprResult ICE;
if (checkRangedIntegralArgument<IntelFPGABankBitsAttr>(E, &TmpAttr, ICE))
ExprResult ICE = VerifyIntegerConstantExpression(E, &Value);
if (ICE.isInvalid())
return;
if (!Value.isNonNegative()) {
Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
<< CI << /*non-negative*/ 1;
return;
if (E->EvaluateAsInt(Result, Context))
Value = Result.Val.getInt();
}
E = ICE.get();
}
Args.push_back(E);
Expand Down Expand Up @@ -5893,8 +5895,7 @@ static void handleIntelFPGAPrivateCopiesAttr(Sema &S, Decl *D,

S.CheckDeprecatedSYCLAttributeSpelling(A);

S.AddOneConstantValueAttr<IntelFPGAPrivateCopiesAttr>(D, A,
A.getArgAsExpr(0));
S.addIntelSingleArgAttr<IntelFPGAPrivateCopiesAttr>(D, A, A.getArgAsExpr(0));
}

static void handleIntelFPGAForcePow2DepthAttr(Sema &S, Decl *D,
Expand Down
Loading