Skip to content

Commit c5566b2

Browse files
elizabethandrewselizabethandrews
authored
[SYCL] Store expression as ConstantExpr in Semantic Attribute (#3169)
Currently , the behavior is inconsistent for FPGA attributes. We evaluate and store the ConstantExpr for some SYCL/FPGA attributes , and discard it for others. This patch is part 1 of making the behavior consistent. This patch saves the ConstantExpr for the following attributes. The ConstantExpr is then extracted from the Sematic attributes and used in CodeGen instead of reevaluating it. SYCLIntelNoGlobalWorkOffsetAttr, SYCLIntelMaxGlobalWorkDimAttr, SYCLIntelNumSimdWorkItemsAttr , IntelReqdSubGroupSizeAttr , SYCLIntelSchedulerTargetFmaxMhzAttr , ReqdWorkGroupSizeAttr, IntelReqdSubGroupSizeAttr, SYCLIntelMaxWorkGroupSizeAttr A consequence of this change is a change in diagnostic when the attribute argument is not an integer constant expression. The new diagnostic is an existing llvm-project diagnostic. A follow-up patch(s) will address the remaining attributes. Signed-off-by: Elizabeth Andrews [email protected]
1 parent 7b96311 commit c5566b2

18 files changed

+331
-125
lines changed

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

Lines changed: 34 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -13065,14 +13065,12 @@ void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D,
1306513065
assert(E && "Attribute must have an argument.");
1306613066

1306713067
if (!E->isInstantiationDependent()) {
13068-
Optional<llvm::APSInt> ArgVal = E->getIntegerConstantExpr(getASTContext());
13069-
if (!ArgVal) {
13070-
Diag(E->getExprLoc(), diag::err_attribute_argument_type)
13071-
<< CI.getAttrName() << AANT_ArgumentIntegerConstant
13072-
<< E->getSourceRange();
13068+
llvm::APSInt ArgVal;
13069+
ExprResult ICE = VerifyIntegerConstantExpression(E, &ArgVal);
13070+
if (ICE.isInvalid())
1307313071
return;
13074-
}
13075-
int32_t ArgInt = ArgVal->getSExtValue();
13072+
E = ICE.get();
13073+
int32_t ArgInt = ArgVal.getSExtValue();
1307613074
if (CI.getParsedKind() == ParsedAttr::AT_SYCLIntelNumSimdWorkItems ||
1307713075
CI.getParsedKind() == ParsedAttr::AT_IntelReqdSubGroupSize) {
1307813076
if (ArgInt <= 0) {
@@ -13098,77 +13096,59 @@ void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D,
1309813096
D->addAttr(::new (Context) AttrType(Context, CI, E));
1309913097
}
1310013098

13101-
template <typename AttrInfo>
13102-
static bool handleMaxWorkSizeAttrExpr(Sema &S, const AttrInfo &AI,
13103-
const Expr *E, unsigned &Val,
13104-
unsigned Idx) {
13099+
static Expr *checkMaxWorkSizeAttrExpr(Sema &S, const AttributeCommonInfo &CI,
13100+
Expr *E) {
1310513101
assert(E && "Attribute must have an argument.");
1310613102

1310713103
if (!E->isInstantiationDependent()) {
13108-
Optional<llvm::APSInt> ArgVal =
13109-
E->getIntegerConstantExpr(S.getASTContext());
13104+
llvm::APSInt ArgVal;
13105+
ExprResult ICE = S.VerifyIntegerConstantExpression(E, &ArgVal);
1311013106

13111-
if (!ArgVal) {
13112-
S.Diag(AI.getLocation(), diag::err_attribute_argument_type)
13113-
<< &AI << AANT_ArgumentIntegerConstant << E->getSourceRange();
13114-
return false;
13115-
}
13107+
if (ICE.isInvalid())
13108+
return nullptr;
13109+
13110+
E = ICE.get();
1311613111

13117-
if (ArgVal->isNegative()) {
13112+
if (ArgVal.isNegative()) {
1311813113
S.Diag(E->getExprLoc(),
1311913114
diag::warn_attribute_requires_non_negative_integer_argument)
1312013115
<< E->getType() << S.Context.UnsignedLongLongTy
1312113116
<< E->getSourceRange();
13122-
return true;
13117+
return E;
1312313118
}
1312413119

13125-
Val = ArgVal->getZExtValue();
13120+
unsigned Val = ArgVal.getZExtValue();
1312613121
if (Val == 0) {
1312713122
S.Diag(E->getExprLoc(), diag::err_attribute_argument_is_zero)
13128-
<< &AI << E->getSourceRange();
13129-
return false;
13123+
<< CI << E->getSourceRange();
13124+
return nullptr;
1313013125
}
1313113126
}
13132-
return true;
13133-
}
13134-
13135-
template <typename AttrType>
13136-
static bool checkMaxWorkSizeAttrArguments(Sema &S, Expr *XDimExpr,
13137-
Expr *YDimExpr, Expr *ZDimExpr,
13138-
const AttrType &Attr) {
13139-
// Accept template arguments for now as they depend on something else.
13140-
// We'll get to check them when they eventually get instantiated.
13141-
if (XDimExpr->isValueDependent() ||
13142-
(YDimExpr && YDimExpr->isValueDependent()) ||
13143-
(ZDimExpr && ZDimExpr->isValueDependent()))
13144-
return false;
13145-
13146-
unsigned XDim = 0;
13147-
if (!handleMaxWorkSizeAttrExpr(S, Attr, XDimExpr, XDim, 0))
13148-
return true;
13149-
13150-
unsigned YDim = 0;
13151-
if (YDimExpr && !handleMaxWorkSizeAttrExpr(S, Attr, YDimExpr, YDim, 1))
13152-
return true;
13153-
13154-
unsigned ZDim = 0;
13155-
if (ZDimExpr && !handleMaxWorkSizeAttrExpr(S, Attr, ZDimExpr, ZDim, 2))
13156-
return true;
13157-
13158-
return false;
13127+
return E;
1315913128
}
1316013129

1316113130
template <typename WorkGroupAttrType>
1316213131
void Sema::addIntelSYCLTripleArgFunctionAttr(Decl *D,
1316313132
const AttributeCommonInfo &CI,
1316413133
Expr *XDimExpr, Expr *YDimExpr,
1316513134
Expr *ZDimExpr) {
13166-
WorkGroupAttrType TmpAttr(Context, CI, XDimExpr, YDimExpr, ZDimExpr);
1316713135

13168-
if (checkMaxWorkSizeAttrArguments(*this, XDimExpr, YDimExpr, ZDimExpr,
13169-
TmpAttr))
13170-
return;
13136+
assert((XDimExpr && YDimExpr && ZDimExpr) &&
13137+
"argument has unexpected null value");
13138+
13139+
// Accept template arguments for now as they depend on something else.
13140+
// We'll get to check them when they eventually get instantiated.
13141+
if (!XDimExpr->isValueDependent() && !YDimExpr->isValueDependent() &&
13142+
!ZDimExpr->isValueDependent()) {
13143+
13144+
// Save ConstantExpr in semantic attribute
13145+
XDimExpr = checkMaxWorkSizeAttrExpr(*this, CI, XDimExpr);
13146+
YDimExpr = checkMaxWorkSizeAttrExpr(*this, CI, YDimExpr);
13147+
ZDimExpr = checkMaxWorkSizeAttrExpr(*this, CI, ZDimExpr);
1317113148

13149+
if (!XDimExpr || !YDimExpr || !ZDimExpr)
13150+
return;
13151+
}
1317213152
D->addAttr(::new (Context)
1317313153
WorkGroupAttrType(Context, CI, XDimExpr, YDimExpr, ZDimExpr));
1317413154
}

‎clang/lib/CodeGen/CodeGenFunction.cpp

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,6 @@ void CodeGenFunction::EmitOpenCLKernelMetadata(const FunctionDecl *FD,
620620
}
621621

622622
if (const ReqdWorkGroupSizeAttr *A = FD->getAttr<ReqdWorkGroupSizeAttr>()) {
623-
llvm::LLVMContext &Context = getLLVMContext();
624623
ASTContext &ClangCtx = FD->getASTContext();
625624
Optional<llvm::APSInt> XDimVal = A->getXDimVal(ClangCtx);
626625
Optional<llvm::APSInt> YDimVal = A->getYDimVal(ClangCtx);
@@ -643,10 +642,9 @@ void CodeGenFunction::EmitOpenCLKernelMetadata(const FunctionDecl *FD,
643642

644643
if (const IntelReqdSubGroupSizeAttr *A =
645644
FD->getAttr<IntelReqdSubGroupSizeAttr>()) {
646-
llvm::LLVMContext &Context = getLLVMContext();
647-
Optional<llvm::APSInt> ArgVal =
648-
A->getValue()->getIntegerConstantExpr(FD->getASTContext());
649-
assert(ArgVal.hasValue() && "Not an integer constant expression");
645+
const auto *CE = dyn_cast<ConstantExpr>(A->getValue());
646+
assert(CE && "Not an integer constant expression");
647+
Optional<llvm::APSInt> ArgVal = CE->getResultAsAPSInt();
650648
llvm::Metadata *AttrMDArgs[] = {llvm::ConstantAsMetadata::get(
651649
Builder.getInt32(ArgVal->getSExtValue()))};
652650
Fn->setMetadata("intel_reqd_sub_group_size",
@@ -663,10 +661,9 @@ void CodeGenFunction::EmitOpenCLKernelMetadata(const FunctionDecl *FD,
663661

664662
if (const SYCLIntelNumSimdWorkItemsAttr *A =
665663
FD->getAttr<SYCLIntelNumSimdWorkItemsAttr>()) {
666-
llvm::LLVMContext &Context = getLLVMContext();
667-
Optional<llvm::APSInt> ArgVal =
668-
A->getValue()->getIntegerConstantExpr(FD->getASTContext());
669-
assert(ArgVal.hasValue() && "Not an integer constant expression");
664+
const auto *CE = dyn_cast<ConstantExpr>(A->getValue());
665+
assert(CE && "Not an integer constant expression");
666+
Optional<llvm::APSInt> ArgVal = CE->getResultAsAPSInt();
670667
llvm::Metadata *AttrMDArgs[] = {llvm::ConstantAsMetadata::get(
671668
Builder.getInt32(ArgVal->getSExtValue()))};
672669
Fn->setMetadata("num_simd_work_items",
@@ -675,9 +672,9 @@ void CodeGenFunction::EmitOpenCLKernelMetadata(const FunctionDecl *FD,
675672

676673
if (const SYCLIntelSchedulerTargetFmaxMhzAttr *A =
677674
FD->getAttr<SYCLIntelSchedulerTargetFmaxMhzAttr>()) {
678-
Optional<llvm::APSInt> ArgVal =
679-
A->getValue()->getIntegerConstantExpr(FD->getASTContext());
680-
assert(ArgVal.hasValue() && "Not an integer constant expression");
675+
const auto *CE = dyn_cast<ConstantExpr>(A->getValue());
676+
assert(CE && "Not an integer constant expression");
677+
Optional<llvm::APSInt> ArgVal = CE->getResultAsAPSInt();
681678
llvm::Metadata *AttrMDArgs[] = {llvm::ConstantAsMetadata::get(
682679
Builder.getInt32(ArgVal->getSExtValue()))};
683680
Fn->setMetadata("scheduler_target_fmax_mhz",
@@ -686,10 +683,9 @@ void CodeGenFunction::EmitOpenCLKernelMetadata(const FunctionDecl *FD,
686683

687684
if (const SYCLIntelMaxGlobalWorkDimAttr *A =
688685
FD->getAttr<SYCLIntelMaxGlobalWorkDimAttr>()) {
689-
llvm::LLVMContext &Context = getLLVMContext();
690-
Optional<llvm::APSInt> ArgVal =
691-
A->getValue()->getIntegerConstantExpr(FD->getASTContext());
692-
assert(ArgVal.hasValue() && "Not an integer constant expression");
686+
const auto *CE = dyn_cast<ConstantExpr>(A->getValue());
687+
assert(CE && "Not an integer constant expression");
688+
Optional<llvm::APSInt> ArgVal = CE->getResultAsAPSInt();
693689
llvm::Metadata *AttrMDArgs[] = {llvm::ConstantAsMetadata::get(
694690
Builder.getInt32(ArgVal->getSExtValue()))};
695691
Fn->setMetadata("max_global_work_dim",
@@ -698,7 +694,6 @@ void CodeGenFunction::EmitOpenCLKernelMetadata(const FunctionDecl *FD,
698694

699695
if (const SYCLIntelMaxWorkGroupSizeAttr *A =
700696
FD->getAttr<SYCLIntelMaxWorkGroupSizeAttr>()) {
701-
llvm::LLVMContext &Context = getLLVMContext();
702697
ASTContext &ClangCtx = FD->getASTContext();
703698
Optional<llvm::APSInt> XDimVal = A->getXDimVal(ClangCtx);
704699
Optional<llvm::APSInt> YDimVal = A->getYDimVal(ClangCtx);
@@ -723,9 +718,9 @@ void CodeGenFunction::EmitOpenCLKernelMetadata(const FunctionDecl *FD,
723718
FD->getAttr<SYCLIntelNoGlobalWorkOffsetAttr>()) {
724719
const Expr *Arg = A->getValue();
725720
assert(Arg && "Got an unexpected null argument");
726-
Optional<llvm::APSInt> ArgVal =
727-
Arg->getIntegerConstantExpr(FD->getASTContext());
728-
assert(ArgVal.hasValue() && "Not an integer constant expression");
721+
const auto *CE = dyn_cast<ConstantExpr>(Arg);
722+
assert(CE && "Not an integer constant expression");
723+
Optional<llvm::APSInt> ArgVal = CE->getResultAsAPSInt();
729724
if (ArgVal->getBoolValue())
730725
Fn->setMetadata("no_global_work_offset", llvm::MDNode::get(Context, {}));
731726
}

‎clang/test/SemaSYCL/intel-fpga-no-global-work-offset.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,34 @@ struct FuncObj {
1414
int main() {
1515
q.submit([&](handler &h) {
1616
// CHECK: SYCLIntelNoGlobalWorkOffsetAttr {{.*}}
17+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
18+
// CHECK-NEXT: value: Int 1
1719
// CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
1820
h.single_task<class test_kernel1>(FuncObj());
1921

2022
// CHECK: SYCLIntelNoGlobalWorkOffsetAttr {{.*}}
23+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
24+
// CHECK-NEXT: value: Int 0
2125
// CHECK-NEXT: IntegerLiteral{{.*}}0{{$}}
2226
h.single_task<class test_kernel2>(
2327
[]() [[intel::no_global_work_offset(0)]]{});
2428

2529
// CHECK: SYCLIntelNoGlobalWorkOffsetAttr{{.*}}
30+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
31+
// CHECK-NEXT: value: Int 42
2632
// CHECK-NEXT: IntegerLiteral{{.*}}42{{$}}
2733
h.single_task<class test_kernel3>(
2834
[]() [[intel::no_global_work_offset(42)]]{});
2935

3036
// CHECK: SYCLIntelNoGlobalWorkOffsetAttr{{.*}}
37+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
38+
// CHECK-NEXT: value: Int -1
3139
// CHECK-NEXT: UnaryOperator{{.*}} 'int' prefix '-'
3240
// CHECK-NEXT-NEXT: IntegerLiteral{{.*}}1{{$}}
3341
h.single_task<class test_kernel4>(
3442
[]() [[intel::no_global_work_offset(-1)]]{});
3543

36-
// expected-error@+2{{'no_global_work_offset' attribute requires an integer constant}}
44+
// expected-error@+2{{integral constant expression must have integral or unscoped enumeration type, not 'const char [4]'}}
3745
h.single_task<class test_kernel5>(
3846
[]() [[intel::no_global_work_offset("foo")]]{});
3947

‎clang/test/SemaSYCL/intel-max-global-work-dim-device.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,15 @@ int main() {
6161
q.submit([&](handler &h) {
6262
// CHECK-LABEL: FunctionDecl {{.*}}test_kernel1
6363
// CHECK: SYCLIntelMaxGlobalWorkDimAttr {{.*}}
64+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
65+
// CHECK-NEXT: value: Int 1
6466
// CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
6567
h.single_task<class test_kernel1>(FuncObj());
6668

6769
// CHECK-LABEL: FunctionDecl {{.*}}test_kernel2
6870
// CHECK: SYCLIntelMaxGlobalWorkDimAttr {{.*}}
71+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
72+
// CHECK-NEXT: value: Int 2
6973
// CHECK-NEXT: IntegerLiteral{{.*}}2{{$}}
7074
// expected-warning@+3 {{attribute 'intelfpga::max_global_work_dim' is deprecated}}
7175
// expected-note@+2 {{did you mean to use 'intel::max_global_work_dim' instead?}}
@@ -74,34 +78,64 @@ int main() {
7478

7579
// CHECK-LABEL: FunctionDecl {{.*}}test_kernel3
7680
// CHECK: SYCLIntelMaxGlobalWorkDimAttr {{.*}}
81+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
82+
// CHECK-NEXT: value: Int 2
7783
// CHECK-NEXT: IntegerLiteral{{.*}}2{{$}}
7884
h.single_task<class test_kernel3>(
7985
[]() { func_do_not_ignore(); });
8086

8187
h.single_task<class test_kernel4>(TRIFuncObjGood1());
8288
// CHECK-LABEL: FunctionDecl {{.*}}test_kernel4
8389
// CHECK: ReqdWorkGroupSizeAttr {{.*}}
90+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
91+
// CHECK-NEXT: value: Int 1
8492
// CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
93+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
94+
// CHECK-NEXT: value: Int 1
8595
// CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
96+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
97+
// CHECK-NEXT: value: Int 1
8698
// CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
8799
// CHECK: SYCLIntelMaxWorkGroupSizeAttr {{.*}}
100+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
101+
// CHECK-NEXT: value: Int 1
88102
// CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
103+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
104+
// CHECK-NEXT: value: Int 1
89105
// CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
106+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
107+
// CHECK-NEXT: value: Int 1
90108
// CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
91109
// CHECK: SYCLIntelMaxGlobalWorkDimAttr {{.*}}
110+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
111+
// CHECK-NEXT: value: Int 0
92112
// CHECK-NEXT: IntegerLiteral{{.*}}0{{$}}
93113

94114
h.single_task<class test_kernel5>(TRIFuncObjGood2());
95115
// CHECK-LABEL: FunctionDecl {{.*}}test_kernel5
96116
// CHECK: ReqdWorkGroupSizeAttr {{.*}}
117+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
118+
// CHECK-NEXT: value: Int 4
97119
// CHECK-NEXT: IntegerLiteral{{.*}}4{{$}}
120+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
121+
// CHECK-NEXT: value: Int 1
98122
// CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
123+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
124+
// CHECK-NEXT: value: Int 1
99125
// CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
100126
// CHECK: SYCLIntelMaxWorkGroupSizeAttr {{.*}}
127+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
128+
// CHECK-NEXT: value: Int 8
101129
// CHECK-NEXT: IntegerLiteral{{.*}}8{{$}}
130+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
131+
// CHECK-NEXT: value: Int 1
102132
// CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
133+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
134+
// CHECK-NEXT: value: Int 1
103135
// CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
104136
// CHECK: SYCLIntelMaxGlobalWorkDimAttr {{.*}}
137+
// CHECK-NEXT: ConstantExpr {{.*}} 'int'
138+
// CHECK-NEXT: value: Int 3
105139
// CHECK-NEXT: IntegerLiteral{{.*}}3{{$}}
106140
#ifdef TRIGGER_ERROR
107141
[[intel::max_global_work_dim(1)]] int Var = 0; // expected-error{{'max_global_work_dim' attribute only applies to functions}}

0 commit comments

Comments
 (0)