Skip to content

Commit 242054d

Browse files
smanna12smanna12
committed
Update patch and address review comments
Signed-off-by: Soumi Manna <[email protected]>
1 parent 66e2cc7 commit 242054d

File tree

4 files changed

+83
-135
lines changed

4 files changed

+83
-135
lines changed

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1304,6 +1304,11 @@ def SYCLIntelMaxWorkGroupSize : InheritableAttr {
13041304
ArrayRef<const Expr *> dimensions() const {
13051305
return {getXDim(), getYDim(), getZDim()};
13061306
}
1307+
bool isValueDependent() const {
1308+
return (getXDim() && getXDim()->isValueDependent()) ||
1309+
(getYDim() && getYDim()->isValueDependent()) ||
1310+
(getZDim() && getZDim()->isValueDependent());
1311+
}
13071312
Optional<llvm::APSInt> getXDimVal(ASTContext &Ctx) const {
13081313
return getXDim()->getIntegerConstantExpr(Ctx);
13091314
}
@@ -2845,7 +2850,7 @@ def ReqdWorkGroupSize : InheritableAttr {
28452850
ArrayRef<const Expr *> dimensions() const {
28462851
return {getXDim(), getYDim(), getZDim()};
28472852
}
2848-
bool isDependent() const {
2853+
bool isValueDependent() const {
28492854
return (getXDim() && getXDim()->isValueDependent()) ||
28502855
(getYDim() && getYDim()->isValueDependent()) ||
28512856
(getZDim() && getZDim()->isValueDependent());

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

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13074,22 +13074,22 @@ void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D,
1307413074
if (CI.getParsedKind() == ParsedAttr::AT_IntelReqdSubGroupSize) {
1307513075
if (ArgInt <= 0) {
1307613076
Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
13077-
<< CI << /*positive*/ 0;
13077+
<< CI.getAttrName() << /*positive*/ 0;
1307813078
return;
1307913079
}
1308013080
}
1308113081
if (CI.getParsedKind() == ParsedAttr::AT_SYCLIntelMaxGlobalWorkDim ||
1308213082
CI.getParsedKind() == ParsedAttr::AT_SYCLIntelNumSimdWorkItems) {
1308313083
if (ArgInt < 0) {
1308413084
Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
13085-
<< CI << /*non-negative*/ 1;
13085+
<< CI.getAttrName() << /*non-negative*/ 1;
1308613086
return;
1308713087
}
1308813088
}
1308913089
if (CI.getParsedKind() == ParsedAttr::AT_SYCLIntelMaxGlobalWorkDim) {
1309013090
if (ArgInt > 3) {
1309113091
Diag(E->getBeginLoc(), diag::err_attribute_argument_out_of_range)
13092-
<< CI << 0 << 3 << E->getSourceRange();
13092+
<< CI.getAttrName() << 0 << 3 << E->getSourceRange();
1309313093
return;
1309413094
}
1309513095
}
@@ -13098,8 +13098,8 @@ void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D,
1309813098
D->addAttr(::new (Context) AttrType(Context, CI, E));
1309913099
}
1310013100

13101-
static Expr *checkWorkSizeAttrExpr(Sema &S, const AttributeCommonInfo &CI,
13102-
Expr *E) {
13101+
static Expr *checkMaxWorkSizeAttrExpr(Sema &S, const AttributeCommonInfo &CI,
13102+
Expr *E) {
1310313103
assert(E && "Attribute must have an argument.");
1310413104

1310513105
if (!E->isInstantiationDependent()) {
@@ -13144,9 +13144,9 @@ void Sema::addIntelSYCLTripleArgFunctionAttr(Decl *D,
1314413144
!ZDimExpr->isValueDependent()) {
1314513145

1314613146
// Save ConstantExpr in semantic attribute
13147-
XDimExpr = checkWorkSizeAttrExpr(*this, CI, XDimExpr);
13148-
YDimExpr = checkWorkSizeAttrExpr(*this, CI, YDimExpr);
13149-
ZDimExpr = checkWorkSizeAttrExpr(*this, CI, ZDimExpr);
13147+
XDimExpr = checkMaxWorkSizeAttrExpr(*this, CI, XDimExpr);
13148+
YDimExpr = checkMaxWorkSizeAttrExpr(*this, CI, YDimExpr);
13149+
ZDimExpr = checkMaxWorkSizeAttrExpr(*this, CI, ZDimExpr);
1315013150

1315113151
if (!XDimExpr || !YDimExpr || !ZDimExpr)
1315213152
return;
@@ -13231,7 +13231,8 @@ FPGALoopAttrT *Sema::BuildSYCLIntelFPGALoopAttr(const AttributeCommonInfo &A,
1323113231

1323213232
if (!ArgVal) {
1323313233
Diag(E->getExprLoc(), diag::err_attribute_argument_type)
13234-
<< A << AANT_ArgumentIntegerConstant << E->getSourceRange();
13234+
<< A.getAttrName() << AANT_ArgumentIntegerConstant
13235+
<< E->getSourceRange();
1323513236
return nullptr;
1323613237
}
1323713238

@@ -13241,7 +13242,7 @@ FPGALoopAttrT *Sema::BuildSYCLIntelFPGALoopAttr(const AttributeCommonInfo &A,
1324113242
A.getParsedKind() == ParsedAttr::AT_SYCLIntelFPGALoopCoalesce) {
1324213243
if (Val <= 0) {
1324313244
Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
13244-
<< A << /* positive */ 0;
13245+
<< A.getAttrName() << /* positive */ 0;
1324513246
return nullptr;
1324613247
}
1324713248
} else if (A.getParsedKind() ==
@@ -13252,7 +13253,7 @@ FPGALoopAttrT *Sema::BuildSYCLIntelFPGALoopAttr(const AttributeCommonInfo &A,
1325213253
ParsedAttr::AT_SYCLIntelFPGASpeculatedIterations) {
1325313254
if (Val < 0) {
1325413255
Diag(E->getExprLoc(), diag::err_attribute_requires_positive_integer)
13255-
<< A << /* non-negative */ 1;
13256+
<< A.getAttrName() << /* non-negative */ 1;
1325613257
return nullptr;
1325713258
}
1325813259
} else {

‎clang/lib/Sema/SemaDeclAttr.cpp

Lines changed: 28 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -3095,21 +3095,6 @@ static bool checkWorkGroupSizeValues(Sema &S, Decl *D, const ParsedAttr &AL) {
30953095
return Result;
30963096
}
30973097

3098-
static Expr *checkWorkSizeAttrExpr(Sema &S, const ParsedAttr &CI, Expr *E) {
3099-
assert(E && "Attribute must have an argument.");
3100-
3101-
if (!E->isInstantiationDependent()) {
3102-
llvm::APSInt ArgVal;
3103-
ExprResult ICE = S.VerifyIntegerConstantExpression(E, &ArgVal);
3104-
3105-
if (ICE.isInvalid())
3106-
return nullptr;
3107-
3108-
E = ICE.get();
3109-
}
3110-
return E;
3111-
}
3112-
31133098
// Handles reqd_work_group_size and max_work_group_size.
31143099
template <typename WorkGroupAttr>
31153100
static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
@@ -3149,50 +3134,45 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
31493134

31503135
if (!XDimExpr->isValueDependent() && !YDimExpr->isValueDependent() &&
31513136
!ZDimExpr->isValueDependent()) {
3152-
Optional<llvm::APSInt> XDimVal = XDimExpr->getIntegerConstantExpr(Ctx);
3153-
Optional<llvm::APSInt> YDimVal = YDimExpr->getIntegerConstantExpr(Ctx);
3154-
Optional<llvm::APSInt> ZDimVal = ZDimExpr->getIntegerConstantExpr(Ctx);
3155-
3156-
XDimExpr = checkWorkSizeAttrExpr(S, AL, XDimExpr);
3157-
YDimExpr = checkWorkSizeAttrExpr(S, AL, YDimExpr);
3158-
ZDimExpr = checkWorkSizeAttrExpr(S, AL, ZDimExpr);
3137+
llvm::APSInt XDimVal, YDimVal, ZDimVal;
3138+
ExprResult XDim = S.VerifyIntegerConstantExpression(XDimExpr, &XDimVal);
3139+
ExprResult YDim = S.VerifyIntegerConstantExpression(YDimExpr, &YDimVal);
3140+
ExprResult ZDim = S.VerifyIntegerConstantExpression(ZDimExpr, &ZDimVal);
31593141

3160-
if (!XDimExpr || !YDimExpr || !ZDimExpr)
3142+
if (XDim.isInvalid())
31613143
return;
3144+
XDimExpr = XDim.get();
31623145

3163-
// Skip SEMA if we're in a template, this will be diagnosed later.
3164-
if (S.getCurLexicalContext()->isDependentContext())
3146+
if (YDim.isInvalid())
31653147
return;
3148+
YDimExpr = YDim.get();
31663149

3167-
if (AL.getKind() == ParsedAttr::AT_ReqdWorkGroupSize) {
3168-
if (const auto *A = D->getAttr<SYCLIntelNumSimdWorkItemsAttr>()) {
3169-
int64_t NumSimdWorkItems =
3170-
A->getValue()->getIntegerConstantExpr(Ctx)->getSExtValue();
3171-
3172-
if (!(XDimVal->getZExtValue() % NumSimdWorkItems == 0 ||
3173-
YDimVal->getZExtValue() % NumSimdWorkItems == 0 ||
3174-
ZDimVal->getZExtValue() % NumSimdWorkItems == 0)) {
3175-
S.Diag(A->getLocation(), diag::err_sycl_num_kernel_wrong_reqd_wg_size)
3176-
<< A << AL;
3177-
S.Diag(AL.getLoc(), diag::note_conflicting_attribute);
3178-
return;
3179-
}
3150+
if (ZDim.isInvalid())
3151+
return;
3152+
ZDimExpr = ZDim.get();
3153+
3154+
if (const auto *A = D->getAttr<SYCLIntelNumSimdWorkItemsAttr>()) {
3155+
int64_t NumSimdWorkItems =
3156+
A->getValue()->getIntegerConstantExpr(Ctx)->getSExtValue();
3157+
3158+
if (!(XDimVal.getZExtValue() % NumSimdWorkItems == 0 ||
3159+
YDimVal.getZExtValue() % NumSimdWorkItems == 0 ||
3160+
ZDimVal.getZExtValue() % NumSimdWorkItems == 0)) {
3161+
S.Diag(A->getLocation(), diag::err_sycl_num_kernel_wrong_reqd_wg_size)
3162+
<< A << AL;
3163+
S.Diag(AL.getLoc(), diag::note_conflicting_attribute);
3164+
return;
31803165
}
31813166
}
3182-
3183-
if (WorkGroupAttr *ExistingAttr = D->getAttr<WorkGroupAttr>()) {
3184-
Optional<llvm::APSInt> ExistingXDimVal = ExistingAttr->getXDimVal(Ctx);
3185-
Optional<llvm::APSInt> ExistingYDimVal = ExistingAttr->getYDimVal(Ctx);
3186-
Optional<llvm::APSInt> ExistingZDimVal = ExistingAttr->getZDimVal(Ctx);
3187-
3167+
if (const auto *ExistingAttr = D->getAttr<WorkGroupAttr>()) {
31883168
// Compare attribute arguments value and warn for a mismatch.
3189-
if (ExistingXDimVal != XDimVal || ExistingYDimVal != YDimVal ||
3190-
ExistingZDimVal != ZDimVal) {
3169+
if (ExistingAttr->getXDimVal(Ctx) != XDimVal ||
3170+
ExistingAttr->getYDimVal(Ctx) != YDimVal ||
3171+
ExistingAttr->getZDimVal(Ctx) != ZDimVal) {
31913172
S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL;
31923173
S.Diag(ExistingAttr->getLocation(), diag::note_conflicting_attribute);
31933174
}
31943175
}
3195-
31963176
if (!checkWorkGroupSizeValues(S, D, AL))
31973177
return;
31983178
}
@@ -3255,7 +3235,7 @@ static void handleNumSimdWorkItemsAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
32553235

32563236
S.CheckDeprecatedSYCLAttributeSpelling(AL);
32573237

3258-
if (!E->isValueDependent() || !E->isInstantiationDependent()) {
3238+
if (!E->isValueDependent()) {
32593239
ASTContext &Ctx = S.getASTContext();
32603240
llvm::APSInt ArgVal;
32613241
ExprResult ICE = S.VerifyIntegerConstantExpression(E, &ArgVal);

0 commit comments

Comments
 (0)