[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