[M68k][Disassembler] Adopt the new variable length decoder

[0x59616e]

Summary

This is an example usage of D120958.

After these patches are landed, we can strip off the codebeads officially.

[0x59616e]

Fix a minor problem in the test

[0x59616e]

rebase

[0x59616e]

Remove tablegen variable isVarLenInst. It's useless

[0x59616e]

formatting

[0x59616e]

rebase

[0x59616e]

rebase

[myhsu]

just want to double check: all of the previously-XFAIL tests are now passing, right?

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

23

I'm wonder if we want to rename this file as well?

121

is it possible to use llvm::alignTo here?

129

[0x59616e]

address feedback:

• use llvm::alignTo
• construct APInt directly with constructor instead of assignment copy constructor

[0x59616e]

In D120960#3484760, @myhsu wrote:

just want to double check: all of the previously-XFAIL tests are now passing, right?

Yes. I don't see any 'unexpectedly passed' test.

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

23

I think MCDecoderOps.h is more suitable.

Should we create another revision for it ?

[myhsu]

In D120960#3486430, @0x59616e wrote:

In D120960#3484760, @myhsu wrote:

just want to double check: all of the previously-XFAIL tests are now passing, right?

Yes. I don't see any 'unexpectedly passed' test.

This is a good news and proofs that the new variable-length decoder infrastructure works at scale :-)
(at least at a certain scale given that we still more coverage on our disassembler tests)

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

23

I think MCDecoderOps.h is more suitable.

Agree.

Should we create another revision for it ?

Sounds good to me

[0x59616e]

In D120960#3488691, @myhsu wrote:

In D120960#3486430, @0x59616e wrote:

In D120960#3484760, @myhsu wrote:

just want to double check: all of the previously-XFAIL tests are now passing, right?

Yes. I don't see any 'unexpectedly passed' test.

This is a good news and proofs that the new variable-length decoder infrastructure works at scale :-)
(at least at a certain scale given that we still more coverage on our disassembler tests)

I'll open a bug tracker for this after landing this revision.

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

23

I'll do it later. Thanks ;)

[0x59616e]

rebase

[0x59616e]

Thanks ! Appreciate your help ;)

[RKSimon]

@0x59616e I'm seeing several "Decoding Conflict" warnings in M68k builds - e.g.:

Decoding Conflict:
                ................................................................0010...001010...
                ................................................................0010...00.010...
                ................................................................0010...00.01....
                ................................................................0010......01....
                ................................................................0010............
                ................................................................................
        MOV32aj 0010___001010___
        MOV32aj_TC 0010___001010___

[0x59616e]

That's expected. I'll open an issue for it later this day.

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

 //===-- M68kDisassembler.cpp - Disassembler for M68k ------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file is part of the M68k Disassembler.
 //
 //===----------------------------------------------------------------------===//
 
 #include "M68k.h"
 #include "M68kRegisterInfo.h"
 #include "M68kSubtarget.h"
 #include "MCTargetDesc/M68kMCCodeEmitter.h"
 #include "MCTargetDesc/M68kMCTargetDesc.h"
 #include "TargetInfo/M68kTargetInfo.h"
 
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCDisassembler/MCDisassembler.h"
 #include "llvm/MC/MCFixedLenDisassembler.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/ErrorHandling.h"
 
 using namespace llvm;
 
 #define DEBUG_TYPE "m68k-disassembler"

 };
 
 DecodeStatus M68kDisassembler::getInstruction(MCInst &Instr, uint64_t &Size,
                                               ArrayRef<uint8_t> Bytes,
                                               uint64_t Address,
                                               raw_ostream &CStream) const {
   DecodeStatus Result;
   auto MakeUp = [&](APInt &Insn, unsigned InstrBits) {
-    unsigned i = Insn.getBitWidth();
-    unsigned RoundUp = ((InstrBits + 0xf) & ~0xf);
+    unsigned Idx = Insn.getBitWidth() >> 3;
+    unsigned RoundUp = alignTo(InstrBits, Align(16));
     Insn = Insn.zextOrSelf(RoundUp);
     RoundUp = RoundUp >> 3;
-    for (i = i >> 3; i < RoundUp; i += 2) {
-      Insn.insertBits(support::endian::read16be(&Bytes[i]), i * 8, 16);
+    for (; Idx < RoundUp; Idx += 2) {
+      Insn.insertBits(support::endian::read16be(&Bytes[Idx]), Idx * 8, 16);
     }
   };
-  APInt Insn = APInt(16, support::endian::read16be(Bytes.data()));
+  APInt Insn(16, support::endian::read16be(Bytes.data()));
   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,