This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Add a16 feature to gfx10
ClosedPublic

Authored by sebastian-ne on Feb 4 2020, 5:21 AM.

Details

Summary

Based on D72931

This adds a new feature called A16 which is enabled for gfx10.
gfx9 keeps the R128A16 feature so it can share all the instruction encodings
with gfx7/8.

Diff Detail

Event Timeline

sebastian-ne created this revision.Feb 4 2020, 5:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 4 2020, 5:21 AM
arsenm added a comment.Feb 4 2020, 7:54 AM

Seems like it's missing some assembler/disassembler tests

Add gfx10 to a16 tests

The encoding changes cause a mess with the feature definitions, but cleaning it up would require splitting up machine opcodes further which would bloat TableGen tables... so let's keep this approach. I'd ask for one cleanup though.

llvm/lib/Target/AMDGPU/AMDGPU.td
372–376

Please rename the feature to gfx10a16, and the description to: "Support gfx10-style A16 for 16-bit coordinates/gradients/lod/clamp/mip image operands". (Note the "coordinates" typo).

The idea here is to hopefully reduce future confusion sightly a bit by explicitly calling out that there is gfx9-style A16 vs. gfx10-style A16.

At the same time, it seems a good idea to change the description of the R128A16 analogously, perhaps adding ", where a16 is aliased with r128".

Rename a16 to gfx10a16 in some places

nhaehnle accepted this revision.Feb 7 2020, 12:54 AM

Thanks, LGTM.

This revision is now accepted and ready to land.Feb 7 2020, 12:54 AM
This revision was automatically updated to reflect the committed changes.