[PATCH] D11950: [CUDA] Check register names on appropriate side of cuda compilation only.

Artem Belevich via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 19 14:01:56 PDT 2015


tra added inline comments.

================
Comment at: lib/Sema/SemaDecl.cpp:5944
@@ -5943,3 +5943,3 @@
   ProcessDeclAttributes(S, NewVD, D);
-
+  bool ShouldHandleTargetErrors = DeclAttrsMatchCUDAMode(getLangOpts(), NewVD);
   if (getLangOpts().CUDA) {
----------------
eliben wrote:
> Since this is a CUDA-only thing, ShouldHandleTargetErrors can perhaps be named better. The checks you added are very selective now - some diags are disabled, some are not. It's not clear why some fall under the "should handle target error" umbrella. A clearer name like MatchingCUDAMode or something of the sort, may help? 
Considering it's only used in two places, I may as well use DeclAttrsMatchCUDAMode() directly, making variable naming moot and hopefully making intent somewhat clearer.

CUDA code may contain constructs that can only be validated by appropriate TargetInfo() and we currently have access only to one used during current compilation mode. We will check for those errors in another CUDA compilation pass. The error set is further restricted to error classes I've ran into in practice and can reproduce and write tests for.



================
Comment at: lib/Sema/SemaDecl.cpp:5971
@@ -5970,3 +5970,3 @@
   // Handle GNU asm-label extension (encoded as an attribute).
   if (Expr *E = (Expr*)D.getAsmLabel()) {
     // The parser guarantees this is a string.
----------------
eliben wrote:
> Do we plan to support this for CUDA at all? Why not disable here on top?
It does not quite fit virtual registers model used by NVPTX, but as a feature per se I believe we should keep it enabled, because it should be OK to use it for targets where it's valid. For example, using this feature in host code on x86 should be acceptable.


http://reviews.llvm.org/D11950





More information about the cfe-commits mailing list