Skip to content

Commit 862e3f2

Browse files
committed
Address review comments
Signed-off-by: Soumi Manna <[email protected]>
1 parent 4088a60 commit 862e3f2

File tree

3 files changed

+21
-33
lines changed

3 files changed

+21
-33
lines changed

clang/lib/Sema/SemaDeclAttr.cpp

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3467,7 +3467,7 @@ static AttrArgResult AreAllAttrArgsOne(const Expr *E, const Expr *E1,
34673467
if (!CE || !CE1 || !CE2 || !CE3)
34683468
return AttrArgResult::Unknown;
34693469

3470-
// Otherwise, test that the attribute values.
3470+
// Otherwise, check if the attribute values are equal to one.
34713471
if (CE->getResultAsAPSInt() == 0 &&
34723472
(CE1->getResultAsAPSInt() != 1 || CE2->getResultAsAPSInt() != 1 ||
34733473
CE3->getResultAsAPSInt() != 1)) {
@@ -3520,10 +3520,7 @@ void Sema::AddSYCLIntelMaxWorkGroupSizeAttr(Decl *D,
35203520
// the attribute holds equal values to (1, 1, 1) in case the value of
35213521
// SYCLIntelMaxGlobalWorkDimAttr equals to 0.
35223522
if (const auto *DeclAttr = D->getAttr<SYCLIntelMaxGlobalWorkDimAttr>()) {
3523-
AttrArgResult Results[] = {
3524-
AreAllAttrArgsOne(DeclAttr->getValue(), XDim, YDim, ZDim)};
3525-
3526-
if (llvm::is_contained(Results, AttrArgResult::NotEqualToOne)) {
3523+
if (AreAllAttrArgsOne(DeclAttr->getValue(), XDim, YDim, ZDim) == AttrArgResult::NotEqualToOne) {
35273524
Diag(CI.getLoc(), diag::err_sycl_x_y_z_arguments_must_be_one)
35283525
<< CI << DeclAttr;
35293526
return;
@@ -3584,10 +3581,7 @@ SYCLIntelMaxWorkGroupSizeAttr *Sema::MergeSYCLIntelMaxWorkGroupSizeAttr(
35843581
// (1, 1, 1) in case the value of SYCLIntelMaxGlobalWorkDimAttr
35853582
// equals to 0.
35863583
if (const auto *DeclAttr = D->getAttr<SYCLIntelMaxGlobalWorkDimAttr>()) {
3587-
AttrArgResult Results[] = {AreAllAttrArgsOne(
3588-
DeclAttr->getValue(), A.getXDim(), A.getYDim(), A.getZDim())};
3589-
3590-
if (llvm::is_contained(Results, AttrArgResult::NotEqualToOne)) {
3584+
if (AreAllAttrArgsOne(DeclAttr->getValue(), A.getXDim(), A.getYDim(), A.getZDim()) == AttrArgResult::NotEqualToOne) {
35913585
Diag(A.getLoc(), diag::err_sycl_x_y_z_arguments_must_be_one)
35923586
<< &A << DeclAttr;
35933587
return nullptr;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ struct TRIFuncObjGood9 {
111111
[[intel::max_global_work_dim(1)]] void TRIFuncObjGood9::operator()() const {}
112112

113113
// FIXME: We do not have support yet for checking
114-
// max_work_group_size and max_global_work_dim
114+
// reqd_work_group_size and max_global_work_dim
115115
// attributes when merging, so the test compiles without
116116
// any diagnostic when it shouldn't.
117117
struct TRIFuncObjBad1 {
@@ -123,7 +123,7 @@ struct TRIFuncObjBad1 {
123123
void TRIFuncObjBad1::operator()() const {}
124124

125125
// FIXME: We do not have support yet for checking
126-
// max_work_group_size and max_global_work_dim
126+
// reqd_work_group_size and max_global_work_dim
127127
// attributes when merging, so the test compiles without
128128
// any diagnostic when it shouldn't.
129129
struct TRIFuncObjBad2 {

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

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,13 @@
1515
[[intel::max_work_group_size(32, 32, 32)]] void f3(); // OK
1616

1717
// Produce a conflicting attribute warning when the args are different.
18-
[[intel::max_work_group_size(16, 16, 16)]] void f4(); // expected-note {{previous attribute is here}}
19-
[[intel::max_work_group_size(32, 32, 32)]] void f4() {} // expected-warning {{attribute 'max_work_group_size' is already applied with different arguments}} \
20-
2118
[[intel::max_work_group_size(6, 6, 6)]] // expected-note {{previous attribute is here}}
2219
[[intel::max_work_group_size(16, 16, 16)]] void // expected-warning {{attribute 'max_work_group_size' is already applied with different arguments}}
23-
f5() {}
24-
25-
[[intel::max_work_group_size(2, 2, 2)]] void f6(); // expected-note {{previous attribute is here}}
26-
[[intel::max_work_group_size(32, 32, 32)]] void f6(); // expected-warning {{attribute 'max_work_group_size' is already applied with different arguments}} \
20+
f4() {}
2721

2822
// Catch the easy case where the attributes are all specified at once with
2923
// different arguments.
30-
[[intel::max_work_group_size(16, 16, 16), intel::max_work_group_size(2, 2, 2)]] void f7(); // expected-warning {{attribute 'max_work_group_size' is already applied with different arguments}} expected-note {{previous attribute is here}}
24+
[[intel::max_work_group_size(16, 16, 16), intel::max_work_group_size(2, 2, 2)]] void f5(); // expected-warning {{attribute 'max_work_group_size' is already applied with different arguments}} expected-note {{previous attribute is here}}
3125

3226
// Show that the attribute works on member functions.
3327
class Functor {
@@ -38,30 +32,30 @@ class Functor {
3832

3933
// Ensure that template arguments behave appropriately based on instantiations.
4034
template <int N>
41-
[[intel::max_work_group_size(N, 1, 1)]] void f8(); // #f8
35+
[[intel::max_work_group_size(N, 1, 1)]] void f6(); // #f6
4236

4337
// Test that template redeclarations also get diagnosed properly.
4438
template <int X, int Y, int Z>
45-
[[intel::max_work_group_size(1, 1, 1)]] void f9(); // #f9prev
39+
[[intel::max_work_group_size(1, 1, 1)]] void f7(); // #f7prev
4640

4741
template <int X, int Y, int Z>
48-
[[intel::max_work_group_size(X, Y, Z)]] void f9() {} // #f9
42+
[[intel::max_work_group_size(X, Y, Z)]] void f7() {} // #f7
4943

5044
// Test that a template redeclaration where the difference is known up front is
5145
// diagnosed immediately, even without instantiation.
5246
template <int X, int Y, int Z>
53-
[[intel::max_work_group_size(X, 1, Z)]] void f10(); // expected-note {{previous attribute is here}}
47+
[[intel::max_work_group_size(X, 1, Z)]] void f8(); // expected-note {{previous attribute is here}}
5448
template <int X, int Y, int Z>
55-
[[intel::max_work_group_size(X, 2, Z)]] void f10(); // expected-warning {{attribute 'max_work_group_size' is already applied with different arguments}}
49+
[[intel::max_work_group_size(X, 2, Z)]] void f8(); // expected-warning {{attribute 'max_work_group_size' is already applied with different arguments}}
5650

5751
void instantiate() {
58-
f8<1>(); // OK
59-
// expected-warning@#f8 {{implicit conversion changes signedness: 'int' to 'unsigned long long'}}
60-
f8<-1>(); // expected-note {{in instantiation}}
61-
// expected-error@#f8 {{'max_work_group_size' attribute must be greater than 0}}
62-
f8<0>(); // expected-note {{in instantiation}}
63-
f9<1, 1, 1>(); // OK, args are the same on the redecl.
64-
// expected-warning@#f9 {{attribute 'max_work_group_size' is already applied with different arguments}}
65-
// expected-note@#f9prev {{previous attribute is here}}
66-
f9<2, 2, 2>(); // expected-note {{in instantiation}}
52+
f6<1>(); // OK
53+
// expected-warning@#f6 {{implicit conversion changes signedness: 'int' to 'unsigned long long'}}
54+
f6<-1>(); // expected-note {{in instantiation}}
55+
// expected-error@#f6 {{'max_work_group_size' attribute must be greater than 0}}
56+
f6<0>(); // expected-note {{in instantiation}}
57+
f7<1, 1, 1>(); // OK, args are the same on the redecl.
58+
// expected-warning@#f7 {{attribute 'max_work_group_size' is already applied with different arguments}}
59+
// expected-note@#f7prev {{previous attribute is here}}
60+
f7<2, 2, 2>(); // expected-note {{in instantiation}}
6761
}

0 commit comments

Comments
 (0)