[PATCH] D73956: [AMDGPU] Add a16 feature to gfx10

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle added a comment.

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.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPU.td:366-370
+def FeatureA16 : SubtargetFeature<"a16",
+  "HasA16",
+  "true",
+  "Support 16 bit coordindates/gradients/lod/clamp/mip types on gfx10"
+>;
----------------
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".


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73956/new/

https://reviews.llvm.org/D73956