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

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 LA

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

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/DiagnosticSemaKi

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

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 LA

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

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

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

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/includ

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

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_

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

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.ll

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

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 `st

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

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 > > __devic

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

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; }

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

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

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

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/Diagnostic

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

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

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

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__, t

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

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 `__const

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

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

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

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.

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

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 ill

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

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 a

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

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 devic

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

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 thi