yaxunl marked an inline comment as done. yaxunl added inline comments.
================ Comment at: lib/Basic/Cuda.cpp:113 + case CudaArch::GFX906_SRAM_ECC: // TBA + return "gfx906+sram-ecc"; case CudaArch::GFX909: // TBA ---------------- tra wrote: > Wording nit: > Does it mean `+(SRAM, ECC)` or `+SRAM, -ECC` ? > > From the rest of the changes I guess it's the former, but the syntax used can > easily be interpreted as the later. > > Perhaps change the feature name to `sram_ecc` and GPU name to something less > verbose. `gfx906se` ? It actually means +(SRAM-ECC). There is actually documentation about what features are supported and their format https://llvm.org/docs/AMDGPUUsage.html#code-object-target-identification https://llvm.org/docs/AMDGPUUsage.html#amdgpu-target-features Since the name has been used in runtime, we cannot change that. Not all combinations are useful, therefore this patch only supports the one that is known to be useful. I am considering a more generic approach, but so far I do not see its necessity yet. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59863/new/ https://reviews.llvm.org/D59863 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
