[PATCH] D25139: [CUDA] Add Sema::CUDADiagBuilder and Sema::CUDADiagIfDeviceCode().

[Justin Lebar via cfe-commits]

jlebar added inline comments.


> rnk wrote in Sema.h:9238
> I'm concerned that this usage pattern isn't going to be efficient because you build the complete diagnostic before calling the bool conversion operator to determine that it doesn't need to be emitted. I think you want to construct something more like:
> 
>   if (isCUDADeviceCode())
>     CUDADiag(...) << ...;
> 
> Otherwise you are going to construct and destruct a large number of diagnostics about language features that are forbidden in device code, but are legal in host code, and 99% of the TU is going to be host code that uses these illegal features.

I think the comment is misleading -- I tried to update it to resolve this misunderstanding.  Does it make more sense now?

> rnk wrote in Sema.h:9258
> Remind me why we need to do this? Which arena is this stuff allocated in and where would I go to read more about it? My thought is that, if we don't construct very many of these, we should just allocate them in the usual ASTContext arena and let them live forever. It would be more consistent with our usual way of doing things.

These diagnostics live until the end of codegen, and so are destroyed after the ASTContext.

I am becoming increasingly displeased with emitting these errors during codegen.  In particular, it makes it annoying to write tests.  You cannot test deferred and immediate errors in the same test, because if you have any immediate errors, we never codegen, so we never emit the deferred ones.  This will also be a suboptimal user experience.

The only serious alternative mooted thus far is emitting the deferred errors when a function is marked used.  But this is going to emit deferred errors if e.g. two inline host+device functions have mutual recursion but are otherwise never touched (so don't need to be codegen'ed).  I am not sure if this will be OK or not, but I need to look at it.

If we move the errors to being emitted earlier, we won't need to do this dance.

https://reviews.llvm.org/D25139