[PATCH] D50784: [TableGen] Add Instruction custom byte sequence emission

[Artyom Goncharov via Phabricator via llvm-commits]

m4yers added inline comments.


================
Comment at: utils/TableGen/CodeBeadsGen.cpp:13-14
+/// defined for an instruction. They are used to pass very target specific
+/// binary information directly from a tablegen file, e.g. instruction
+/// encoding.
+///
----------------
lebedev.ri wrote:
> m4yers wrote:
> > efriedma wrote:
> > > lebedev.ri wrote:
> > > > Ignorant question: but surely the problem of storing instruction encoding in tablegen has already arose?
> > > > How does this solution differ from the existing practice?
> > > Most other targets use CodeEmitterGen to generate a function getBinaryCodeForInstr.  This is nice, but it only works for targets with fixed-length instructions of length 4 or less.
> > > 
> > > x86 doesn't store instruction encodings in TableGen; instead, it uses a hand-written implementation of X86MCCodeEmitter::encodeInstruction, driven by flags in the instruction description.
> > It allows to pass encoding strings of various length. Also it is semantically different from TSFlags and I thought it'd be a poor choice to extend its size and reuse.
> (At the very least this should be explained here in the comment.)
I will qualify that the binary string is of arbitrary size.


================
Comment at: utils/TableGen/CodeBeadsGen.cpp:52-54
+  // Current limit of beads is 192 bits
+  constexpr unsigned BeadsLength = 192;
+  constexpr unsigned BeadsNumber = BeadsLength / BeadSize;
----------------
lebedev.ri wrote:
> I wonder if it would be more straight-forward to go the other way around?
> You wouldn't need to manually worry to specify multiple-of-BeadSize-bits value.
Do you mean like this?
```
constexpr unsigned BeadSize = 8;
constexpr unsigned BeadsNumber = 24;
constexpr unsigned BeadsLength = BeadsNumber * BeadSize;
```


Repository:
  rL LLVM

https://reviews.llvm.org/D50784