Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

tra updated this revision to Diff 46696. tra marked 8 inline comments as done. tra added a comment. Addressed Richard's comments. Relaxed restrictions a bit to allow constant initializers even those CUDA would not considered to be empty. Updated test case accordingly.

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

This revision was automatically updated to reflect the committed changes. Closed by commit rL259592: [CUDA] Do not allow dynamic initialization of global device side variables. (authored by tra). Changed prior to commit: http://reviews.llvm.org/D15305?vs=46696=46707#toc Repository: rL LLVM

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

tra added inline comments. Comment at: lib/Sema/SemaCUDA.cpp:429-430 @@ +428,4 @@ + CXXConstructorDecl *CD) { + if (!CD->isDefined() && CD->isTemplateInstantiation()) +InstantiateFunctionDefinition(VarLoc, CD->getFirstDecl()); +

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

Richard, On Fri, Jan 15, 2016 at 5:32 PM, Richard Smith wrote: > On Fri, Jan 15, 2016 at 5:29 PM, Richard Smith > wrote: > > On Fri, Jan 15, 2016 at 4:22 PM, Artem Belevich wrote: > >> tra added inline comments. > >> > >>

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Some minor things, but feel free to commit after addressing them. I agree that we should figure out what to do about the zero/undef initialization separately. Comment at:

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

jpienaar added a comment. @jlebar: We defer it to your and Richard's approval. Thanks http://reviews.llvm.org/D15305 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

jlebar added a comment. jingyue/jpienaar/rsmith - friendly ping? Without this, -O0 builds don't work, because they emit empty global initializers that don't get optimized out. http://reviews.llvm.org/D15305 ___ cfe-commits mailing list

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

tra marked 3 inline comments as done. Comment at: lib/Sema/SemaCUDA.cpp:436 @@ +435,3 @@ + if (CD->isTrivial()) +return true; + jlebar wrote: > The test passes if I comment out this if statement. I'm not sure if that's > expected; this may or may not be

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

tra updated this revision to Diff 45312. tra marked 2 inline comments as done. tra added a comment. Addressed Justin's comments. http://reviews.llvm.org/D15305 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/CodeGen/CGDeclCXX.cpp

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

jlebar added a comment. tra asked me to check for coverage. Looks pretty good in that respect. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6419 @@ +6418,3 @@ +"dynamic initialization is not supported for " +"__device__, __constant__ and __shared__

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

On Fri, Jan 15, 2016 at 5:29 PM, Richard Smith wrote: > On Fri, Jan 15, 2016 at 4:22 PM, Artem Belevich wrote: >> tra added inline comments. >> >> >> Comment at: lib/CodeGen/CodeGenModule.cpp:2334 >> @@ -2339,1 +2333,3 @@ >> +

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

On Fri, Jan 15, 2016 at 4:22 PM, Artem Belevich wrote: > tra added inline comments. > > > Comment at: lib/CodeGen/CodeGenModule.cpp:2334 > @@ -2339,1 +2333,3 @@ > + D->hasAttr()) > Init = llvm::UndefValue::get(getTypes().ConvertType(ASTTy)); > + else

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

tra added a reviewer: jlebar. tra updated this revision to Diff 45044. tra added a comment. Moved initializer checks from CodeGen to Sema. Added test cases for initializers of non-class variables. http://reviews.llvm.org/D15305 Files: include/clang/Basic/DiagnosticSemaKinds.td

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

tra marked an inline comment as done. tra added a comment. In http://reviews.llvm.org/D15305#327226, @rsmith wrote: > I think you missed this from my previous review: > > > This should be checked and diagnosed in Sema, not in CodeGen. > Done. http://reviews.llvm.org/D15305

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

rsmith added inline comments. Comment at: lib/CodeGen/CGDeclCXX.cpp:312 @@ +311,3 @@ + // the checks have been done in Sema by now. Whatever initializers + // areallowed are empty and we just need to ignore them here. + if (getLangOpts().CUDA && getLangOpts().CUDAIsDevice &&

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

tra updated this revision to Diff 45051. tra marked an inline comment as done. tra added a comment. Typo fix. http://reviews.llvm.org/D15305 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/CodeGen/CGDeclCXX.cpp lib/CodeGen/CodeGenModule.cpp

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

tra added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:2334 @@ -2339,1 +2333,3 @@ + D->hasAttr()) Init = llvm::UndefValue::get(getTypes().ConvertType(ASTTy)); + else if (!InitExpr) { rsmith wrote: > As this is a global variable, it should

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

rsmith added a comment. I think you missed this from my previous review: > This should be checked and diagnosed in Sema, not in CodeGen. Comment at: lib/CodeGen/CGDeclCXX.cpp:333-337 @@ +332,7 @@ + [](const CXXMethodDecl *Method) { return Method->isVirtual(); })) +

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

tra updated this revision to Diff 44687. tra added a comment. Check all variable initializers and only allow 'empty constructors' as Richard has suggested. Changed test structure so that we test for allowed/disallowed constructors separately from testing how we handle initialization of base

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

tra added a comment. Richard, I've updated the patch as you've suggested -- it indeed simplifies things quite a bit and handles the corner cases you've mentioned. Comment at: lib/CodeGen/CGDeclCXX.cpp:323-324 @@ +322,4 @@ + + // The constructor function has no parameters, +

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

rsmith added a comment. This should be checked and diagnosed in Sema, not in CodeGen. Comment at: lib/CodeGen/CGDeclCXX.cpp:323-324 @@ +322,4 @@ + + // The constructor function has no parameters, + if (CD->getNumParams() != 0) +return false; What if the

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

tra added a comment. ping. http://reviews.llvm.org/D15305 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

tra created this revision. tra added reviewers: rsmith, jingyue, jpienaar. tra added a subscriber: cfe-commits. In general CUDA does not allow dynamic initialization of global device-side variables except for records with empty constructors as described in section [[

Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

tra added inline comments. Comment at: lib/CodeGen/CGDeclCXX.cpp:329 @@ +328,3 @@ + for (const CXXCtorInitializer *CI: CD->inits()) +if (CI->isAnyMemberInitializer() && CI->isWritten()) + return false; @rsmith: is this a good way to find member