[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

tra added a comment. This triggers an assertion: clang: /usr/local/google/home/tra/work/llvm/repo/clang/lib/AST/Decl.cpp:2697: clang::Expr *clang::ParmVarDecl::getDefaultArg(): Assertion `!hasUninstantiatedDefaultArg() && "Default argument is not yet instantiated!"' failed. #2 0x7f

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

hliao added a comment. In D79344#2026349 , @tra wrote: > Here's a slightly smaller variant which may be a good clue for tracking down > the root cause. This one fails with: > > var.cc:6:14: error: no matching function for call to 'copysign' > double

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

tra added a comment. Here's a slightly smaller variant which may be a good clue for tracking down the root cause. This one fails with: var.cc:6:14: error: no matching function for call to 'copysign' double g = copysign(0, g); ^~~~ var.cc:5:56: note: candidate template

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

tra added a comment. In D79344#2026180 , @tra wrote: > The problem is reproducible in upstream clang. Let's see if I can reduce it > to something simpler. Reduced it down to this -- compiles with clang w/o the patch, but fails with it. __attribute__(

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

hliao added a comment. In D79344#2026180 , @tra wrote: > The problem is reproducible in upstream clang. Let's see if I can reduce it > to something simpler. I remembered found similar errors when the math part is refactored out into the current but, la

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

tra added a comment. The problem is reproducible in upstream clang. Let's see if I can reduce it to something simpler. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79344/new/ https://reviews.llvm.org/D79344

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

tra added a comment. In D79344#2026126 , @hliao wrote: > In D79344#2026025 , @tra wrote: > > > We're calling `copysign( int, double)`. The standard library provides > > `copysign(double, double)`, CUDA provides onl

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

hliao added a comment. In D79344#2026025 , @tra wrote: > We're calling `copysign( int, double)`. The standard library provides > `copysign(double, double)`, CUDA provides only `copysign(float, double)`. As > far as C++ is concerned, both require one typ

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

tra added a comment. We're calling `copysign( int, double)`. The standard library provides `copysign(double, double)`, CUDA provides only `copysign(float, double)`. As far as C++ is concerned, both require one type conversion. I guess previously we would give `__device__` one provided by CUDA

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

tra added a comment. In D79344#2018915 , @tra wrote: > If you can wait, I can try patching this change into our clang tree and then > see if it breaks anything obvious. If nothing falls apart, I'll be fine with > the patch as is. The patch appears to b

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

tra added a comment. In D79344#2018693 , @hliao wrote: > OK, I will add one option, But, do we turn it on by default or off? As a rule of thumb, if it's an experimental feature, then the default would be off. For a change which should be the default, bu

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

hliao added a comment. In D79344#2018683 , @tra wrote: > In D79344#2018628 , @hliao wrote: > > > In D79344#2018561 , @tra wrote: > > > > > This has a good chance of breaking

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

tra added a comment. In D79344#2018628 , @hliao wrote: > In D79344#2018561 , @tra wrote: > > > This has a good chance of breaking existing code. It would be great to add > > an escape hatch option to revert to the

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

hliao marked an inline comment as done. hliao added inline comments. Comment at: clang/lib/Sema/SemaCUDA.cpp:156 + if (VD->getType()->isCUDADeviceBuiltinSurfaceType() || + VD->getType()->isCUDADeviceBuiltinTextureType()) +return CFT_HostDevice; yaxunl w

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

yaxunl added inline comments. Comment at: clang/test/SemaCUDA/variable-target.cu:42 + return 0; +} hliao wrote: > yaxunl wrote: > > we need to have a test to check captured local host variable is allowed in > > device lambda. > > > > we need to have some test

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

hliao marked an inline comment as done. hliao added inline comments. Comment at: clang/test/SemaCUDA/variable-target.cu:6 + +static int gvar; +// expected-note@-1{{'gvar' declared here}} tra wrote: > The current set of tests only verifies access of host variable

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

hliao marked an inline comment as done. hliao added inline comments. Comment at: clang/test/SemaCUDA/variable-target.cu:42 + return 0; +} yaxunl wrote: > we need to have a test to check captured local host variable is allowed in > device lambda. > > we need to

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

hliao added a comment. In D79344#2018561 , @tra wrote: > This has a good chance of breaking existing code. It would be great to add an > escape hatch option to revert to the old behavior if we run into problems. > The change is relatively simple, so reve

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

tra added a comment. This has a good chance of breaking existing code. It would be great to add an escape hatch option to revert to the old behavior if we run into problems. The change is relatively simple, so reverting it in case something goes wrong should work, too. Up to you. ===

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

yaxunl added inline comments. Comment at: clang/lib/Sema/SemaCUDA.cpp:156 + if (VD->getType()->isCUDADeviceBuiltinSurfaceType() || + VD->getType()->isCUDADeviceBuiltinTextureType()) +return CFT_HostDevice; We may need to mark constexpr variables as host

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

hliao updated this revision to Diff 261904. hliao added a comment. Reformatting test code following pre-merge checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79344/new/ https://reviews.llvm.org/D79344 Files: clang/include/clang/Basic/Diagn

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

hliao added a comment. That test code just passed compilation on clang trunk if only assembly code is generated, https://godbolt.org/z/XYjRcT. But NVCC generates errors on all cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79344/new/ https:/

[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

hliao created this revision. hliao added reviewers: tra, rjmccall, yaxunl. Herald added a project: clang. Herald added a subscriber: cfe-commits. hliao added a comment. That test code just passed compilation on clang trunk if only assembly code is generated, https://godbolt.org/z/XYjRcT. But NVCC