Skip to content

Commit d4e97ab

Browse files
elizabethandrewselizabethandrews
committed
Apply review comments
Signed-off-by: Elizabeth Andrews <[email protected]>
1 parent 839c3ff commit d4e97ab

File tree

10 files changed

+63
-53
lines changed

10 files changed

+63
-53
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1282,7 +1282,7 @@ def SYCLSpecialClass: InheritableAttr {
12821282
}
12831283

12841284
def SYCLType: InheritableAttr {
1285-
let Spellings = [Clang<"sycl_type">];
1285+
let Spellings = [CXX11<"__sycl_detail__", "sycl_type">];
12861286
let Subjects = SubjectList<[CXXRecord, Enum], ErrorDiag>;
12871287
let LangOpts = [SYCLIsDevice, SYCLIsHost];
12881288
let Args = [EnumArgument<"Type", "SYCLType",
@@ -1295,7 +1295,7 @@ def SYCLType: InheritableAttr {
12951295
"no_alias", "accessor_property_list", "group",
12961296
"private_memory", "aspect"]>];
12971297
// Only used internally by SYCL implementation
1298-
let Documentation = [Undocumented];
1298+
let Documentation = [InternalOnly];
12991299
}
13001300

13011301
def SYCLDeviceHas : InheritableAttr {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4098,6 +4098,8 @@ def warn_transparent_union_attribute_zero_fields : Warning<
40984098
def warn_attribute_type_not_supported : Warning<
40994099
"%0 attribute argument not supported: %1">,
41004100
InGroup<IgnoredAttributes>;
4101+
def err_attribute_argument_not_supported : Error<
4102+
"%0 attribute argument %1 is not supported">;
41014103
def warn_attribute_unknown_visibility : Warning<"unknown visibility %0">,
41024104
InGroup<IgnoredAttributes>;
41034105
def warn_attribute_protected_visibility :

‎clang/lib/Sema/SemaDeclAttr.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10563,7 +10563,7 @@ SYCLTypeAttr *Sema::MergeSYCLTypeAttr(Decl *D, const AttributeCommonInfo &CI,
1056310563
SYCLTypeAttr::SYCLType TypeName) {
1056410564
if (const auto ExistingAttr = D->getAttr<SYCLTypeAttr>()) {
1056510565
if (ExistingAttr->getType() != TypeName) {
10566-
Diag(ExistingAttr->getLoc(), diag::warn_duplicate_attribute)
10566+
Diag(ExistingAttr->getLoc(), diag::err_duplicate_attribute)
1056710567
<< ExistingAttr;
1056810568
Diag(CI.getLoc(), diag::note_previous_attribute);
1056910569
}
@@ -10584,7 +10584,7 @@ static void handleSYCLTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
1058410584
SYCLTypeAttr::SYCLType Type;
1058510585

1058610586
if (!SYCLTypeAttr::ConvertStrToSYCLType(II->getName(), Type)) {
10587-
S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported) << AL << II;
10587+
S.Diag(AL.getLoc(), diag::err_attribute_argument_not_supported) << AL << II;
1058810588
return;
1058910589
}
1059010590

‎clang/lib/Sema/SemaSYCL.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,10 @@ static bool isAccessorPropertyType(QualType Ty,
103103
return false;
104104
}
105105

106+
static bool isSyclSpecialType(QualType Ty, Sema &S) {
107+
return S.isTypeDecoratedWithDeclAttribute<SYCLSpecialClassAttr>(Ty);
108+
}
109+
106110
ExprResult Sema::ActOnSYCLBuiltinNumFieldsExpr(ParsedType PT) {
107111
TypeSourceInfo *TInfo = nullptr;
108112
QualType QT = GetTypeFromParser(PT, &TInfo);
@@ -1152,11 +1156,10 @@ class KernelObjVisitor {
11521156
for (const auto &Base : Range) {
11531157
QualType BaseTy = Base.getType();
11541158
// Handle accessor class as base
1155-
if (SemaRef.isTypeDecoratedWithDeclAttribute<SYCLSpecialClassAttr>(
1156-
BaseTy)) {
1159+
if (isSyclSpecialType(BaseTy, SemaRef))
11571160
(void)std::initializer_list<int>{
11581161
(Handlers.handleSyclSpecialType(Owner, Base, BaseTy), 0)...};
1159-
} else
1162+
else
11601163
// For all other bases, visit the record
11611164
visitRecord(Owner, Base, BaseTy->getAsCXXRecordDecl(), BaseTy,
11621165
Handlers...);
@@ -1235,7 +1238,7 @@ class KernelObjVisitor {
12351238
template <typename... HandlerTys>
12361239
void visitField(const CXXRecordDecl *Owner, FieldDecl *Field,
12371240
QualType FieldTy, HandlerTys &... Handlers) {
1238-
if (SemaRef.isTypeDecoratedWithDeclAttribute<SYCLSpecialClassAttr>(FieldTy))
1241+
if (isSyclSpecialType(FieldTy, SemaRef))
12391242
KF_FOR_EACH(handleSyclSpecialType, Field, FieldTy);
12401243
else if (isSyclType(FieldTy, SYCLTypeAttr::spec_constant))
12411244
KF_FOR_EACH(handleSyclSpecConstantType, Field, FieldTy);
@@ -1579,7 +1582,7 @@ class SyclKernelFieldChecker : public SyclKernelFieldHandler {
15791582
}
15801583

15811584
bool checkSyclSpecialType(QualType Ty, SourceRange Loc) {
1582-
assert(SemaRef.isTypeDecoratedWithDeclAttribute<SYCLSpecialClassAttr>(Ty) &&
1585+
assert(isSyclSpecialType(Ty, SemaRef) &&
15831586
"Should only be called on sycl special class types.");
15841587
const RecordDecl *RecD = Ty->getAsRecordDecl();
15851588
if (IsSIMD && !isSyclAccessorType(Ty))

‎clang/test/CodeGenSYCL/Inputs/sycl.hpp

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#pragma once
22

33
#define ATTR_SYCL_KERNEL __attribute__((sycl_kernel))
4+
#define __SYCL_TYPE(x) [[__sycl_detail__::sycl_type(x)]]
45

56
extern "C" int printf(const char* fmt, ...);
67

@@ -29,7 +30,7 @@ class __attribute__((sycl_special_class)) sampler {
2930
};
3031

3132
template <int dimensions = 1>
32-
class __attribute__((sycl_type(group))) group {
33+
class __SYCL_TYPE(group) group {
3334
public:
3435
group() = default; // fake constructor
3536
};
@@ -69,7 +70,7 @@ enum class address_space : int {
6970
} // namespace access
7071

7172
// Dummy aspect enum with limited enumerators
72-
enum class __attribute__((sycl_type(aspect))) aspect {
73+
enum class __SYCL_TYPE(aspect) aspect {
7374
host = 0,
7475
cpu = 1,
7576
gpu = 2,
@@ -119,7 +120,7 @@ namespace ext {
119120
namespace intel {
120121
namespace property {
121122
// Compile time known accessor property
122-
struct __attribute__((sycl_type(buffer_location))) buffer_location {
123+
struct __SYCL_TYPE(buffer_location) buffer_location {
123124
template <int> class instance {};
124125
};
125126
} // namespace property
@@ -130,7 +131,7 @@ namespace ext {
130131
namespace oneapi {
131132
namespace property {
132133
// Compile time known accessor property
133-
struct __attribute__((sycl_type(no_alias))) no_alias {
134+
struct __SYCL_TYPE(no_alias) no_alias {
134135
template <bool> class instance {};
135136
};
136137
} // namespace property
@@ -172,7 +173,7 @@ template <int dim> struct item {
172173
namespace ext {
173174
namespace oneapi {
174175
template <typename... properties>
175-
class __attribute__((sycl_type(accessor_property_list))) accessor_property_list {};
176+
class __SYCL_TYPE(accessor_property_list) accessor_property_list {};
176177
} // namespace oneapi
177178
} // namespace ext
178179

@@ -201,7 +202,7 @@ template <typename dataT, int dimensions, access::mode accessmode,
201202
access::target accessTarget = access::target::global_buffer,
202203
access::placeholder isPlaceholder = access::placeholder::false_t,
203204
typename propertyListT = ext::oneapi::accessor_property_list<>>
204-
class __attribute__((sycl_special_class)) __attribute__((sycl_type(accessor))) accessor {
205+
class __attribute__((sycl_special_class)) __SYCL_TYPE(accessor) accessor {
205206

206207
public:
207208
void use(void) const {}
@@ -267,7 +268,7 @@ struct _ImageImplT {
267268
};
268269

269270
template <typename dataT, int dimensions, access::mode accessmode>
270-
class __attribute__((sycl_special_class)) __attribute__((sycl_type(accessor))) accessor<dataT, dimensions, accessmode, access::target::image, access::placeholder::false_t> {
271+
class __attribute__((sycl_special_class)) __SYCL_TYPE(accessor) accessor<dataT, dimensions, accessmode, access::target::image, access::placeholder::false_t> {
271272
public:
272273
void use(void) const {}
273274
template <typename... T>
@@ -292,7 +293,7 @@ class accessor<dataT, dimensions, accessmode, access::target::host_image, access
292293
};
293294

294295
template <typename dataT, int dimensions>
295-
class __attribute__((sycl_special_class)) __attribute__((sycl_type(local_accessor)))
296+
class __attribute__((sycl_special_class)) __SYCL_TYPE(local_accessor)
296297
local_accessor: public accessor<dataT,
297298
dimensions, access::mode::read_write,
298299
access::target::local> {
@@ -344,7 +345,7 @@ namespace ext {
344345
namespace oneapi {
345346
namespace experimental {
346347
template <typename T, typename ID = T>
347-
class __attribute__((sycl_type(spec_constant))) spec_constant {
348+
class __SYCL_TYPE(spec_constant) spec_constant {
348349
public:
349350
spec_constant() {}
350351
spec_constant(T Cst) {}
@@ -375,11 +376,11 @@ int printf(const __SYCL_CONSTANT_AS char *__format, Args... args) {
375376
} // namespace oneapi
376377
} // namespace ext
377378

378-
class __attribute__((sycl_type(kernel_handler))) kernel_handler {
379+
class __SYCL_TYPE(kernel_handler) kernel_handler {
379380
void __init_specialization_constants_buffer(char *specialization_constants_buffer) {}
380381
};
381382

382-
template <typename T> class __attribute__((sycl_type(specialization_id))) specialization_id {
383+
template <typename T> class __SYCL_TYPE(specialization_id) specialization_id {
383384
public:
384385
using value_type = T;
385386

‎clang/test/CodeGenSYCL/accessor-readonly-invalid-lib.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ struct _ImplT {
6060
template <typename dataT, int dimensions, access::mode accessmode,
6161
access::target accessTarget = access::target::global_buffer,
6262
access::placeholder isPlaceholder = access::placeholder::false_t>
63-
class __attribute__((sycl_special_class)) __attribute__((sycl_type(accessor))) accessor {
63+
class __attribute__((sycl_special_class)) [[__sycl_detail__::sycl_type(accessor)]] accessor {
6464

6565
public:
6666
void use(void) const {}

‎clang/test/SemaSYCL/Inputs/sycl.hpp

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#ifndef SYCL_HPP
22
#define SYCL_HPP
33

4+
#define __SYCL_TYPE(x) [[__sycl_detail__::sycl_type(x)]]
5+
46
// Shared code for SYCL tests
57

68
namespace sycl {
@@ -39,7 +41,7 @@ enum class address_space : int {
3941
} // namespace access
4042

4143
// Dummy aspect enum with limited enumerators
42-
enum class __attribute__((sycl_type(aspect))) aspect {
44+
enum class __SYCL_TYPE(aspect) aspect {
4345
host = 0,
4446
cpu = 1,
4547
gpu = 2,
@@ -54,7 +56,7 @@ class property_list {};
5456
namespace ext {
5557
namespace intel {
5658
namespace property {
57-
struct __attribute__((sycl_type(buffer_location))) buffer_location {
59+
struct __SYCL_TYPE(buffer_location) buffer_location {
5860
template <int> class instance {};
5961
};
6062
} // namespace property
@@ -64,7 +66,7 @@ struct __attribute__((sycl_type(buffer_location))) buffer_location {
6466
namespace ext {
6567
namespace oneapi {
6668
template <typename... properties>
67-
class __attribute__((sycl_type(accessor_property_list))) accessor_property_list {};
69+
class __SYCL_TYPE(accessor_property_list) accessor_property_list {};
6870

6971
// device_global type decorated with attributes
7072
template <typename T>
@@ -131,7 +133,7 @@ template <typename dataT, int dimensions, access::mode accessmode,
131133
access::target accessTarget = access::target::global_buffer,
132134
access::placeholder isPlaceholder = access::placeholder::false_t,
133135
typename propertyListT = ext::oneapi::accessor_property_list<>>
134-
class __attribute__((sycl_special_class)) __attribute__((sycl_type(accessor))) accessor {
136+
class __attribute__((sycl_special_class)) __SYCL_TYPE(accessor) accessor {
135137
public:
136138
void use(void) const {}
137139
void use(void *) const {}
@@ -194,7 +196,7 @@ struct _ImageImplT {
194196
};
195197

196198
template <typename dataT, int dimensions, access::mode accessmode>
197-
class __attribute__((sycl_special_class)) __attribute__((sycl_type(accessor))) accessor<dataT, dimensions, accessmode, access::target::image, access::placeholder::false_t> {
199+
class __attribute__((sycl_special_class)) __SYCL_TYPE(accessor) accessor<dataT, dimensions, accessmode, access::target::image, access::placeholder::false_t> {
198200
public:
199201
void use(void) const {}
200202
template <typename... T>
@@ -208,7 +210,7 @@ class __attribute__((sycl_special_class)) __attribute__((sycl_type(accessor)))
208210
};
209211

210212
template <typename dataT, int dimensions>
211-
class __attribute__((sycl_special_class)) __attribute__((sycl_type(local_accessor)))
213+
class __attribute__((sycl_special_class)) __SYCL_TYPE(local_accessor)
212214
local_accessor: public accessor<dataT,
213215
dimensions, access::mode::read_write,
214216
access::target::local> {
@@ -260,12 +262,12 @@ struct get_kernel_name_t<auto_name, Type> {
260262
};
261263

262264
template <int dimensions = 1>
263-
class __attribute__((sycl_type(group))) group {
265+
class __SYCL_TYPE(group) group {
264266
public:
265267
group() = default; // fake constructor
266268
};
267269

268-
class __attribute__((sycl_type(kernel_handler))) kernel_handler {
270+
class __SYCL_TYPE(kernel_handler) kernel_handler {
269271
void __init_specialization_constants_buffer(char *specialization_constants_buffer) {}
270272
};
271273

@@ -393,7 +395,7 @@ namespace ext {
393395
namespace oneapi {
394396
namespace experimental {
395397
template <typename T, typename ID = T>
396-
class __attribute__((sycl_type(spec_constant))) spec_constant {
398+
class __SYCL_TYPE(spec_constant) spec_constant {
397399
public:
398400
spec_constant() {}
399401
explicit constexpr spec_constant(T defaultVal) : DefaultValue(defaultVal) {}

‎clang/test/SemaSYCL/sycl-type-attr-ast.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,31 +2,32 @@
22

33
// Tests for AST of sycl_type() attribute
44

5-
class __attribute__((sycl_type(accessor))) accessor {};
5+
class [[__sycl_detail__::sycl_type(accessor)]] accessor {};
66
// CHECK: CXXRecordDecl {{.*}} class accessor definition
77
// CHECK: SYCLTypeAttr {{.*}} accessor
88

99
class accessor;
1010
// CHECK: CXXRecordDecl {{.*}} prev {{.*}} class accessor
1111
// CHECK: SYCLTypeAttr {{.*}} Inherited accessor
1212

13-
enum class __attribute__((sycl_type(aspect))) aspect {};
13+
enum class [[__sycl_detail__::sycl_type(aspect)]] aspect {};
1414
// CHECK: EnumDecl {{.*}} class aspect 'int'
1515
// CHECK: SYCLTypeAttr {{.*}} aspect
1616

1717
template <typename T>
18-
class __attribute__((sycl_type(local_accessor))) local_accessor {};
18+
class [[__sycl_detail__::sycl_type(local_accessor)]] local_accessor {};
1919
// CHECK: ClassTemplateDecl {{.*}} local_accessor
2020
// CHECK: CXXRecordDecl {{.*}} class local_accessor definition
2121
// CHECK: SYCLTypeAttr {{.*}} local_accessor
2222

23-
class __attribute__((sycl_type(spec_constant))) __attribute__((sycl_type(spec_constant))) spec_constant;
23+
class [[__sycl_detail__::sycl_type(spec_constant)]]
24+
[[__sycl_detail__::sycl_type(spec_constant)]] spec_constant;
2425
// CHECK: CXXRecordDecl {{.*}} spec_constant
2526
// CHECK: SYCLTypeAttr {{.*}} spec_constant
2627
// CHECK-NOT: SYCLTypeAttr {{.*}} spec_constant
2728

2829
template <>
29-
class __attribute__((sycl_type(local_accessor))) local_accessor <int> {};
30+
class [[__sycl_detail__::sycl_type(local_accessor)]] local_accessor <int> {};
3031
// CHECK: ClassTemplateSpecializationDecl {{.*}} class local_accessor definition
3132
// CHECK: SYCLTypeAttr {{.*}} local_accessor
3233

‎clang/test/SemaSYCL/sycl-type-attr.cpp

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,41 +5,42 @@
55
#include "sycl.hpp"
66

77
// expected-error@+1{{'sycl_type' attribute only applies to classes}}
8-
__attribute__((sycl_type(accessor))) int a;
8+
[[__sycl_detail__::sycl_type(accessor)]] int a;
99

1010
// expected-error@+1{{'sycl_type' attribute only applies to classes}}
11-
__attribute__((sycl_type(accessor))) void func1() {}
11+
[[__sycl_detail__::sycl_type(accessor)]] void func1() {}
1212

1313
// expected-error@+1{{'sycl_type' attribute requires an identifier}}
14-
class __attribute__((sycl_type("accessor"))) A {};
14+
class [[__sycl_detail__::sycl_type("accessor")]] A {};
1515

1616
// expected-error@+1{{'sycl_type' attribute takes one argument}}
17-
class __attribute__((sycl_type())) B {};
17+
class [[__sycl_detail__::sycl_type()]] B {};
1818

19-
// expected-warning@+1{{'sycl_type' attribute argument not supported: 'NotValidType'}}
20-
class __attribute__((sycl_type(NotValidType))) C {};
19+
// expected-error@+1{{'sycl_type' attribute argument 'NotValidType' is not supported}}
20+
class [[__sycl_detail__::sycl_type(NotValidType)]] C {};
2121

2222
// expected-note@+1{{previous attribute is here}}
23-
class __attribute__((sycl_type(spec_constant))) spec_constant;
24-
// expected-warning@+1{{attribute 'sycl_type' is already applied with different arguments}}
25-
class __attribute__((sycl_type(accessor))) spec_constant {};
23+
class [[__sycl_detail__::sycl_type(spec_constant)]] spec_constant;
24+
// expected-error@+1{{attribute 'sycl_type' is already applied with different arguments}}
25+
class [[__sycl_detail__::sycl_type(accessor)]] spec_constant {};
2626

27-
// expected-warning@+2{{attribute 'sycl_type' is already applied with different arguments}}
27+
// expected-error@+2{{attribute 'sycl_type' is already applied with different arguments}}
2828
// expected-note@+1{{previous attribute is here}}
29-
class __attribute__((sycl_type(group))) __attribute__((sycl_type(accessor))) group {};
29+
class [[__sycl_detail__::sycl_type(group)]] [[__sycl_detail__::sycl_type(accessor)]] group {};
3030

3131
// Valid usage -
3232

33-
class __attribute__((sycl_type(accessor))) accessor {};
33+
class [[__sycl_detail__::sycl_type(accessor)]] accessor {};
3434

3535
template <typename T>
36-
class __attribute__((sycl_type(local_accessor))) local_accessor {};
36+
class [[__sycl_detail__::sycl_type(local_accessor)]] local_accessor {};
3737

38-
enum class __attribute__((sycl_type(aspect))) aspect {};
38+
enum class [[__sycl_detail__::sycl_type(aspect)]] aspect {};
3939

4040
// No diagnostic for matching arguments.
41-
class __attribute__((sycl_type(kernel_handler))) kernel_handler;
42-
class __attribute__((sycl_type(kernel_handler))) __attribute__((sycl_type(kernel_handler))) kernel_handler {};
41+
class [[__sycl_detail__::sycl_type(kernel_handler)]] kernel_handler;
42+
class [[__sycl_detail__::sycl_type(kernel_handler)]]
43+
[[__sycl_detail__::sycl_type(kernel_handler)]] kernel_handler {};
4344
class kernel_handler;
4445

4546

‎sycl/include/sycl/detail/defines.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@
2727
#define __SYCL_SPECIAL_CLASS
2828
#endif
2929

30-
#if __has_attribute(sycl_type)
31-
#define __SYCL_TYPE(x) __attribute__((sycl_type(x)))
30+
#if __has_cpp_attribute(__sycl_detail__::sycl_type)
31+
#define __SYCL_TYPE(x) [[__sycl_detail__::sycl_type(x)]]
3232
#else
3333
#define __SYCL_TYPE(x)
3434
#endif

0 commit comments

Comments
 (0)