t-tye added inline comments.

================
Comment at: include/clang/Basic/Cuda.h:49-57
+  GFX700,
+  GFX701,
+  GFX800,
+  GFX801,
+  GFX802,
+  GFX803,
+  GFX810,
----------------
Should complete list of processors for the amdgcn architecture be included? See 
https://llvm.org/docs/AMDGPUUsage.html#processors .


================
Comment at: include/clang/Basic/Cuda.h:79
   COMPUTE_72,
+  COMPUTE_GCN,
 };
----------------
Suggest using amdgcn which matches the architecture name in 
https://llvm.org/docs/AMDGPUUsage.html#processors .


================
Comment at: lib/Basic/Targets/AMDGPU.h:85
     return TT.getEnvironmentName() == "amdgiz" ||
+           TT.getOS() == llvm::Triple::CUDA ||
            TT.getEnvironmentName() == "amdgizcl";
----------------
We still want to use the amdhsa OS for amdgpu which currently supports the 
different environments. So can cuda simply support the same environments? Is 
the plan is to eliminate the environments and simply always use the default 
address space for generic so this code is no longer needed?


https://reviews.llvm.org/D42800



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to