Skip to content

Commit 84d2eb5

Browse files
smanna12smanna12
authored
[SYCL][NFC] Diagnostic improvement for work_group_size attributes (#2988)
Template parameter support for work_group_size attributes was added on #2906 This is a followup comments on #2906 This patch improves the diagnostic message for "warn_attribute_requires_non_negative_integer_argument" and updates tests related to this diagnostic. Signed-off-by: Soumi Manna <soumi.manna@intel.com>
1 parent 237675b commit 84d2eb5

File tree

5 files changed

+51
-26
lines changed

5 files changed

+51
-26
lines changed

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11207,10 +11207,8 @@ def warn_attribute_spelling_deprecated : Warning<
1120711207
InGroup<DeprecatedAttributes>;
1120811208
def note_spelling_suggestion : Note<
1120911209
"did you mean to use %0 instead?">;
11210-
def warn_attribute_requires_non_negative_integer_argument : Warning<
11211-
"the resulting value of the %0 attribute %select{first|second|third}1"
11212-
" parameter is always non-negative after implicit conversion">,
11213-
InGroup<AcceptedAttributes>;
11210+
def warn_attribute_requires_non_negative_integer_argument :
11211+
Warning<warn_impcast_integer_sign.Text>, InGroup<AcceptedAttributes>;
1121411212

1121511213
// errors of expect.with.probability
1121611214
def err_probability_not_constant_float : Error<

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12964,31 +12964,32 @@ void Sema::addIntelSYCLSingleArgFunctionAttr(Decl *D,
1296412964

1296512965
template <typename AttrInfo>
1296612966
static bool handleMaxWorkSizeAttrExpr(Sema &S, const AttrInfo &AI,
12967-
const Expr *Expr, unsigned &Val,
12967+
const Expr *E, unsigned &Val,
1296812968
unsigned Idx) {
12969-
assert(Expr && "Attribute must have an argument.");
12969+
assert(E && "Attribute must have an argument.");
1297012970

12971-
if (!Expr->isInstantiationDependent()) {
12971+
if (!E->isInstantiationDependent()) {
1297212972
Optional<llvm::APSInt> ArgVal =
12973-
Expr->getIntegerConstantExpr(S.getASTContext());
12973+
E->getIntegerConstantExpr(S.getASTContext());
1297412974

1297512975
if (!ArgVal) {
1297612976
S.Diag(AI.getLocation(), diag::err_attribute_argument_type)
12977-
<< &AI << AANT_ArgumentIntegerConstant << Expr->getSourceRange();
12977+
<< &AI << AANT_ArgumentIntegerConstant << E->getSourceRange();
1297812978
return false;
1297912979
}
1298012980

1298112981
if (ArgVal->isNegative()) {
12982-
S.Diag(Expr->getExprLoc(),
12982+
S.Diag(E->getExprLoc(),
1298312983
diag::warn_attribute_requires_non_negative_integer_argument)
12984-
<< &AI << Idx << Expr->getSourceRange();
12984+
<< E->getType() << S.Context.UnsignedLongLongTy
12985+
<< E->getSourceRange();
1298512986
return true;
1298612987
}
1298712988

1298812989
Val = ArgVal->getZExtValue();
1298912990
if (Val == 0) {
12990-
S.Diag(Expr->getExprLoc(), diag::err_attribute_argument_is_zero)
12991-
<< &AI << Expr->getSourceRange();
12991+
S.Diag(E->getExprLoc(), diag::err_attribute_argument_is_zero)
12992+
<< &AI << E->getSourceRange();
1299212993
return false;
1299312994
}
1299412995
}

‎clang/test/SemaOpenCL/invalid-kernel-attrs.cl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ kernel __attribute__((intel_reqd_sub_group_size(-1))) void kernel16() {} // expe
4242
kernel __attribute__((intel_reqd_sub_group_size(8))) __attribute__((intel_reqd_sub_group_size(16))) void kernel17() {} //expected-warning{{attribute 'intel_reqd_sub_group_size' is already applied with different parameters}}
4343

4444
__kernel __attribute__((work_group_size_hint(8,-16,32))) void neg1() {} //expected-error{{'work_group_size_hint' attribute requires a non-negative integral compile time constant expression}}
45-
__kernel __attribute__((reqd_work_group_size(8, 16, -32))) void neg2() {} //expected-warning{{the resulting value of the 'reqd_work_group_size' attribute third parameter is always non-negative after implicit conversion}}
45+
__kernel __attribute__((reqd_work_group_size(8, 16, -32))) void neg2() {} //expected-warning{{implicit conversion changes signedness: 'int' to 'unsigned long long'}}
4646

4747
// 4294967294 is a negative integer if treated as signed.
4848
// Should compile successfully, since we expect an unsigned.

‎clang/test/SemaSYCL/intel-max-work-group-size-device.cpp

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,24 +75,34 @@ int main() {
7575
// CHECK-NEXT: IntegerLiteral{{.*}}8{{$}}
7676
// CHECK-NEXT: UnaryOperator{{.*}} 'int' prefix '-'
7777
// CHECK-NEXT: IntegerLiteral{{.*}}8{{$}}
78-
// expected-warning@+2{{the resulting value of the 'max_work_group_size' attribute third parameter is always non-negative after implicit conversion}}
78+
// expected-warning@+2{{implicit conversion changes signedness: 'int' to 'unsigned long long'}}
7979
h.single_task<class test_kernel4>(
8080
[]() [[intel::max_work_group_size(8, 8, -8)]]{});
8181

82+
// CHECK-LABEL: FunctionDecl {{.*}}test_kernel5
83+
// CHECK: SYCLIntelMaxWorkGroupSizeAttr {{.*}}
84+
// CHECK-NEXT: UnaryOperator{{.*}} 'int' prefix '-'
85+
// CHECK-NEXT: IntegerLiteral{{.*}}8{{$}}
86+
// CHECK-NEXT: IntegerLiteral{{.*}}8{{$}}
87+
// CHECK-NEXT: UnaryOperator{{.*}} 'int' prefix '-'
88+
// CHECK-NEXT: IntegerLiteral{{.*}}8{{$}}
89+
// expected-warning@+2 2{{implicit conversion changes signedness: 'int' to 'unsigned long long'}}
90+
h.single_task<class test_kernel5>(
91+
[]() [[intel::max_work_group_size(-8, 8, -8)]]{});
8292
#ifdef TRIGGER_ERROR
8393
[[intel::max_work_group_size(1, 1, 1)]] int Var = 0; // expected-error{{'max_work_group_size' attribute only applies to functions}}
8494

85-
h.single_task<class test_kernel5>(
95+
h.single_task<class test_kernel6>(
8696
[]() [[intel::max_work_group_size(0, 1, 3)]]{}); // expected-error{{'max_work_group_size' attribute must be greater than 0}}
8797

88-
h.single_task<class test_kernel6>(
98+
h.single_task<class test_kernel7>(
8999
[]() [[intel::max_work_group_size(1.2f, 1, 3)]]{}); // expected-error{{'max_work_group_size' attribute requires an integer constant}}
90100

91-
h.single_task<class test_kernel7>(
101+
h.single_task<class test_kernel8>(
92102
[]() [[intel::max_work_group_size(16, 16, 16), // expected-note{{conflicting attribute is here}}
93103
intel::max_work_group_size(2, 2, 2)]]{}); // expected-warning{{attribute 'max_work_group_size' is already applied with different parameters}}
94104

95-
h.single_task<class test_kernel8>(
105+
h.single_task<class test_kernel9>(
96106
DAFuncObj());
97107

98108
#endif // TRIGGER_ERROR

‎clang/test/SemaSYCL/intel-reqd-work-group-size-device.cpp

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,16 @@ class Functor32 {
4040

4141
class Functor33 {
4242
public:
43-
// expected-warning@+1{{the resulting value of the 'reqd_work_group_size' attribute second parameter is always non-negative after implicit conversion}}
43+
// expected-warning@+1{{implicit conversion changes signedness: 'int' to 'unsigned long long'}}
4444
[[intel::reqd_work_group_size(32, -4)]] void operator()() const {}
4545
};
4646

47+
class Functor30 {
48+
public:
49+
// expected-warning@+1 2{{implicit conversion changes signedness: 'int' to 'unsigned long long'}}
50+
[[intel::reqd_work_group_size(30, -30, -30)]] void operator()() const {}
51+
};
52+
4753
class Functor16 {
4854
public:
4955
[[intel::reqd_work_group_size(16)]] void operator()() const {}
@@ -95,30 +101,33 @@ int main() {
95101
Functor33 f33;
96102
h.single_task<class kernel_name5>(f33);
97103

98-
h.single_task<class kernel_name6>([]() [[intel::reqd_work_group_size(32, 32, 32)]] {
104+
Functor30 f30;
105+
h.single_task<class kernel_name6>(f30);
106+
107+
h.single_task<class kernel_name7>([]() [[intel::reqd_work_group_size(32, 32, 32)]] {
99108
f32x32x32();
100109
});
101110
#ifdef TRIGGER_ERROR
102111
Functor8 f8;
103-
h.single_task<class kernel_name7>(f8);
112+
h.single_task<class kernel_name8>(f8);
104113

105-
h.single_task<class kernel_name8>([]() { // expected-error {{conflicting attributes applied to a SYCL kernel}}
114+
h.single_task<class kernel_name9>([]() { // expected-error {{conflicting attributes applied to a SYCL kernel}}
106115
f4x1x1();
107116
f32x1x1();
108117
});
109118

110-
h.single_task<class kernel_name9>([]() { // expected-error {{conflicting attributes applied to a SYCL kernel}}
119+
h.single_task<class kernel_name10>([]() { // expected-error {{conflicting attributes applied to a SYCL kernel}}
111120
f16x1x1();
112121
f16x16x1();
113122
});
114123

115-
h.single_task<class kernel_name10>([]() { // expected-error {{conflicting attributes applied to a SYCL kernel}}
124+
h.single_task<class kernel_name11>([]() { // expected-error {{conflicting attributes applied to a SYCL kernel}}
116125
f32x32x32();
117126
f32x32x1();
118127
});
119128

120129
// expected-error@+1 {{expected variable name or 'this' in lambda capture list}}
121-
h.single_task<class kernel_name11>([[intel::reqd_work_group_size(32, 32, 32)]][]() {
130+
h.single_task<class kernel_name12>([[intel::reqd_work_group_size(32, 32, 32)]][]() {
122131
f32x32x32();
123132
});
124133

@@ -155,6 +164,13 @@ int main() {
155164
// CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
156165
// CHECK: FunctionDecl {{.*}} {{.*}}kernel_name6
157166
// CHECK: ReqdWorkGroupSizeAttr {{.*}}
167+
// CHECK-NEXT: IntegerLiteral{{.*}}30{{$}}
168+
// CHECK-NEXT: UnaryOperator{{.*}} 'int' prefix '-'
169+
// CHECK-NEXT: IntegerLiteral{{.*}}30{{$}}
170+
// CHECK-NEXT: UnaryOperator{{.*}} 'int' prefix '-'
171+
// CHECK-NEXT: IntegerLiteral{{.*}}30{{$}}
172+
// CHECK: FunctionDecl {{.*}} {{.*}}kernel_name7
173+
// CHECK: ReqdWorkGroupSizeAttr {{.*}}
158174
// CHECK-NEXT: IntegerLiteral{{.*}}32{{$}}
159175
// CHECK-NEXT: IntegerLiteral{{.*}}32{{$}}
160176
// CHECK-NEXT: IntegerLiteral{{.*}}32{{$}}

0 commit comments

Comments
 (0)