[PATCH] D89217: [AMDGPU] Base getSubRegFromChannel on TableGen data

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:49
+//      meaning index 7 in SubRegFromChannelTable.
+static const uint16_t SubRegFromChannelTableWidthMap[17] = {
+    0, 1, 2, 3, 4, 5, 6, 7, 8, 0, 0, 0, 0, 0, 0, 0, 9};
----------------
critson wrote:
> foad wrote:
> > `uint16_t` seems like an odd type to use. `unsigned` would be more normal, or `uint8_t` if you want to keep the size down. But it really doesn't matter.
> Currently SubReg is allocated 12 bits in MachineOperand, so needs to be bigger than uint8_t, but fits nicely in uint16_t.
I understand that argument for the ChannelTable but not for the WidthMap.


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:97
+      unsigned Offset = AMDGPUSubRegIdxRanges[Idx].Offset / 32;
+      assert(Width < array_lengthof(SubRegFromChannelTableWidthMap));
+      Width = SubRegFromChannelTableWidthMap[Width];
----------------
critson wrote:
> foad wrote:
> > You can use `.size()` here.
> It's not a std::array.
Sorry, I missed that. But it could be :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89217