[PATCH] D107088: [ARC] Add additional mov immediate instruction formats with a fix for u6 decoding

[Thomas Johnson via Phabricator via llvm-commits]

thomasjohns added inline comments.


================
Comment at: llvm/lib/Target/ARC/ARCInstrFormats.td:289
-}
-
 // Dual Operand Instructions.  Inst[21-16] specifies the specific operation
----------------
This change was really just a rename from `F32_SOP_CC_RU6` to `F32_DOP_CC_RU6`. The instruction is really a "dual-operand" instruction, and was just named incorrectly earlier.

The full diff shows up rather than the rename because the definition was moved to be adjacent to other dual operand instructions.


================
Comment at: llvm/lib/Target/ARC/ARCInstrFormats.td:367
   bits<6> B;
-  bits<6> A;
 
----------------
This field was unused.


================
Comment at: llvm/lib/Target/ARC/Disassembler/ARCDisassembler.cpp:307
   using Field = decltype(Insn);
-  Field U6Field = fieldFromInstruction(Insn, 6, 11);
+  Field U6Field = fieldFromInstruction(Insn, 6, 6);
   Inst.addOperand(MCOperand::createImm(U6Field));
----------------
This API was misinterpreted earlier to be `fieldFromInstruction(..., startBit, endBit)` instead of `fieldFromInstruction(..., startBit, numBits)` (correct). This didn't materialize earlier because all of the associated tests had 0'd out high order bits. So overreading the bits for the u6 instruction wasn't noticed because it didn't change the u6 value.

Once the `f` flag was added, it caused the u6 decoding here to overflow, because the `f` flag bit was 11 bits away from the u6 start bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107088