[PATCH] D52100: [tblgen] Allow FixedLenDecoderEmitter to use APInt-like objects as InsnType

[Ryan Houdek via Phabricator via llvm-commits]

Sonicadvance1 added a comment.

In https://reviews.llvm.org/D52100#1277817, @dsanders wrote:

> In https://reviews.llvm.org/D52100#1277769, @Sonicadvance1 wrote:
>
> > This commit breaks my 128bit instruction target.
> >  I'm currently using a workaround locally for enabling APInt based encodings when your instruction encoding is >64bit.
> >  The main issue with this approach is that getBinaryCodeForInstr completely falls apart.
>
>
> Could you elaborate on this? getBinaryCodeForInstr() is for encoding instructions whereas FixedLenDecoderEmitter generates a decoder so I'm surprised that you're seeing a connection between getBinaryCodeForInstr() and this patch. Our out-of-tree target didn't use the FixedLenDecoderEmitter until after this patch made it possible but we've been using getBinaryCodeForInstr() for a very long time so I think there's unlikely to be a connection between the two. Additionally, not affecting encoding was a requirement for us as we're also working around the lack of >64-bit support in getBinaryCodeForInstr()


I can only assume you aren't using a disassembler in your backend currently? That also breaks immediately.
Any form of reasonably mature backend is going to hit issues due to this being half baked at the moment.
I was using the getBinaryCodeForInstr function as an example of missing features that this misses but there are definitely more than what I've stated.
Sadly my backend is out of tree so I can't complain too much.


Repository:
  rL LLVM

https://reviews.llvm.org/D52100