[M68k][Disassembler] Fix decoding conflict

[0x59616e]

Summary

This diff fixes decoding conflict between these pair of instructions:

ADD(16|32)dd / ADD(16|32)dr
SUB(16|32)dd / SUB(16|32)dr
AND(16|32)dd / AND(16|32)dr
OR(16|32)dd  / OR(16|32)dr

[RKSimon]

llvm/lib/Target/M68k/Disassembler/M68kDisassembler.cpp

130

Some kind of description comment for this kind of thing is always useful.

[0x59616e]

Add comment

[ricky26]

llvm/lib/Target/M68k/M68kInstrArithmetic.td

79

How does this avoid the conflict? Presumably the 'r' version could still specify a data register and it would have the same encoding? 🤔

[0x59616e]

llvm/lib/Target/M68k/M68kInstrArithmetic.td

79

In our cases, the decoder emitter will recurse down to only two cases, that is, the one with 'r' addressing mode and the 'd' addressing mode. In this special case, decoder emitter will prefer the one with less entropy, i.e. 'd' addressing mode.

[ricky26]

LGTM

[0x59616e]

Thanks !

llvm/lib/Target/M68k/Disassembler/M68kDisassembler.cpp

auto MakeUp = [&](APInt &Insn, unsigned InstrBits) {
     unsigned RoundUp = alignTo(InstrBits, Align(16));
     Insn = Insn.zextOrSelf(RoundUp);
     RoundUp = RoundUp >> 3;
     for (; Idx < RoundUp; Idx += 2) {
       Insn.insertBits(support::endian::read16be(&Bytes[Idx]), Idx * 8, 16);
     }
   };
   APInt Insn(16, support::endian::read16be(Bytes.data()));
+  // 2 bytes of data are consumed, so set Size to 2
+  // If we don't do this, disassembler may generate result even
+  // the encoding is invalid. We need to let it fail correctly.
+  Size = 2;
   Result = decodeInstruction(DecoderTable80, Instr, Insn, Address, this, STI,
                              MakeUp);
   if (Result == DecodeStatus::Success)
     Size = InstrLenTable[Instr.getOpcode()] >> 3;
   return Result;
 }
 
 static MCDisassembler *createM68kDisassembler(const Target &T,

llvm/lib/Target/M68k/M68kInstrArithmetic.td

 class MxBiArOp_R_RR_xEA<string MN, SDNode NODE, MxType DST_TYPE, MxType SRC_TYPE,
                         bits<4> CMD>
     : MxInst<(outs DST_TYPE.ROp:$dst), (ins DST_TYPE.ROp:$src, SRC_TYPE.ROp:$opd),
              MN#"."#DST_TYPE.Prefix#"\t$opd, $dst",
              [(set DST_TYPE.VT:$dst, CCR, (NODE DST_TYPE.VT:$src, SRC_TYPE.VT:$opd))]> {
   let Inst = (descend
     CMD, (operand "$dst", 3),
     !cast<MxOpModeEncoding>("MxOpMode"#DST_TYPE.Size#"_"#DST_TYPE.RLet#"_EA").Value,
-    (descend /*MODE without last bit*/0b00,
-             /*REGISTER prefixed with D/A bit*/(operand "$opd", 4))
+    !cond(
+      !eq(SRC_TYPE.RLet, "r") : (descend 0b00, (operand "$opd", 4)),
+      !eq(SRC_TYPE.RLet, "d") : (descend 0b000, (operand "$opd", 3))
+    )
   );
 }
 
 /// This Op is similar to the one above except it uses reversed opmode, some
 /// commands(e.g. eor) do not support dEA or rEA modes and require EAd for
 /// register only operations.
 /// NOTE when using dd commands it is irrelevant which opmode to use(as it seems)
 /// but some opcodes support address register and some do not which creates this

llvm/test/MC/Disassembler/M68k/arithmetic.txt

-# FIXME: The first two tests are disabled due to decoding conflict
-# RUN: llvm-mc -disassemble -triple m68k %s | not FileCheck -check-prefix=CHECK-CONFLICT %s
 # RUN: llvm-mc -disassemble -triple m68k %s | FileCheck %s
 
-# CHECK-CONFLICT: adda.l %a0, %a1
+# CHECK: adda.l %a0, %a1
 0xd3 0xc8
-# CHECK-CONFLICT: sub.w %d3, %d1
+# CHECK: sub.w %d3, %d1
 0x92 0x43
 
 # CHECK: add.w    (56,%a4,%d3), %d2
 0xd4 0x74 0x38 0x38
 
 # CHECK: add.w    #35172, %d1
 0xd2 0x7c 0x89 0x64
 

 0x8c 0xbb 0x98 0x37
 
 # CHECK: or.l    (38967,%a3), %d6
 0x8c 0xab 0x98 0x37
 
 # CHECK: or.l    %d6, (48,%a4,%a2)
 0x8d 0xb4 0xa8 0x30
 
-# CHECK: or.b    %a2, %d7
-0x8e 0x0a
+# CHECK: or.b  %d2, %d7
+0x8e 0x02
 
 # CHECK: or.b    (4163,%a1), %d0
 0x80 0x29 0x10 0x43
 
 # CHECK: or.b    #16, (3,%a3,%d4)
 0x00 0x33 0x00 0x10 0x48 0x03
 
 # CHECK: sub.w   %d5, (16,%a3,%a2)
 0x9b 0x73 0xa8 0x10
 
 # CHECK: suba.l  (85,%a4,%a2), %a0
 0x91 0xf4 0xa8 0x55
 
 # CHECK: suba.l  %d4, %a1
 0x93 0xc4
 
 # CHECK: sub.l   #16843009, %d3
-0x96 0xbc 0x01 0x01 0x01 0x01
+0x96 0xbc 0x01 0x01 0x01 0x01
+
+# CHECK: add.w %d0, %d4
+0xd8 0x40
+
+# CHECK: add.w %a2, %d3
+0xd6 0x4a
+
+# CHECK: add.l %d1, %d2
+0xd4 0x81
+
+# CHECK: add.l %a0, %d1
+0xd2 0x88
+
+# CHECK: sub.w %a5, %d1
+0x92 0x4d
+
+# CHECK: sub.w %d2, %d4
+0x98 0x42
+
+# CHECK: sub.l %d2, %d5
+0x9a 0x82
+
+# CHECK: sub.l %a2, %d5
+0x9a 0x8a
+
+# CHECK: subx.w %d2, %d4
+0x99 0x42
+
+# CHECK: subx.l %d5, %d3
+0x97 0x85
+
+# CHECK: suba.l %d4, %a0
+0x91 0xc4
+
+# CHECK: suba.l #12576, %a2
+0x95 0xfc 0x00 0x00 0x31 0x20
+
+# CHECK: suba.l (%sp), %a0
+0x91 0xd7
+
+# CHECK: and.b (256,%a2), %d0
+0xc0 0x2a 0x01 0x00
+
+# CHECK: and.w %d0, %d7
+0xce 0x40
+
+# CHECK: and.w %a1, %d3
+0xc6 0x49
+
+# CHECK: and.w (32,%a0,%d1), %d1
+0xc2 0x70 0x18 0x20
+
+# CHECK: and.w (288,%a2), %d4
+0xc8 0x6a 0x01 0x20
+
+# CHECK: and.l %d1, (49,%a0,%sp)
+0xc3 0xb0 0xf8 0x31
+
+# CHECK: and.l #100, (1,%a1,%d4)
+0x02 0xb1 0x00 0x00 0x00 0x64 0x48 0x01
+
+# CHECK: and.l %d0, %d1
+0xc2 0x80
+
+# CHECK: and.l %a1, %d2
+0xc4 0x89
+
+# CHECK: or.b #7, (%a1)
+0x00 0x11 0x00 0x07
+
+# CHECK: or.w %d2, %d5
+0x8a 0x42
+
+# CHECK: or.w %a0, %d2
+0x84 0x48
+
+# CHECK: or.w #0, (0,%a2,%a0)
+0x00 0x72 0x00 0x00 0x88 0x00
+
+# CHECK: or.l (15,%pc,%a1), %d0
+0x80 0xbb 0x98 0x0f
+
+# CHECK: or.l %a0, %d0
+0x80 0x88
+
+# CHECK: or.l %d1, %d6
+0x8c 0x81