[PATCH] D28321: [CUDA] Add __declspec spellings for CUDA attributes.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL291134: [CUDA] Add __declspec spellings for CUDA attributes.
(authored by jlebar).
Changed prior to commit:
https://reviews.llvm.org/D28321?vs=83128&id=83257#toc
Repository:
rL LLVM
https://reviews.llvm.org/D28321
Files:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/test/SemaCUDA/attr-declspec.cu
Index: cfe/trunk/test/SemaCUDA/attr-declspec.cu
===
--- cfe/trunk/test/SemaCUDA/attr-declspec.cu
+++ cfe/trunk/test/SemaCUDA/attr-declspec.cu
@@ -0,0 +1,34 @@
+// Test the __declspec spellings of CUDA attributes.
+//
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -fcuda-is-device -verify %s
+// Now pretend that we're compiling a C file. There should be warnings.
+// RUN: %clang_cc1 -DEXPECT_WARNINGS -fms-extensions -fsyntax-only -verify -x c %s
+
+#if defined(EXPECT_WARNINGS)
+// expected-warning@+12 {{'__device__' attribute ignored}}
+// expected-warning@+12 {{'__global__' attribute ignored}}
+// expected-warning@+12 {{'__constant__' attribute ignored}}
+// expected-warning@+12 {{'__shared__' attribute ignored}}
+// expected-warning@+12 {{'__host__' attribute ignored}}
+//
+// (Currently we don't for the other attributes. They are implemented with
+// IgnoredAttr, which is ignored irrespective of any LangOpts.)
+#else
+// expected-no-diagnostics
+#endif
+
+__declspec(__device__) void f_device();
+__declspec(__global__) void f_global();
+__declspec(__constant__) int* g_constant;
+__declspec(__shared__) float *g_shared;
+__declspec(__host__) void f_host();
+__declspec(__device_builtin__) void f_device_builtin();
+typedef __declspec(__device_builtin__) const void *t_device_builtin;
+enum __declspec(__device_builtin__) e_device_builtin {E};
+__declspec(__device_builtin__) int v_device_builtin;
+__declspec(__cudart_builtin__) void f_cudart_builtin();
+__declspec(__device_builtin_surface_type__) unsigned long long surface_var;
+__declspec(__device_builtin_texture_type__) unsigned long long texture_var;
+
+// Note that there's no __declspec spelling of nv_weak.
Index: cfe/trunk/include/clang/Basic/Attr.td
===
--- cfe/trunk/include/clang/Basic/Attr.td
+++ cfe/trunk/include/clang/Basic/Attr.td
@@ -601,49 +601,53 @@
let Documentation = [Undocumented];
}
+// CUDA attributes are spelled __attribute__((attr)) or __declspec(__attr__).
+
def CUDAConstant : InheritableAttr {
- let Spellings = [GNU<"constant">];
+ let Spellings = [GNU<"constant">, Declspec<"__constant__">];
let Subjects = SubjectList<[Var]>;
let LangOpts = [CUDA];
let Documentation = [Undocumented];
}
def CUDACudartBuiltin : IgnoredAttr {
- let Spellings = [GNU<"cudart_builtin">];
+ let Spellings = [GNU<"cudart_builtin">, Declspec<"__cudart_builtin__">];
let LangOpts = [CUDA];
}
def CUDADevice : InheritableAttr {
- let Spellings = [GNU<"device">];
+ let Spellings = [GNU<"device">, Declspec<"__device__">];
let Subjects = SubjectList<[Function, Var]>;
let LangOpts = [CUDA];
let Documentation = [Undocumented];
}
def CUDADeviceBuiltin : IgnoredAttr {
- let Spellings = [GNU<"device_builtin">];
+ let Spellings = [GNU<"device_builtin">, Declspec<"__device_builtin__">];
let LangOpts = [CUDA];
}
def CUDADeviceBuiltinSurfaceType : IgnoredAttr {
- let Spellings = [GNU<"device_builtin_surface_type">];
+ let Spellings = [GNU<"device_builtin_surface_type">,
+ Declspec<"__device_builtin_surface_type__">];
let LangOpts = [CUDA];
}
def CUDADeviceBuiltinTextureType : IgnoredAttr {
- let Spellings = [GNU<"device_builtin_texture_type">];
+ let Spellings = [GNU<"device_builtin_texture_type">,
+ Declspec<"__device_builtin_texture_type__">];
let LangOpts = [CUDA];
}
def CUDAGlobal : InheritableAttr {
- let Spellings = [GNU<"global">];
+ let Spellings = [GNU<"global">, Declspec<"__global__">];
let Subjects = SubjectList<[Function]>;
let LangOpts = [CUDA];
let Documentation = [Undocumented];
}
def CUDAHost : InheritableAttr {
- let Spellings = [GNU<"host">];
+ let Spellings = [GNU<"host">, Declspec<"__host__">];
let Subjects = SubjectList<[Function]>;
let LangOpts = [CUDA];
let Documentation = [Undocumented];
@@ -657,7 +661,7 @@
}
def CUDALaunchBounds : InheritableAttr {
- let Spellings = [GNU<"launch_bounds">];
+ let Spellings = [GNU<"launch_bounds">, Declspec<"__launch_bounds__">];
let Args = [ExprArgument<"MaxThreads">, ExprArgument<"MinBlocks", 1>];
let LangOpts = [CUDA];
let Subjects = SubjectList<[ObjCMethod, FunctionLike], WarnDiag,
@@ -669,7 +673,7 @@
}
def CUDAShared : InheritableAttr {
- let Spellings = [GNU<"shared">];
+ let Spellings = [GNU<"shared">, Declspec<"__shared__">];
let Subjects = SubjectList<[Var]>;
let LangOpt
[PATCH] D28321: [CUDA] Add __declspec spellings for CUDA attributes.
jlebar added a comment. Thank you for the review! Comment at: clang/include/clang/Basic/Attr.td:604 +// CUDA attributes are spelled __attribute__((attr)) or __declspec(__attr__). + aaron.ballman wrote: > jlebar wrote: > > aaron.ballman wrote: > > > jlebar wrote: > > > > aaron.ballman wrote: > > > > > For my own edification, do you have a link to some documentation of > > > > > what CUDA attributes are spelled with `__declspec`? > > > > I do not believe such documentation exists. It is an implementation > > > > detail in the CUDA headers -- users never write > > > > `__attribute__((device))` or `__declspec(__device__)`. Instead, they > > > > write `__device__`. > > > Then why are we introducing `__declspec(__device__)` rather than a > > > keyword attribute `__device__`? > > > > > > My biggest concern is: I would like to verify that these actually should > > > be supported as a `__declspec` attribute. From my simple testing in MSVC, > > > it does not appear to support `__declspec(__device__)`, but perhaps I am > > > doing it wrong (I'm mostly unfamiliar with CUDA). If this isn't something > > > MSVC supports, then it is the first attribute we're supporting with a > > > __declspec spelling that is not actually a Microsoft attribute, which is > > > something worth discussing. > > > Then why are we introducing __declspec(__device__) rather than a keyword > > > attribute __device__? > > > > `__device__` is a macro defined in the CUDA headers, which must include and > > we are not able to modify. > Okay, it makes sense as to why we can't have a keyword spelling, but it also > doesn't answer why we need the `__declspec` spelling for it. Are you saying > that there are CUDA headers out there where this attribute is spelled with > `__attribute__` and others with `__declspec`? If so, then this change makes a > bit more sense to me. (Phab bug; I can't delete this comment. Please ignore.) Comment at: clang/include/clang/Basic/Attr.td:610 let LangOpts = [CUDA]; let Documentation = [Undocumented]; } aaron.ballman wrote: > jlebar wrote: > > aaron.ballman wrote: > > > jlebar wrote: > > > > aaron.ballman wrote: > > > > > Now would be a good time to add the documentation for these > > > > > attributes, otherwise there's a lot less chance users will know what > > > > > ways they can spell the attributes (or what the attribute do). > > > > See above, these are an implementation detail. > > > We can still document that we support the attributes under their macro > > > names, or do users not typically think of these macros as being > > > attributes? I am mostly concerned about discoverability of the attributes > > > -- how is a user to know what Clang does or does not support? > > > how is a user to know what Clang does or does not support? > > > > These macros are fundamental to CUDA support. The statement "you can > > compile CUDA code with clang" will immediately imply to every CUDA > > developer in existence the statement "you can use `__device__` in your > > code". > Yet we add new CUDA attributes periodically, and CUDA comes out with new > versions and new features. > > Looking at the CUDA docs, I see `__managed__`, but I don't see such an > attribute in Clang. How is a user to know whether we do/don't support such a > construct? This is a fair point, I agree on this basis that we should add documentation here. https://reviews.llvm.org/D28321 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28321: [CUDA] Add __declspec spellings for CUDA attributes.
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. After getting some realtime clarifications in IRC, I now understand better why this is needed. This patch LGTM! The documentation points I raised are still valid, but are by no means required for this patch to go in. https://reviews.llvm.org/D28321 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28321: [CUDA] Add __declspec spellings for CUDA attributes.
aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:604 +// CUDA attributes are spelled __attribute__((attr)) or __declspec(__attr__). + jlebar wrote: > aaron.ballman wrote: > > jlebar wrote: > > > aaron.ballman wrote: > > > > For my own edification, do you have a link to some documentation of > > > > what CUDA attributes are spelled with `__declspec`? > > > I do not believe such documentation exists. It is an implementation > > > detail in the CUDA headers -- users never write `__attribute__((device))` > > > or `__declspec(__device__)`. Instead, they write `__device__`. > > Then why are we introducing `__declspec(__device__)` rather than a keyword > > attribute `__device__`? > > > > My biggest concern is: I would like to verify that these actually should be > > supported as a `__declspec` attribute. From my simple testing in MSVC, it > > does not appear to support `__declspec(__device__)`, but perhaps I am doing > > it wrong (I'm mostly unfamiliar with CUDA). If this isn't something MSVC > > supports, then it is the first attribute we're supporting with a __declspec > > spelling that is not actually a Microsoft attribute, which is something > > worth discussing. > > Then why are we introducing __declspec(__device__) rather than a keyword > > attribute __device__? > > `__device__` is a macro defined in the CUDA headers, which must include and > we are not able to modify. Okay, it makes sense as to why we can't have a keyword spelling, but it also doesn't answer why we need the `__declspec` spelling for it. Are you saying that there are CUDA headers out there where this attribute is spelled with `__attribute__` and others with `__declspec`? If so, then this change makes a bit more sense to me. Comment at: clang/include/clang/Basic/Attr.td:610 let LangOpts = [CUDA]; let Documentation = [Undocumented]; } jlebar wrote: > aaron.ballman wrote: > > jlebar wrote: > > > aaron.ballman wrote: > > > > Now would be a good time to add the documentation for these attributes, > > > > otherwise there's a lot less chance users will know what ways they can > > > > spell the attributes (or what the attribute do). > > > See above, these are an implementation detail. > > We can still document that we support the attributes under their macro > > names, or do users not typically think of these macros as being attributes? > > I am mostly concerned about discoverability of the attributes -- how is a > > user to know what Clang does or does not support? > > how is a user to know what Clang does or does not support? > > These macros are fundamental to CUDA support. The statement "you can compile > CUDA code with clang" will immediately imply to every CUDA developer in > existence the statement "you can use `__device__` in your code". Yet we add new CUDA attributes periodically, and CUDA comes out with new versions and new features. Looking at the CUDA docs, I see `__managed__`, but I don't see such an attribute in Clang. How is a user to know whether we do/don't support such a construct? https://reviews.llvm.org/D28321 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28321: [CUDA] Add __declspec spellings for CUDA attributes.
jlebar added inline comments. Comment at: clang/include/clang/Basic/Attr.td:604 +// CUDA attributes are spelled __attribute__((attr)) or __declspec(__attr__). + aaron.ballman wrote: > jlebar wrote: > > aaron.ballman wrote: > > > For my own edification, do you have a link to some documentation of what > > > CUDA attributes are spelled with `__declspec`? > > I do not believe such documentation exists. It is an implementation detail > > in the CUDA headers -- users never write `__attribute__((device))` or > > `__declspec(__device__)`. Instead, they write `__device__`. > Then why are we introducing `__declspec(__device__)` rather than a keyword > attribute `__device__`? > > My biggest concern is: I would like to verify that these actually should be > supported as a `__declspec` attribute. From my simple testing in MSVC, it > does not appear to support `__declspec(__device__)`, but perhaps I am doing > it wrong (I'm mostly unfamiliar with CUDA). If this isn't something MSVC > supports, then it is the first attribute we're supporting with a __declspec > spelling that is not actually a Microsoft attribute, which is something worth > discussing. > Then why are we introducing __declspec(__device__) rather than a keyword > attribute __device__? `__device__` is a macro defined in the CUDA headers, which must include and we are not able to modify. Comment at: clang/include/clang/Basic/Attr.td:610 let LangOpts = [CUDA]; let Documentation = [Undocumented]; } aaron.ballman wrote: > jlebar wrote: > > aaron.ballman wrote: > > > Now would be a good time to add the documentation for these attributes, > > > otherwise there's a lot less chance users will know what ways they can > > > spell the attributes (or what the attribute do). > > See above, these are an implementation detail. > We can still document that we support the attributes under their macro names, > or do users not typically think of these macros as being attributes? I am > mostly concerned about discoverability of the attributes -- how is a user to > know what Clang does or does not support? > how is a user to know what Clang does or does not support? These macros are fundamental to CUDA support. The statement "you can compile CUDA code with clang" will immediately imply to every CUDA developer in existence the statement "you can use `__device__` in your code". https://reviews.llvm.org/D28321 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28321: [CUDA] Add __declspec spellings for CUDA attributes.
aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:604 +// CUDA attributes are spelled __attribute__((attr)) or __declspec(__attr__). + jlebar wrote: > aaron.ballman wrote: > > For my own edification, do you have a link to some documentation of what > > CUDA attributes are spelled with `__declspec`? > I do not believe such documentation exists. It is an implementation detail > in the CUDA headers -- users never write `__attribute__((device))` or > `__declspec(__device__)`. Instead, they write `__device__`. Then why are we introducing `__declspec(__device__)` rather than a keyword attribute `__device__`? My biggest concern is: I would like to verify that these actually should be supported as a `__declspec` attribute. From my simple testing in MSVC, it does not appear to support `__declspec(__device__)`, but perhaps I am doing it wrong (I'm mostly unfamiliar with CUDA). If this isn't something MSVC supports, then it is the first attribute we're supporting with a __declspec spelling that is not actually a Microsoft attribute, which is something worth discussing. Comment at: clang/include/clang/Basic/Attr.td:610 let LangOpts = [CUDA]; let Documentation = [Undocumented]; } jlebar wrote: > aaron.ballman wrote: > > Now would be a good time to add the documentation for these attributes, > > otherwise there's a lot less chance users will know what ways they can > > spell the attributes (or what the attribute do). > See above, these are an implementation detail. We can still document that we support the attributes under their macro names, or do users not typically think of these macros as being attributes? I am mostly concerned about discoverability of the attributes -- how is a user to know what Clang does or does not support? https://reviews.llvm.org/D28321 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28321: [CUDA] Add __declspec spellings for CUDA attributes.
jlebar added inline comments. Comment at: clang/include/clang/Basic/Attr.td:604 +// CUDA attributes are spelled __attribute__((attr)) or __declspec(__attr__). + aaron.ballman wrote: > For my own edification, do you have a link to some documentation of what CUDA > attributes are spelled with `__declspec`? I do not believe such documentation exists. It is an implementation detail in the CUDA headers -- users never write `__attribute__((device))` or `__declspec(__device__)`. Instead, they write `__device__`. Comment at: clang/include/clang/Basic/Attr.td:610 let LangOpts = [CUDA]; let Documentation = [Undocumented]; } aaron.ballman wrote: > Now would be a good time to add the documentation for these attributes, > otherwise there's a lot less chance users will know what ways they can spell > the attributes (or what the attribute do). See above, these are an implementation detail. https://reviews.llvm.org/D28321 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28321: [CUDA] Add __declspec spellings for CUDA attributes.
aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:604 +// CUDA attributes are spelled __attribute__((attr)) or __declspec(__attr__). + For my own edification, do you have a link to some documentation of what CUDA attributes are spelled with `__declspec`? Comment at: clang/include/clang/Basic/Attr.td:610 let LangOpts = [CUDA]; let Documentation = [Undocumented]; } Now would be a good time to add the documentation for these attributes, otherwise there's a lot less chance users will know what ways they can spell the attributes (or what the attribute do). https://reviews.llvm.org/D28321 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28321: [CUDA] Add __declspec spellings for CUDA attributes.
jlebar created this revision.
jlebar added a reviewer: tra.
jlebar added subscribers: rnk, cfe-commits.
CUDA attributes are spelled __declspec(__foo__) on Windows.
https://reviews.llvm.org/D28321
Files:
clang/include/clang/Basic/Attr.td
clang/test/SemaCUDA/attr-declspec.cu
Index: clang/test/SemaCUDA/attr-declspec.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/attr-declspec.cu
@@ -0,0 +1,34 @@
+// Test the __declspec spellings of CUDA attributes.
+//
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -fcuda-is-device -verify %s
+// Now pretend that we're compiling a C file. There should be warnings.
+// RUN: %clang_cc1 -DEXPECT_WARNINGS -fms-extensions -fsyntax-only -verify -x c %s
+
+#if defined(EXPECT_WARNINGS)
+// expected-warning@+12 {{'__device__' attribute ignored}}
+// expected-warning@+12 {{'__global__' attribute ignored}}
+// expected-warning@+12 {{'__constant__' attribute ignored}}
+// expected-warning@+12 {{'__shared__' attribute ignored}}
+// expected-warning@+12 {{'__host__' attribute ignored}}
+//
+// (Currently we don't for the other attributes. They are implemented with
+// IgnoredAttr, which is ignored irrespective of any LangOpts.)
+#else
+// expected-no-diagnostics
+#endif
+
+__declspec(__device__) void f_device();
+__declspec(__global__) void f_global();
+__declspec(__constant__) int* g_constant;
+__declspec(__shared__) float *g_shared;
+__declspec(__host__) void f_host();
+__declspec(__device_builtin__) void f_device_builtin();
+typedef __declspec(__device_builtin__) const void *t_device_builtin;
+enum __declspec(__device_builtin__) e_device_builtin {E};
+__declspec(__device_builtin__) int v_device_builtin;
+__declspec(__cudart_builtin__) void f_cudart_builtin();
+__declspec(__device_builtin_surface_type__) unsigned long long surface_var;
+__declspec(__device_builtin_texture_type__) unsigned long long texture_var;
+
+// Note that there's no __declspec spelling of nv_weak.
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -601,49 +601,53 @@
let Documentation = [Undocumented];
}
+// CUDA attributes are spelled __attribute__((attr)) or __declspec(__attr__).
+
def CUDAConstant : InheritableAttr {
- let Spellings = [GNU<"constant">];
+ let Spellings = [GNU<"constant">, Declspec<"__constant__">];
let Subjects = SubjectList<[Var]>;
let LangOpts = [CUDA];
let Documentation = [Undocumented];
}
def CUDACudartBuiltin : IgnoredAttr {
- let Spellings = [GNU<"cudart_builtin">];
+ let Spellings = [GNU<"cudart_builtin">, Declspec<"__cudart_builtin__">];
let LangOpts = [CUDA];
}
def CUDADevice : InheritableAttr {
- let Spellings = [GNU<"device">];
+ let Spellings = [GNU<"device">, Declspec<"__device__">];
let Subjects = SubjectList<[Function, Var]>;
let LangOpts = [CUDA];
let Documentation = [Undocumented];
}
def CUDADeviceBuiltin : IgnoredAttr {
- let Spellings = [GNU<"device_builtin">];
+ let Spellings = [GNU<"device_builtin">, Declspec<"__device_builtin__">];
let LangOpts = [CUDA];
}
def CUDADeviceBuiltinSurfaceType : IgnoredAttr {
- let Spellings = [GNU<"device_builtin_surface_type">];
+ let Spellings = [GNU<"device_builtin_surface_type">,
+ Declspec<"__device_builtin_surface_type__">];
let LangOpts = [CUDA];
}
def CUDADeviceBuiltinTextureType : IgnoredAttr {
- let Spellings = [GNU<"device_builtin_texture_type">];
+ let Spellings = [GNU<"device_builtin_texture_type">,
+ Declspec<"__device_builtin_texture_type__">];
let LangOpts = [CUDA];
}
def CUDAGlobal : InheritableAttr {
- let Spellings = [GNU<"global">];
+ let Spellings = [GNU<"global">, Declspec<"__global__">];
let Subjects = SubjectList<[Function]>;
let LangOpts = [CUDA];
let Documentation = [Undocumented];
}
def CUDAHost : InheritableAttr {
- let Spellings = [GNU<"host">];
+ let Spellings = [GNU<"host">, Declspec<"__host__">];
let Subjects = SubjectList<[Function]>;
let LangOpts = [CUDA];
let Documentation = [Undocumented];
@@ -657,7 +661,7 @@
}
def CUDALaunchBounds : InheritableAttr {
- let Spellings = [GNU<"launch_bounds">];
+ let Spellings = [GNU<"launch_bounds">, Declspec<"__launch_bounds__">];
let Args = [ExprArgument<"MaxThreads">, ExprArgument<"MinBlocks", 1>];
let LangOpts = [CUDA];
let Subjects = SubjectList<[ObjCMethod, FunctionLike], WarnDiag,
@@ -669,7 +673,7 @@
}
def CUDAShared : InheritableAttr {
- let Spellings = [GNU<"shared">];
+ let Spellings = [GNU<"shared">, Declspec<"__shared__">];
let Subjects = SubjectList<[Var]>;
let LangOpts = [CUDA];
let Documentation = [Undocumented];
@@ -1195,6 +1199,8 @@
}
def NvWeak : IgnoredAttr {
+ // No Declspec spelling of this attribute; the CUDA headers use
+
