[PATCH] D59863: [HIP] Support gpu arch gfx906+sram-ecc

[Yaxun Liu via Phabricator via cfe-commits]

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