[PATCH] D142636: AMDGPU/MC: Refactor decoders. Rework decoders for float immediates

[Joe Nash via Phabricator via llvm-commits]

Joe_Nash added a comment.

Overall I like the patch. It definitely improves readability.



================
Comment at: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp:139
+    return addOperand(Inst, DAsm->decodeSrcOp(AMDGPUDisassembler::OpWidth,     \
+                                              EncImm, Deffered, ImmWidth));    \
+  }
----------------
The argument name in decodeSrcOp is MandatoryLiteral, so you should probably use the same. Though I am good with it if you want to rename the argument to HasDeferredLiteral or something similar to match the terminology in SIRegisterInfo


================
Comment at: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp:1489
 
-  // ToDo: case 248: 1/(2*PI) - is allowed only on VI
-  switch (Width) {
----------------
This comment should probably be maintained?


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.td:1078
+
+  class RegOrB16 <string RegisterClass, string OperandTypePreffix>
+    : RegOrImmOperand <RegisterClass, OperandTypePreffix # "_INT16",
----------------
Spelling: Preffix -> Prefix, here and below


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

https://reviews.llvm.org/D142636