[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Artem Belevich via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbe86b6773b6b: [CUDA] Allow local static variables with 
target attributes. (authored by tra).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
  clang/test/SemaCUDA/bad-attributes.cu
  clang/test/SemaCUDA/device-var-init.cu

Index: clang/test/SemaCUDA/device-var-init.cu
===
--- clang/test/SemaCUDA/device-var-init.cu
+++ clang/test/SemaCUDA/device-var-init.cu
@@ -24,6 +24,12 @@
 
 __shared__ T s_t_i = {2};
 // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+__device__ T d_t_i = {2};
+__constant__ T c_t_i = {2};
+
+__device__ ECD d_ecd_i{};
+__shared__ ECD s_ecd_i{};
+__constant__ ECD c_ecd_i{};
 
 __device__ EC d_ec_i(3);
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
@@ -196,34 +202,218 @@
 __constant__ T_FA_NED c_t_fa_ned;
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 
-// Verify that only __shared__ local variables may be static on device
-// side and that they are not allowed to be initialized.
+// Verify that local variables may be static on device
+// side and that they conform to the initialization constraints.
+// __shared__ can't be initialized at all and others don't support dynamic initialization.
 __device__ void df_sema() {
-  static __shared__ NCFS s_ncfs;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-  static __shared__ UC s_uc;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-  static __shared__ NED s_ned;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-
   static __device__ int ds;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static __constant__ int dc;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static int v;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static const int cv = 1;
   static const __device__ int cds = 1;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static const __constant__ int cdc = 1;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
+
+
+  // __shared__ does not need to be explicitly static.
+  __shared__ int lsi;
+  // __constant__ and __device__ can not be non-static local
+  __constant__ int lci;
+  // expected-error@-1 {{__constant__ and __device__ are not allowed on non-static local variables}}
+  __device__ int ldi;
+  // expected-error@-1 {{__constant__ and __device__ are not allowed on non-static local variables}}
+
+  // Same test cases as for the globals above.
+
+  static __device__ int d_v_f = f();
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ int s_v_f = f();
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ int c_v_f = f();
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __shared__ T s_t_i = {2};
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __device__ T d_t_i = {2};
+  static __constant__ T c_t_i = {2};
+
+  static __device__ ECD d_ecd_i;
+  static __shared__ ECD s_ecd_i;
+  static __constant__ ECD c_ecd_i;
+
+  static __device__ EC d_ec_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ EC s_ec_i(3);
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ EC c_ec_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __device__ EC d_ec_i2 = {3};
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Artem Belevich via Phabricator via cfe-commits]

tra updated this revision to Diff 302419.
tra added a comment.

Remove the assert which is no longer valid.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
  clang/test/SemaCUDA/bad-attributes.cu
  clang/test/SemaCUDA/device-var-init.cu

Index: clang/test/SemaCUDA/device-var-init.cu
===
--- clang/test/SemaCUDA/device-var-init.cu
+++ clang/test/SemaCUDA/device-var-init.cu
@@ -24,6 +24,12 @@
 
 __shared__ T s_t_i = {2};
 // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+__device__ T d_t_i = {2};
+__constant__ T c_t_i = {2};
+
+__device__ ECD d_ecd_i{};
+__shared__ ECD s_ecd_i{};
+__constant__ ECD c_ecd_i{};
 
 __device__ EC d_ec_i(3);
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
@@ -196,34 +202,218 @@
 __constant__ T_FA_NED c_t_fa_ned;
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 
-// Verify that only __shared__ local variables may be static on device
-// side and that they are not allowed to be initialized.
+// Verify that local variables may be static on device
+// side and that they conform to the initialization constraints.
+// __shared__ can't be initialized at all and others don't support dynamic initialization.
 __device__ void df_sema() {
-  static __shared__ NCFS s_ncfs;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-  static __shared__ UC s_uc;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-  static __shared__ NED s_ned;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-
   static __device__ int ds;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static __constant__ int dc;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static int v;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static const int cv = 1;
   static const __device__ int cds = 1;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static const __constant__ int cdc = 1;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
+
+
+  // __shared__ does not need to be explicitly static.
+  __shared__ int lsi;
+  // __constant__ and __device__ can not be non-static local
+  __constant__ int lci;
+  // expected-error@-1 {{__constant__ and __device__ are not allowed on non-static local variables}}
+  __device__ int ldi;
+  // expected-error@-1 {{__constant__ and __device__ are not allowed on non-static local variables}}
+
+  // Same test cases as for the globals above.
+
+  static __device__ int d_v_f = f();
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ int s_v_f = f();
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ int c_v_f = f();
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __shared__ T s_t_i = {2};
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __device__ T d_t_i = {2};
+  static __constant__ T c_t_i = {2};
+
+  static __device__ ECD d_ecd_i;
+  static __shared__ ECD s_ecd_i;
+  static __constant__ ECD c_ecd_i;
+
+  static __device__ EC d_ec_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ EC s_ec_i(3);
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ EC c_ec_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __device__ EC d_ec_i2 = {3};
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ EC s_ec_i2 = {3};
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static 

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Artem Belevich via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf38a9e51178a: [CUDA] Allow local static variables with 
target attributes. (authored by tra).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
  clang/test/SemaCUDA/bad-attributes.cu
  clang/test/SemaCUDA/device-var-init.cu

Index: clang/test/SemaCUDA/device-var-init.cu
===
--- clang/test/SemaCUDA/device-var-init.cu
+++ clang/test/SemaCUDA/device-var-init.cu
@@ -24,6 +24,12 @@
 
 __shared__ T s_t_i = {2};
 // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+__device__ T d_t_i = {2};
+__constant__ T c_t_i = {2};
+
+__device__ ECD d_ecd_i{};
+__shared__ ECD s_ecd_i{};
+__constant__ ECD c_ecd_i{};
 
 __device__ EC d_ec_i(3);
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
@@ -196,34 +202,218 @@
 __constant__ T_FA_NED c_t_fa_ned;
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 
-// Verify that only __shared__ local variables may be static on device
-// side and that they are not allowed to be initialized.
+// Verify that local variables may be static on device
+// side and that they conform to the initialization constraints.
+// __shared__ can't be initialized at all and others don't support dynamic initialization.
 __device__ void df_sema() {
-  static __shared__ NCFS s_ncfs;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-  static __shared__ UC s_uc;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-  static __shared__ NED s_ned;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-
   static __device__ int ds;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static __constant__ int dc;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static int v;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static const int cv = 1;
   static const __device__ int cds = 1;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static const __constant__ int cdc = 1;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
+
+
+  // __shared__ does not need to be explicitly static.
+  __shared__ int lsi;
+  // __constant__ and __device__ can not be non-static local
+  __constant__ int lci;
+  // expected-error@-1 {{__constant__ and __device__ are not allowed on non-static local variables}}
+  __device__ int ldi;
+  // expected-error@-1 {{__constant__ and __device__ are not allowed on non-static local variables}}
+
+  // Same test cases as for the globals above.
+
+  static __device__ int d_v_f = f();
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ int s_v_f = f();
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ int c_v_f = f();
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __shared__ T s_t_i = {2};
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __device__ T d_t_i = {2};
+  static __constant__ T c_t_i = {2};
+
+  static __device__ ECD d_ecd_i;
+  static __shared__ ECD s_ecd_i;
+  static __constant__ ECD c_ecd_i;
+
+  static __device__ EC d_ec_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ EC s_ec_i(3);
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ EC c_ec_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __device__ EC d_ec_i2 = {3};
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ EC s_ec_i2 = 

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8163
 "%select{__device__|__global__|__host__|__host__ __device__}0 functions">;
-def err_cuda_nonglobal_constant : Error<"__constant__ variables must be 
global">;
+def err_cuda_nonstatic_constdev: Error<"__constant__ and __device__ are not 
allowed on non-static local variables">;
 def err_cuda_ovl_target : Error<

jlebar wrote:
> `__device__` is not allowed on non-static function-scope variables?
> 
> This appears to be more restrictive than we were before.  I want to check, 
> are we OK with the possibility that this will break user code?  
> https://gcc.godbolt.org/z/Y85GKe work with clang, though not with nvcc.
> 
> I notice that we even allow `__device__ int x;` in `__host__ __device__` 
> functions, which is...questionable.  :)  https://gcc.godbolt.org/z/GjjMGx
> 
> I'm OK matching the nvcc behavior here and accepting user breakage so long as 
> we're intentional about it.  Possibly should be called out in relnotes?
It appears to have been an oversight. AFAICT, we just ignored the `__device__` 
attribute of the local vars that sneaked past the `isStaticLocal()` check.




Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4353
   const auto *VD = cast(D);
-  if (!VD->hasGlobalStorage()) {
-S.Diag(AL.getLoc(), diag::err_cuda_nonglobal_constant);
+  if (VD->hasLocalStorage()) {
+S.Diag(AL.getLoc(), diag::err_cuda_nonstatic_constdev);

jlebar wrote:
> So just to check, in our new world, `__constant__` variables don't have to be 
> const.  That matches nvcc, fine.
No. IMO, it's similar to explicitly putting a non-const variable into a 
`.rodata` section -- inadvisable, probably not very useful, and possibly 
implementation-defined, but not illegal.





Comment at: clang/test/CodeGenCUDA/static-device-var-no-rdc.cu:84
+  const static __constant__ int local_static_constant = 42;
+  const static __device__ int local_static_device = 43;
   a[0] = x;

tra wrote:
> yaxunl wrote:
> > what happens to a const static device or constant var with non-trivial 
> > constructor? can we have a test for that?
> I believe constructor trivialness check is orthogonal and will still be 
> applied.
> I'll add a test.
This is checked in `SemaCUDA/device-var-init.cu`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Artem Belevich via Phabricator via cfe-commits]

tra updated this revision to Diff 302394.
tra added a comment.

Added few test cases for allowed initializers on device-side static vars.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
  clang/test/SemaCUDA/bad-attributes.cu
  clang/test/SemaCUDA/device-var-init.cu

Index: clang/test/SemaCUDA/device-var-init.cu
===
--- clang/test/SemaCUDA/device-var-init.cu
+++ clang/test/SemaCUDA/device-var-init.cu
@@ -24,6 +24,12 @@
 
 __shared__ T s_t_i = {2};
 // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+__device__ T d_t_i = {2};
+__constant__ T c_t_i = {2};
+
+__device__ ECD d_ecd_i{};
+__shared__ ECD s_ecd_i{};
+__constant__ ECD c_ecd_i{};
 
 __device__ EC d_ec_i(3);
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
@@ -196,34 +202,218 @@
 __constant__ T_FA_NED c_t_fa_ned;
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 
-// Verify that only __shared__ local variables may be static on device
-// side and that they are not allowed to be initialized.
+// Verify that local variables may be static on device
+// side and that they conform to the initialization constraints.
+// __shared__ can't be initialized at all and others don't support dynamic initialization.
 __device__ void df_sema() {
-  static __shared__ NCFS s_ncfs;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-  static __shared__ UC s_uc;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-  static __shared__ NED s_ned;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-
   static __device__ int ds;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static __constant__ int dc;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static int v;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static const int cv = 1;
   static const __device__ int cds = 1;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static const __constant__ int cdc = 1;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
+
+
+  // __shared__ does not need to be explicitly static.
+  __shared__ int lsi;
+  // __constant__ and __device__ can not be non-static local
+  __constant__ int lci;
+  // expected-error@-1 {{__constant__ and __device__ are not allowed on non-static local variables}}
+  __device__ int ldi;
+  // expected-error@-1 {{__constant__ and __device__ are not allowed on non-static local variables}}
+
+  // Same test cases as for the globals above.
+
+  static __device__ int d_v_f = f();
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ int s_v_f = f();
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ int c_v_f = f();
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __shared__ T s_t_i = {2};
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __device__ T d_t_i = {2};
+  static __constant__ T c_t_i = {2};
+
+  static __device__ ECD d_ecd_i;
+  static __shared__ ECD s_ecd_i;
+  static __constant__ ECD c_ecd_i;
+
+  static __device__ EC d_ec_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ EC s_ec_i(3);
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ EC c_ec_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __device__ EC d_ec_i2 = {3};
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ EC s_ec_i2 = {3};
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static 

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Justin Lebar via Phabricator via cfe-commits]

jlebar accepted this revision.
jlebar added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8163
 "%select{__device__|__global__|__host__|__host__ __device__}0 functions">;
-def err_cuda_nonglobal_constant : Error<"__constant__ variables must be 
global">;
+def err_cuda_nonstatic_constdev: Error<"__constant__ and __device__ are not 
allowed on non-static local variables">;
 def err_cuda_ovl_target : Error<

`__device__` is not allowed on non-static function-scope variables?

This appears to be more restrictive than we were before.  I want to check, are 
we OK with the possibility that this will break user code?  
https://gcc.godbolt.org/z/Y85GKe work with clang, though not with nvcc.

I notice that we even allow `__device__ int x;` in `__host__ __device__` 
functions, which is...questionable.  :)  https://gcc.godbolt.org/z/GjjMGx

I'm OK matching the nvcc behavior here and accepting user breakage so long as 
we're intentional about it.  Possibly should be called out in relnotes?



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4353
   const auto *VD = cast(D);
-  if (!VD->hasGlobalStorage()) {
-S.Diag(AL.getLoc(), diag::err_cuda_nonglobal_constant);
+  if (VD->hasLocalStorage()) {
+S.Diag(AL.getLoc(), diag::err_cuda_nonstatic_constdev);

So just to check, in our new world, `__constant__` variables don't have to be 
const.  That matches nvcc, fine.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Justin Lebar via Phabricator via cfe-commits]

jlebar added a comment.

Hey, I'm leaving on a vacation tomorrow and didn't have a chance to get to
this review today.

Is that ok?  I'm not bringing my work laptop, but I could look at it on my
personal laptop.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/test/SemaCUDA/device-var-init.cu:404
 __host__ __device__ void hd_sema() {
   static int x = 42;
 }

yaxunl wrote:
> tra wrote:
> > yaxunl wrote:
> > > how does this work in device compilation? Is this equivalent to `static 
> > > __device__ int x = 42`?
> > Correct. 
> so static variable without `__device__/__constant__` attribute in host device 
> function implies `__device__` attribute in device compilation.
> 
> Is this also true in device function? We need Sema and CodeGen tests for 
> these cases.
> 
> Also, can we document these changes? It is easily forgotten.
I think of it as a static variable in a `__device__` function. There should be 
no host-side shadow for it, which would normally be created for a `__device__` 
variable.

The tests at the beginning of df_sema() in SemaCUDA/device-var-init.cu already 
check that static w/o attributes is allowed in `__device__`.
I'll update CodeGen tests to verify that we generate correct code.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/test/SemaCUDA/device-var-init.cu:404
 __host__ __device__ void hd_sema() {
   static int x = 42;
 }

tra wrote:
> yaxunl wrote:
> > how does this work in device compilation? Is this equivalent to `static 
> > __device__ int x = 42`?
> Correct. 
so static variable without `__device__/__constant__` attribute in host device 
function implies `__device__` attribute in device compilation.

Is this also true in device function? We need Sema and CodeGen tests for these 
cases.

Also, can we document these changes? It is easily forgotten.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

I've verified that clang with this patch can compile Tensorflow and that it can 
also compile `cooperative_groups.h` from CUDA-11.




Comment at: clang/test/SemaCUDA/device-var-init.cu:404
 __host__ __device__ void hd_sema() {
   static int x = 42;
 }

yaxunl wrote:
> how does this work in device compilation? Is this equivalent to `static 
> __device__ int x = 42`?
Correct. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/test/SemaCUDA/device-var-init.cu:404
 __host__ __device__ void hd_sema() {
   static int x = 42;
 }

how does this work in device compilation? Is this equivalent to `static 
__device__ int x = 42`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Artem Belevich via Phabricator via cfe-commits]

tra updated this revision to Diff 295636.
tra added a comment.
Herald added a reviewer: jdoerfert.

Fixed a test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
  clang/test/SemaCUDA/bad-attributes.cu
  clang/test/SemaCUDA/device-var-init.cu

Index: clang/test/SemaCUDA/device-var-init.cu
===
--- clang/test/SemaCUDA/device-var-init.cu
+++ clang/test/SemaCUDA/device-var-init.cu
@@ -196,34 +196,212 @@
 __constant__ T_FA_NED c_t_fa_ned;
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 
-// Verify that only __shared__ local variables may be static on device
-// side and that they are not allowed to be initialized.
+// Verify that local variables may be static on device
+// side and that they conform to the initialization constraints.
+// __shared__ can't be initialized at all and others don't support dynamic initialization.
 __device__ void df_sema() {
-  static __shared__ NCFS s_ncfs;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-  static __shared__ UC s_uc;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-  static __shared__ NED s_ned;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-
   static __device__ int ds;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static __constant__ int dc;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static int v;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static const int cv = 1;
   static const __device__ int cds = 1;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static const __constant__ int cdc = 1;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
+
+
+  // __shared__ does not need to be explicitly static.
+  __shared__ int lsi;
+  // __constant__ and __device__ can not be non-static local
+  __constant__ int lci;
+  // expected-error@-1 {{__constant__ and __device__ are not allowed on non-static local variables}}
+  __device__ int ldi;
+  // expected-error@-1 {{__constant__ and __device__ are not allowed on non-static local variables}}
+  
+  // Same test cases as for the globals above.
+
+  static __device__ int d_v_f = f();
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ int s_v_f = f();
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ int c_v_f = f();
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __shared__ T s_t_i = {2};
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+
+  static __device__ EC d_ec_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ EC s_ec_i(3);
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ EC c_ec_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __device__ EC d_ec_i2 = {3};
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ EC s_ec_i2 = {3};
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ EC c_ec_i2 = {3};
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __device__ ETC d_etc_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ ETC s_etc_i(3);
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ ETC c_etc_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __device__ ETC 

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Artem Belevich via Phabricator via cfe-commits]

tra updated this revision to Diff 295635.
tra retitled this revision from "[CUDA] Allow local `static {__constant__, 
__device__}` variables." to "[CUDA] Allow local `static const {__constant__, 
__device__}` variables.".
tra edited the summary of this revision.
tra added a comment.
Herald added a reviewer: aaron.ballman.

Further relaxed application of attributes on local static variables.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
  clang/test/SemaCUDA/device-var-init.cu

Index: clang/test/SemaCUDA/device-var-init.cu
===
--- clang/test/SemaCUDA/device-var-init.cu
+++ clang/test/SemaCUDA/device-var-init.cu
@@ -196,34 +196,212 @@
 __constant__ T_FA_NED c_t_fa_ned;
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 
-// Verify that only __shared__ local variables may be static on device
-// side and that they are not allowed to be initialized.
+// Verify that local variables may be static on device
+// side and that they conform to the initialization constraints.
+// __shared__ can't be initialized at all and others don't support dynamic initialization.
 __device__ void df_sema() {
-  static __shared__ NCFS s_ncfs;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-  static __shared__ UC s_uc;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-  static __shared__ NED s_ned;
-  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-
   static __device__ int ds;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static __constant__ int dc;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static int v;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static const int cv = 1;
   static const __device__ int cds = 1;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
   static const __constant__ int cdc = 1;
-  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
+
+
+  // __shared__ does not need to be explicitly static.
+  __shared__ int lsi;
+  // __constant__ and __device__ can not be non-static local
+  __constant__ int lci;
+  // expected-error@-1 {{__constant__ and __device__ are not allowed on non-static local variables}}
+  __device__ int ldi;
+  // expected-error@-1 {{__constant__ and __device__ are not allowed on non-static local variables}}
+  
+  // Same test cases as for the globals above.
+
+  static __device__ int d_v_f = f();
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ int s_v_f = f();
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ int c_v_f = f();
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __shared__ T s_t_i = {2};
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+
+  static __device__ EC d_ec_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ EC s_ec_i(3);
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ EC c_ec_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __device__ EC d_ec_i2 = {3};
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ EC s_ec_i2 = {3};
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  static __constant__ EC c_ec_i2 = {3};
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+  static __device__ ETC d_etc_i(3);
+  // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+  static __shared__ ETC s_etc_i(3);
+  // expected-error@-1 

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Justin Lebar via Phabricator via cfe-commits]

jlebar added a comment.

> It should. I did mention in a previous comment that > Looks like the 
> const-ness check should not be there, either. I need to revise the patch.

Heh, okay.  Sorry I missed that, somehow this patch was confusing to me.

> Except that NVCC allows non-const __constant__, too. Generally speaking, C++ 
> does not care about the attributes. While technically __constant__ is not 
> changeable from the device code, not specifying const is a missed 
> optimization/diagnostic opportunity, but not an error per se. It does not 
> affect how the variable is emitted, but rather what user can do with it and 
> that's beyond the scope of this patch. I don't think it warrants a hard 
> error. A warning, perhaps, that non-const __constant__ is probably an error?

Sure, that makes sense to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D88345#2298879 , @jlebar wrote:

> OK, now I'm starting to I understand this change..
>
> Before, in function scope, we allow static const/non-const `__shared__`, and 
> allow static const so long as it's not `__device__` or `__constant__`.
>
> - `static` -> error?  (I understood us saying above that it is, but now that 
> I read the code, isn't it saying it's an error?)
> - `static const` -> allowed
> - `static __device__` -> error
> - `static const __device__` -> error
> - `static __constant__` -> error
> - `static const __constant__` -> error
>
> After, in function scope, the rule is, allow static const/non-const 
> `__shared__` or anything that's `static const`.
>
> - `static` -> error, must be const
> - `static const` -> allowed
> - `static __device__` -> error, must be const
> - `static const __device__` -> allowed
> - `static __constant__` -> error, must be const
> - `static const __constant__` -> allowed
>
> I guess my question when I write out this table is, why shouldn't it be like 
> this?
>
> - `static` -> allowed
> - `static const` -> allowed
> - `static __device__` -> allowed
> - `const static __device__` -> allowed
> - `static __constant__` -> error, must be const
> - `const static __constant__` -> allowed

It should. I did mention in a previous comment that `> Looks like the 
const-ness check should not be there, either`.
I need to revise the patch.

> This makes some sense to me because we're saying, "`__constant__` must be 
> const", otherwise, anything goes.

Except that NVCC allows non-const `__constant__`, too. Generally speaking, C++ 
does not care about the attributes. While technically `__constant__` is not 
changeable from the device code, not specifying `const` is a missed 
optimization/diagnostic opportunity, but not an error per se. It does not 
affect how the variable is emitted, but rather what user can do with it and 
that's beyond the scope of this patch. I don't think it warrants a hard error. 
A warning, perhaps, that non-const `__constant__` is probably an error?

> Or here's another way of thinking about it.  You're saying that `static` and 
> `static __device__` in function scope are the same as a `__device__` variable 
> in block scope.  And a `__device__` variable in block scope doesn't have to 
> be const (right?).  So why the extra restriction on function-scope static?

Something like that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Justin Lebar via Phabricator via cfe-commits]

jlebar added a comment.

OK, now I'm starting to I understand this change..

Before, in function scope, we allow static const/non-const `__shared__`, and 
allow static const so long as it's not `__device__` or `__constant__`.

- `static` -> error?  (I understood us saying above that it is, but now that I 
read the code, isn't it saying it's an error?)
- `static const` -> allowed
- `static __device__` -> error
- `static const __device__` -> error
- `static __constant__` -> error
- `static const __constant__` -> error

After, in function scope, the rule is, allow static const/non-const 
`__shared__` or anything that's `static const`.

- `static` -> error, must be const
- `static const` -> allowed
- `static __device__` -> error, must be const
- `static const __device__` -> allowed
- `static __constant__` -> error, must be const
- `static const __constant__` -> allowed

I guess my question when I write out this table is, why shouldn't it be like 
this?

- `static` -> allowed
- `static const` -> allowed
- `static __device__` -> allowed
- `const static __device__` -> allowed
- `static __constant__` -> error, must be const
- `const static __constant__` -> allowed

This makes some sense to me because we're saying, "`__constant__` must be 
const", otherwise, anything goes.

Or here's another way of thinking about it.  You're saying that `static` and 
`static __device__` in function scope are the same as a `__device__` variable 
in block scope.  And a `__device__` variable in block scope doesn't have to be 
const (right?).  So why the extra restriction on function-scope static?




Comment at: clang/lib/Sema/SemaDecl.cpp:13161
   // without device memory qualifier is implemented, therefore only static
   // const variable without device memory qualifier is allowed.
   [&]() {

Update comment?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D88345#2298688 , @jlebar wrote:

> OK, backing up, what are the semantics of `static` on `__constant__`, 
> `__device__`, and `__shared__`?
>
> - My understanding is that `__shared__` behaves the same whether or not it's 
> static.  It's not equivalent to `namespace a { __shared__ int c = 4; }`, 
> because that's illegal.

Yes. `__shared__` is an odd duck. It is implicitly static, so whether we 
explicitly specify `static` makes no difference.
We're not changing anything about how it's implemented.

> - Does `__constant__` behave the same whether or not it's static?  A static 
> `__constant__` is equivalent to `namespace a { __constant__ int c = 4; }`, 
> and a non-static `__constant__` is *also* equivalent to that?

No. `__constant__` is not allowed on non-static local variables as it can't be 
allocated on stack.

> - And `__device__` does not behave the same whether or not it's static?

Correct.

> In function scope `__device__ int x;` is a variable local to the thread.

Correct. `__device__` in a device function is effectively a no-op and can be 
placed on stack as a regular local variable.

> Whereas in global scope `__device__ int x;` is a global variable that lives 
> in device memory (?).

Correct.

> In function scope `static __device__ int x;` is equivalent to `static int x;` 
> which is equivalent to `int x;` in namespace scope?

Yes, assuming you mean a `__device__` function and `__device__ int x;` in the 
namespace scope.

> Should we mandate that you initialize `static __constant__` variables in 
> function scope?
> That is, if you write `static __constant__ int x;` in a function, then x is 
> always uninitialized (right)?  You should do `static __constant__ int x = 
> 42;`?

No. Accoring to PTX spec:  `Variables in .const and .global state spaces are 
initialized to zero by default.`
Those are the address spaces `__constant__` and `__device__` variables map to.
https://docs.nvidia.com/cuda/parallel-thread-execution/index.html#state-spaces


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Justin Lebar via Phabricator via cfe-commits]

jlebar added a comment.

OK, backing up, what are the semantics of `static` on `__constant__`, 
`__device__`, and `__shared__`?

- My understanding is that `__shared__` behaves the same whether or not it's 
static.  It's not equivalent to `namespace a { __shared__ int c = 4; }`, 
because that's illegal.
- Does `__constant__` behave the same whether or not it's static?  A static 
`__constant__` is equivalent to `namespace a { __constant__ int c = 4; }`, and 
a non-static `__constant__` is *also* equivalent to that?
- And `__device__` does not behave the same whether or not it's static?  In 
function scope `__device__ int x;` is a variable local to the thread.  Whereas 
in global scope `__device__ int x;` is a global variable that lives in device 
memory (?).  In function scope `static __device__ int x;` is equivalent to 
`static int x;` which is equivalent to `int x;` in namespace scope?

Should we mandate that you initialize `static __constant__` variables in 
function scope?  That is, if you write `static __constant__ int x;` in a 
function, then x is always uninitialized (right)?  You should do `static 
__constant__ int x = 42;`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D88345#2296118 , @jlebar wrote:

> wha... As you know, `const` doesn't mean anything, that can be const-casted 
> away.  And then you'll be able to observe that this nominally-static variable 
> is just a normal variable.

Yes, I'm aware of that. It is irrelevant in this case. 
The point is that `__constant__` and `__device__` are not allowed for the local 
`static` variables. I believe originally only `__shared__` was allowed for 
local static vars is because `__shared__` is already implicitly static, while 
`__constant__` and `__device__` are not.

On the other hand, nothing stops us **giving** `static __constant__` of 
`__device__` variables static storage class and that's what NVIDIA has 
apparently done, though without updating the docs.
The checks on the constructor still apply -- trivialness is enforced, `const` 
must have an initializer, etc.

Looks like the const-ness check should not be there, either. NVCC allows 
non-const statics. We may get rid of this check altogether and allow everything.

> Since this doesn't make sense and contradicts their documentation, I'm 
> tempted to say this should only apply to the nvidia headers.  Is that 
> technically possible?  And then we file a bug against nvidia and/or ask Bryce?

IMO the relaxation is sensible. A `static` var is just a global var in an odd 
namespace and we do support that.
E.g. these produce the same PTX for `c` (modulo mangling of the names):

  namespace a {
__constant__ int c = 4;
  }
  __device__ void foo() {
static __constant__ int c = 4;
  }






Comment at: clang/test/CodeGenCUDA/static-device-var-no-rdc.cu:84
+  const static __constant__ int local_static_constant = 42;
+  const static __device__ int local_static_device = 43;
   a[0] = x;

yaxunl wrote:
> what happens to a const static device or constant var with non-trivial 
> constructor? can we have a test for that?
I believe constructor trivialness check is orthogonal and will still be applied.
I'll add a test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/test/CodeGenCUDA/static-device-var-no-rdc.cu:84
+  const static __constant__ int local_static_constant = 42;
+  const static __device__ int local_static_device = 43;
   a[0] = x;

what happens to a const static device or constant var with non-trivial 
constructor? can we have a test for that?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88345: [CUDA] Allow local `static const {__constant__, __device__}` variables.

[Justin Lebar via Phabricator via cfe-commits]

jlebar added a comment.

wha... As you know, `const` doesn't mean anything, that can be const-casted 
away.  And then you'll be able to observe that this nominally-static variable 
is just a normal variable.

Since this doesn't make sense and contradicts their documentation, I'm tempted 
to say this should only apply to the nvidia headers.  Is that technically 
possible?  And then we file a bug against nvidia and/or ask Bryce?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88345/new/

https://reviews.llvm.org/D88345

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits