[PATCH] D98540: [M68k] Implement Disassembler

Mon Apr 19 10:20:40 PDT 2021

myhsu added a comment.

In D98540#2696459 <https://reviews.llvm.org/D98540#2696459>, @ricky26 wrote:

> In D98540#2690082 <https://reviews.llvm.org/D98540#2690082>, @myhsu wrote:
>
>> First of all, good job :-)
>> Though I only scanned over the patch, I'm a little concerned about the adoption of code beads here because //ideally//, we are going to remove code beads from LLVM in the future.
>
> I agree but this is the path of least resistance and I don't think it's too much 'wasted work'. I'm hoping when the encoding mechanism is changed, it will be staged (introduce new encoding in table-gen, start moving components over in later patches). In which case I don't think the disassembler will get in the way. The benefit being that with this quite shallow approach (using the beads to build bitmasks) we get the extra functioning surface area of the disassembler in the mean time (and at least some of the disassembler won't need to be changed with the encoding rework).
>
> If you think it's better to try and solve the encoding setup first, I have no qualms. I'm pretty interested in helping with the encoding rework too if I can. :)

Nah, let's sort out the disassembler first.

>> As stated in one of my comments, can we get any help from the disassembler table generated by TableGen?
>
> I forgot I included this. I had a look when I initially started poking around the disassembler but it looks like it is generated for instructions with the `Inst` field set.
>
> There are some overrides (X86 being notable) and I considered writing an M68k extension but there're a fair few pieces of information that aren't available in table-gen which we need to parse the beads and produce a disassembly table. I figured that we //would// migrate over to using that (probably still with a bit of a custom extension) once we reworked the encoding.

You're right, it's probably better to consider this option after the encoding rework

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98540