This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Merge initial gfx9 support
ClosedPublic

Authored by arsenm on Feb 17 2017, 4:22 PM.

Details

Diff Detail

Event Timeline

arsenm created this revision.Feb 17 2017, 4:22 PM
kzhuravl accepted this revision.Feb 18 2017, 5:23 AM

LGTM.

lib/Target/AMDGPU/AMDGPU.td
555

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

557–558

Subtarget->getGeneration() >= AMDGPUSubtarget::SEA_ISLANDS?

This revision is now accepted and ready to land.Feb 18 2017, 5:23 AM
arsenm added inline comments.Feb 18 2017, 10:20 AM
lib/Target/AMDGPU/AMDGPU.td
555

Probably. I have the opposite problem and need to look up the numbers

arsenm closed this revision.Feb 18 2017, 10:42 AM

r295554