[PATCH] D30125: AMDGPU: Merge initial gfx9 support

Konstantin Zhuravlyov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 18 05:23:24 PST 2017


kzhuravl accepted this revision.
kzhuravl added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: lib/Target/AMDGPU/AMDGPU.td:555
 
+// TODO: Either the name to be changed or we simply use IsCI!
 def isCIVI : Predicate <
----------------
Do you think we could switch generation from using islands to using GFX numbers for GCN in the long run? I am not sure for pre-GCN. This can be done in a separate patch and is not an immediate need.

SOUTHERN_ISLANDS = GFX6?
SEA_ISLANDS = GFX7
VOLCANIC_ISLANDS = GFX8

Then we could have isGFX7AndUp, isGFX8AndUp, etc.

Using islands is sometimes confusing (at least to me), and I have to open docs to match those to GFX numbers, or to find out if SEA_ISLANDS comes before or after CI :). GFX numbers are also matching nicely with ISA versions, and it is obvious that GFX7 comes after GFX6.

Also, ROCm docs (at least) use GFX numbers in readmes/manuals:
https://radeonopencompute.github.io/install.html


================
Comment at: lib/Target/AMDGPU/AMDGPU.td:557-558
 def isCIVI : Predicate <
   "Subtarget->getGeneration() == AMDGPUSubtarget::SEA_ISLANDS || "
-  "Subtarget->getGeneration() == AMDGPUSubtarget::VOLCANIC_ISLANDS"
+  "Subtarget->getGeneration() >= AMDGPUSubtarget::VOLCANIC_ISLANDS"
 >, AssemblerPredicate<"FeatureCIInsts">;
----------------
Subtarget->getGeneration() >= AMDGPUSubtarget::SEA_ISLANDS?


https://reviews.llvm.org/D30125





More information about the llvm-commits mailing list