[Mips] Emit the correct DINS variant

[spetrovic]

Summary

This patch fixes emitting of correct variant of DINS instruction. I'm referencing the gnu assembler for this solution.
Here is an example of a case where LLVM assembler emits DINS instruction instead of DINSM:

For instruction: dins $2, $1, 28, 6

Output without the patch: 7c220f07 dins v0,at,0x1c,0xffffffe6 -> 07 (opcode for dins)
Output with the patch: 7c220f05 dins v0,at,0x1c,0x6 -> 05 (opcode for dinsm)

Output with gnu assembler: 7c220f05 dins v0,at,0x1c,0x6 -> (05 opcode for dinsm)

[sdardis]

This requires a test. Can you add regression tests to test/MC/Mips/mips64extins.s as part of this patch?

[spetrovic]

I think that llvm-objdump tool has an issue with DINS instruction, so I was using gnu objdump for testing.
For example, for dins $2, $1, 28, 6 llvm-objdump generates: dins 7c 22 0f 05 dinsm $2, $1, 28, 38 (it dumps DINSM instruction, that part is ok, but it generates 38 instead of 6). Thats why I didn't add test case, once when llvm-objdump is fixed I can add test.
The issue with llvm-objdump might be that for dins, bits 15-11 contain (pos + size - 1) and for dinsm (pos + size -33) and it seems that llvm-objdump treats these bits for dinsm as if it was dins, since it always displays size increased for 32.

[sdardis]

LGTM with inline comment addressed.

As for the disassembler, for the moment just match the instruction encoding and opcode.

Ideally, we should emit the assembly text for dins in all cases.

lib/Target/Mips/MCTargetDesc/MipsMCCodeEmitter.cpp

124 ↗

(On Diff #91884)

The second condition is incorrect, it should be (pos + size) <= 64, according to the 5.04 BIS.

[spetrovic]

Comment addressed

llvm/trunk/lib/Target/Mips/MCTargetDesc/MipsMCCodeEmitter.cpp

static void LowerDins(MCInst& InstIn) {
   assert(InstIn.getNumOperands() == 5 &&
          "Invalid no. of machine operands for DINS!");
 
   assert(InstIn.getOperand(2).isImm());
   int64_t pos = InstIn.getOperand(2).getImm();
   assert(InstIn.getOperand(3).isImm());
   int64_t size = InstIn.getOperand(3).getImm();
 
-  if (size <= 32) {
-    if (pos < 32)  // DINS, do nothing
-      return;
+  assert((pos + size) <= 64 &&
+         "DINS cannot have position plus size over 64");
+  if (pos < 32) {
+    if ((pos + size) > 0 && (pos + size) <= 32)
+      return; // DINS, do nothing
+    else if ((pos + size) > 32) {
+      //DINSM
+      InstIn.getOperand(3).setImm(size - 32);
+      InstIn.setOpcode(Mips::DINSM);
+    }
+  } else if ((pos + size) > 32 && (pos + size) <= 64) {
     // DINSU
     InstIn.getOperand(2).setImm(pos - 32);
     InstIn.setOpcode(Mips::DINSU);
-    return;
   }
-  // DINSM
-  assert(pos < 32 && "DINS cannot have both size and pos > 32");
-  InstIn.getOperand(3).setImm(size - 32);
-  InstIn.setOpcode(Mips::DINSM);
 }
 
 // Fix a bad compact branch encoding for beqc/bnec.
 void MipsMCCodeEmitter::LowerCompactBranch(MCInst& Inst) const {
   // Encoding may be illegal !(rs < rt), but this situation is
   // easily fixed.
   unsigned RegOp0 = Inst.getOperand(0).getReg();
   unsigned RegOp1 = Inst.getOperand(1).getReg();