[PATCH] D45061: [NVPTX, CUDA] Use custom feature detection to handle NVPTX target builtins.

[Justin Lebar via Phabricator via llvm-commits]

jlebar accepted this revision.
jlebar added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/include/clang/Basic/Cuda.h:55
 
+static inline const std::vector<CudaArch> CudaKnownArchList() {
+  return {CudaArch::SM_20, CudaArch::SM_21, CudaArch::SM_30, CudaArch::SM_32,
----------------
Why 'static'?


================
Comment at: clang/include/clang/Basic/Cuda.h:59
+          CudaArch::SM_53, CudaArch::SM_60, CudaArch::SM_61, CudaArch::SM_62,
+          CudaArch::SM_70, CudaArch::SM_72};
+}
----------------
Could we instead rely on the fact that the CudaArch enum is dense and goes from UNKNOWN to LAST?  One less bug to make...


================
Comment at: clang/lib/Basic/Targets/NVPTX.cpp:182
+Optional<bool>
+NVPTXTargetInfo::hasRequiredFeature(const llvm::StringMap<bool> FeatureMap,
+                                    const StringRef ReqFeature) const {
----------------
Nit, if you spell it `/*FeatureMap*/` here, it'll be clearer that you're intentionally ignoring that arg.


https://reviews.llvm.org/D45061