[PATCH] D37983: Add instruction subset for the ARC backend

[Tatyana Krasnukha via Phabricator via llvm-commits]

tatyana-krasnukha added a comment.

Hi Pete,



================
Comment at: lib/Target/ARC/Disassembler/ARCDisassembler.cpp:275
+  DEBUG(dbgs() << "Decoding MOV_S h-register\n");
+  auto h = fieldFromInstruction(Insn, 5, 3) |
+           (fieldFromInstruction(Insn, 0, 2) << 3);
----------------
petecoup wrote:
> Can we use the appropriate unsigned type, rather than auto?
Ok. I would like to use decltype(Insn) instead of uint64_t to draw the attention that size of return value is always the same as Insn size. Do you mind?


================
Comment at: lib/Target/ARC/Disassembler/ARCDisassembler.cpp:280
+
+  auto DecodeRegisterOrImm = [&Inst, Address, Decoder](decltype(h) field,
+                                                       decltype(Insn) value) {
----------------
petecoup wrote:
> Can we pull this out into another static routine?
This routine has no sense outside the scope of this function.

May be if the routine will take a parameter determine whether this is 32 or 16 bit instruction and check register number (30/62), depending on it, there will be good reason to make it a separate static function and use in many other places... But I guess to do it in another patch, do you think?


Repository:
  rL LLVM

https://reviews.llvm.org/D37983