Skip to content

Commit 42c09e3

Browse files
AaronBallmanAaronBallman
authored
Fix the clang-tidy build after work_group_size attr changes. (#3016)
SingleWorkItemBarrierCheck.cpp from upstream does not work with our recent changes in ca9d816. This addresses the issue by adding some helper methods to the work group size attributes and using them where appropriate.
1 parent 0983121 commit 42c09e3

File tree

7 files changed

+69
-88
lines changed

7 files changed

+69
-88
lines changed

‎clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,9 @@ void SingleWorkItemBarrierCheck::check(const MatchFinder::MatchResult &Result) {
5757
bool IsNDRange = false;
5858
if (MatchedDecl->hasAttr<ReqdWorkGroupSizeAttr>()) {
5959
const auto *Attribute = MatchedDecl->getAttr<ReqdWorkGroupSizeAttr>();
60-
if (Attribute->getXDim() > 1 || Attribute->getYDim() > 1 ||
61-
Attribute->getZDim() > 1)
60+
if (*Attribute->getXDimVal(*Result.Context) > 1 ||
61+
*Attribute->getYDimVal(*Result.Context) > 1 ||
62+
*Attribute->getZDimVal(*Result.Context) > 1)
6263
IsNDRange = true;
6364
}
6465
if (IsNDRange) // No warning if kernel is treated as an NDRange.

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,6 +1302,15 @@ def SYCLIntelMaxWorkGroupSize : InheritableAttr {
13021302
ArrayRef<const Expr *> dimensions() const {
13031303
return {getXDim(), getYDim(), getZDim()};
13041304
}
1305+
Optional<llvm::APSInt> getXDimVal(ASTContext &Ctx) const {
1306+
return getXDim()->getIntegerConstantExpr(Ctx);
1307+
}
1308+
Optional<llvm::APSInt> getYDimVal(ASTContext &Ctx) const {
1309+
return getYDim()->getIntegerConstantExpr(Ctx);
1310+
}
1311+
Optional<llvm::APSInt> getZDimVal(ASTContext &Ctx) const {
1312+
return getZDim()->getIntegerConstantExpr(Ctx);
1313+
}
13051314
}];
13061315
let Documentation = [SYCLIntelMaxWorkGroupSizeAttrDocs];
13071316
}
@@ -2853,6 +2862,15 @@ def ReqdWorkGroupSize : InheritableAttr {
28532862
ArrayRef<const Expr *> dimensions() const {
28542863
return {getXDim(), getYDim(), getZDim()};
28552864
}
2865+
Optional<llvm::APSInt> getXDimVal(ASTContext &Ctx) const {
2866+
return getXDim()->getIntegerConstantExpr(Ctx);
2867+
}
2868+
Optional<llvm::APSInt> getYDimVal(ASTContext &Ctx) const {
2869+
return getYDim()->getIntegerConstantExpr(Ctx);
2870+
}
2871+
Optional<llvm::APSInt> getZDimVal(ASTContext &Ctx) const {
2872+
return getZDim()->getIntegerConstantExpr(Ctx);
2873+
}
28562874
}];
28572875
let Documentation = [ReqdWorkGroupSizeAttrDocs];
28582876
}

‎clang/lib/CodeGen/CodeGenFunction.cpp

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -621,12 +621,10 @@ void CodeGenFunction::EmitOpenCLKernelMetadata(const FunctionDecl *FD,
621621

622622
if (const ReqdWorkGroupSizeAttr *A = FD->getAttr<ReqdWorkGroupSizeAttr>()) {
623623
llvm::LLVMContext &Context = getLLVMContext();
624-
Optional<llvm::APSInt> XDimVal =
625-
A->getXDim()->getIntegerConstantExpr(FD->getASTContext());
626-
Optional<llvm::APSInt> YDimVal =
627-
A->getYDim()->getIntegerConstantExpr(FD->getASTContext());
628-
Optional<llvm::APSInt> ZDimVal =
629-
A->getZDim()->getIntegerConstantExpr(FD->getASTContext());
624+
ASTContext &ClangCtx = FD->getASTContext();
625+
Optional<llvm::APSInt> XDimVal = A->getXDimVal(ClangCtx);
626+
Optional<llvm::APSInt> YDimVal = A->getYDimVal(ClangCtx);
627+
Optional<llvm::APSInt> ZDimVal = A->getZDimVal(ClangCtx);
630628

631629
// For a SYCLDevice ReqdWorkGroupSizeAttr arguments are reversed.
632630
if (getLangOpts().SYCLIsDevice)
@@ -701,12 +699,10 @@ void CodeGenFunction::EmitOpenCLKernelMetadata(const FunctionDecl *FD,
701699
if (const SYCLIntelMaxWorkGroupSizeAttr *A =
702700
FD->getAttr<SYCLIntelMaxWorkGroupSizeAttr>()) {
703701
llvm::LLVMContext &Context = getLLVMContext();
704-
Optional<llvm::APSInt> XDimVal =
705-
A->getXDim()->getIntegerConstantExpr(FD->getASTContext());
706-
Optional<llvm::APSInt> YDimVal =
707-
A->getYDim()->getIntegerConstantExpr(FD->getASTContext());
708-
Optional<llvm::APSInt> ZDimVal =
709-
A->getZDim()->getIntegerConstantExpr(FD->getASTContext());
702+
ASTContext &ClangCtx = FD->getASTContext();
703+
Optional<llvm::APSInt> XDimVal = A->getXDimVal(ClangCtx);
704+
Optional<llvm::APSInt> YDimVal = A->getYDimVal(ClangCtx);
705+
Optional<llvm::APSInt> ZDimVal = A->getZDimVal(ClangCtx);
710706

711707
// For a SYCLDevice SYCLIntelMaxWorkGroupSizeAttr arguments are reversed.
712708
if (getLangOpts().SYCLIsDevice)

‎clang/lib/CodeGen/TargetInfo.cpp

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8108,15 +8108,10 @@ void TCETargetCodeGenInfo::setTargetAttributes(
81088108

81098109
SmallVector<llvm::Metadata *, 5> Operands;
81108110
Operands.push_back(llvm::ConstantAsMetadata::get(F));
8111-
unsigned XDim = Attr->getXDim()
8112-
->EvaluateKnownConstInt(M.getContext())
8113-
.getZExtValue();
8114-
unsigned YDim = Attr->getYDim()
8115-
->EvaluateKnownConstInt(M.getContext())
8116-
.getZExtValue();
8117-
unsigned ZDim = Attr->getZDim()
8118-
->EvaluateKnownConstInt(M.getContext())
8119-
.getZExtValue();
8111+
ASTContext &Ctx = M.getContext();
8112+
unsigned XDim = Attr->getXDimVal(Ctx)->getZExtValue();
8113+
unsigned YDim = Attr->getYDimVal(Ctx)->getZExtValue();
8114+
unsigned ZDim = Attr->getZDimVal(Ctx)->getZExtValue();
81208115

81218116
Operands.push_back(llvm::ConstantAsMetadata::get(
81228117
llvm::Constant::getIntegerValue(M.Int32Ty, llvm::APInt(32, XDim))));
@@ -9006,24 +9001,15 @@ void AMDGPUTargetCodeGenInfo::setTargetAttributes(
90069001
unsigned XDim = 0;
90079002
unsigned YDim = 0;
90089003
unsigned ZDim = 0;
9004+
ASTContext &Ctx = M.getContext();
90099005
if (FlatWGS) {
9010-
Min = FlatWGS->getMin()
9011-
->EvaluateKnownConstInt(M.getContext())
9012-
.getExtValue();
9013-
Max = FlatWGS->getMax()
9014-
->EvaluateKnownConstInt(M.getContext())
9015-
.getExtValue();
9006+
Min = FlatWGS->getMin()->EvaluateKnownConstInt(Ctx).getExtValue();
9007+
Max = FlatWGS->getMax()->EvaluateKnownConstInt(Ctx).getExtValue();
90169008
}
90179009
if (ReqdWGS) {
9018-
XDim = ReqdWGS->getXDim()
9019-
->EvaluateKnownConstInt(M.getContext())
9020-
.getZExtValue();
9021-
YDim = ReqdWGS->getYDim()
9022-
->EvaluateKnownConstInt(M.getContext())
9023-
.getZExtValue();
9024-
ZDim = ReqdWGS->getZDim()
9025-
->EvaluateKnownConstInt(M.getContext())
9026-
.getZExtValue();
9010+
XDim = ReqdWGS->getXDimVal(Ctx)->getZExtValue();
9011+
YDim = ReqdWGS->getYDimVal(Ctx)->getZExtValue();
9012+
ZDim = ReqdWGS->getZDimVal(Ctx)->getZExtValue();
90279013
}
90289014
if (ReqdWGS && Min == 0 && Max == 0)
90299015
Min = Max = XDim * YDim * ZDim;

‎clang/lib/Sema/SemaDecl.cpp

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3209,18 +3209,10 @@ static void checkDimensionsAndSetDiagnostics(Sema &S, FunctionDecl *New,
32093209
if (!NewDeclAttr || !OldDeclAttr)
32103210
return;
32113211

3212-
/// Returns the usigned constant integer value represented by
3213-
/// given expression.
3214-
auto getExprValue = [](const Expr *E, ASTContext &Ctx) {
3215-
return E->getIntegerConstantExpr(Ctx)->getZExtValue();
3216-
};
3217-
3218-
if ((getExprValue(NewDeclAttr->getXDim(), S.getASTContext()) !=
3219-
getExprValue(OldDeclAttr->getXDim(), S.getASTContext())) ||
3220-
(getExprValue(NewDeclAttr->getYDim(), S.getASTContext()) !=
3221-
getExprValue(OldDeclAttr->getYDim(), S.getASTContext())) ||
3222-
(getExprValue(NewDeclAttr->getZDim(), S.getASTContext()) !=
3223-
getExprValue(OldDeclAttr->getZDim(), S.getASTContext()))) {
3212+
ASTContext &Ctx = S.getASTContext();
3213+
if (NewDeclAttr->getXDimVal(Ctx) != OldDeclAttr->getXDimVal(Ctx) ||
3214+
NewDeclAttr->getYDimVal(Ctx) != OldDeclAttr->getYDimVal(Ctx) ||
3215+
NewDeclAttr->getZDimVal(Ctx) != OldDeclAttr->getZDimVal(Ctx)) {
32243216
S.Diag(New->getLocation(), diag::err_conflicting_sycl_function_attributes)
32253217
<< OldDeclAttr << NewDeclAttr;
32263218
S.Diag(New->getLocation(), diag::warn_duplicate_attribute) << OldDeclAttr;

‎clang/lib/Sema/SemaDeclAttr.cpp

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3096,23 +3096,17 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
30963096
}
30973097

30983098
if (WorkGroupAttr *ExistingAttr = D->getAttr<WorkGroupAttr>()) {
3099-
Optional<llvm::APSInt> XDimVal =
3100-
XDimExpr->getIntegerConstantExpr(S.getASTContext());
3101-
Optional<llvm::APSInt> YDimVal =
3102-
YDimExpr->getIntegerConstantExpr(S.getASTContext());
3103-
Optional<llvm::APSInt> ZDimVal =
3104-
ZDimExpr->getIntegerConstantExpr(S.getASTContext());
3105-
Optional<llvm::APSInt> ExistingXDimVal =
3106-
ExistingAttr->getXDim()->getIntegerConstantExpr(S.getASTContext());
3107-
Optional<llvm::APSInt> ExistingYDimVal =
3108-
ExistingAttr->getYDim()->getIntegerConstantExpr(S.getASTContext());
3109-
Optional<llvm::APSInt> ExistingZDimVal =
3110-
ExistingAttr->getZDim()->getIntegerConstantExpr(S.getASTContext());
3099+
ASTContext &Ctx = S.getASTContext();
3100+
Optional<llvm::APSInt> XDimVal = XDimExpr->getIntegerConstantExpr(Ctx);
3101+
Optional<llvm::APSInt> YDimVal = YDimExpr->getIntegerConstantExpr(Ctx);
3102+
Optional<llvm::APSInt> ZDimVal = ZDimExpr->getIntegerConstantExpr(Ctx);
3103+
Optional<llvm::APSInt> ExistingXDimVal = ExistingAttr->getXDimVal(Ctx);
3104+
Optional<llvm::APSInt> ExistingYDimVal = ExistingAttr->getYDimVal(Ctx);
3105+
Optional<llvm::APSInt> ExistingZDimVal = ExistingAttr->getZDimVal(Ctx);
31113106

31123107
// Compare attribute arguments value and warn for a mismatch.
3113-
if (!((ExistingXDimVal->getZExtValue() == XDimVal->getZExtValue()) &&
3114-
(ExistingYDimVal->getZExtValue() == YDimVal->getZExtValue()) &&
3115-
(ExistingZDimVal->getZExtValue() == ZDimVal->getZExtValue()))) {
3108+
if (ExistingXDimVal != XDimVal || ExistingYDimVal != YDimVal ||
3109+
ExistingZDimVal != ZDimVal) {
31163110
S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL;
31173111
S.Diag(ExistingAttr->getLocation(), diag::note_conflicting_attribute);
31183112
}

‎clang/lib/Sema/SemaSYCL.cpp

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3302,32 +3302,28 @@ void Sema::MarkDevice(void) {
33023302
break;
33033303
}
33043304
case attr::Kind::ReqdWorkGroupSize: {
3305-
auto *Attr = cast<ReqdWorkGroupSizeAttr>(A);
3305+
auto *RWGSA = cast<ReqdWorkGroupSizeAttr>(A);
33063306
if (auto *Existing = SYCLKernel->getAttr<ReqdWorkGroupSizeAttr>()) {
3307-
if ((getIntExprValue(Existing->getXDim(), getASTContext()) !=
3308-
getIntExprValue(Attr->getXDim(), getASTContext())) ||
3309-
(getIntExprValue(Existing->getYDim(), getASTContext()) !=
3310-
getIntExprValue(Attr->getYDim(), getASTContext())) ||
3311-
(getIntExprValue(Existing->getZDim(), getASTContext()) !=
3312-
getIntExprValue(Attr->getZDim(), getASTContext()))) {
3307+
ASTContext &Ctx = getASTContext();
3308+
if (Existing->getXDimVal(Ctx) != RWGSA->getXDimVal(Ctx) ||
3309+
Existing->getYDimVal(Ctx) != RWGSA->getYDimVal(Ctx) ||
3310+
Existing->getZDimVal(Ctx) != RWGSA->getZDimVal(Ctx)) {
33133311
Diag(SYCLKernel->getLocation(),
33143312
diag::err_conflicting_sycl_kernel_attributes);
33153313
Diag(Existing->getLocation(), diag::note_conflicting_attribute);
3316-
Diag(Attr->getLocation(), diag::note_conflicting_attribute);
3314+
Diag(RWGSA->getLocation(), diag::note_conflicting_attribute);
33173315
SYCLKernel->setInvalidDecl();
33183316
}
33193317
} else if (auto *Existing =
33203318
SYCLKernel->getAttr<SYCLIntelMaxWorkGroupSizeAttr>()) {
3321-
if ((getIntExprValue(Existing->getXDim(), getASTContext()) <
3322-
getIntExprValue(Attr->getXDim(), getASTContext())) ||
3323-
(getIntExprValue(Existing->getYDim(), getASTContext()) <
3324-
getIntExprValue(Attr->getYDim(), getASTContext())) ||
3325-
(getIntExprValue(Existing->getZDim(), getASTContext()) <
3326-
getIntExprValue(Attr->getZDim(), getASTContext()))) {
3319+
ASTContext &Ctx = getASTContext();
3320+
if (Existing->getXDimVal(Ctx) < RWGSA->getXDimVal(Ctx) ||
3321+
Existing->getYDimVal(Ctx) < RWGSA->getYDimVal(Ctx) ||
3322+
Existing->getZDimVal(Ctx) < RWGSA->getZDimVal(Ctx)) {
33273323
Diag(SYCLKernel->getLocation(),
33283324
diag::err_conflicting_sycl_kernel_attributes);
33293325
Diag(Existing->getLocation(), diag::note_conflicting_attribute);
3330-
Diag(Attr->getLocation(), diag::note_conflicting_attribute);
3326+
Diag(RWGSA->getLocation(), diag::note_conflicting_attribute);
33313327
SYCLKernel->setInvalidDecl();
33323328
} else {
33333329
SYCLKernel->addAttr(A);
@@ -3338,18 +3334,16 @@ void Sema::MarkDevice(void) {
33383334
break;
33393335
}
33403336
case attr::Kind::SYCLIntelMaxWorkGroupSize: {
3341-
auto *Attr = cast<SYCLIntelMaxWorkGroupSizeAttr>(A);
3337+
auto *SIMWGSA = cast<SYCLIntelMaxWorkGroupSizeAttr>(A);
33423338
if (auto *Existing = SYCLKernel->getAttr<ReqdWorkGroupSizeAttr>()) {
3343-
if ((getIntExprValue(Existing->getXDim(), getASTContext()) >
3344-
getIntExprValue(Attr->getXDim(), getASTContext())) ||
3345-
(getIntExprValue(Existing->getYDim(), getASTContext()) >
3346-
getIntExprValue(Attr->getYDim(), getASTContext())) ||
3347-
(getIntExprValue(Existing->getZDim(), getASTContext()) >
3348-
getIntExprValue(Attr->getZDim(), getASTContext()))) {
3339+
ASTContext &Ctx = getASTContext();
3340+
if (Existing->getXDimVal(Ctx) > SIMWGSA->getXDimVal(Ctx) ||
3341+
Existing->getYDimVal(Ctx) > SIMWGSA->getYDimVal(Ctx) ||
3342+
Existing->getZDimVal(Ctx) > SIMWGSA->getZDimVal(Ctx)) {
33493343
Diag(SYCLKernel->getLocation(),
33503344
diag::err_conflicting_sycl_kernel_attributes);
33513345
Diag(Existing->getLocation(), diag::note_conflicting_attribute);
3352-
Diag(Attr->getLocation(), diag::note_conflicting_attribute);
3346+
Diag(SIMWGSA->getLocation(), diag::note_conflicting_attribute);
33533347
SYCLKernel->setInvalidDecl();
33543348
} else {
33553349
SYCLKernel->addAttr(A);

0 commit comments

Comments
 (0)