[PATCH] D115128: [TableGen][CodeEmitter] Introducing the VarLenCodeEmitterGen infrastructure

[Min-Yih Hsu via Phabricator via llvm-commits]

myhsu added inline comments.


================
Comment at: llvm/include/llvm/Target/Target.td:761
+///     (descend 0b0110, 0b1101)
+/// This results in an encoding of 0b01101101.
+def descend;
----------------
ricky26 wrote:
> These two examples seem to be the same.
you're right. I was intended to use the same arguments for both `ascend` and `descend` but forgot to change the final encodings.


================
Comment at: llvm/utils/TableGen/VarLenCodeEmitterGen.cpp:66
+
+void VarLenCodeEmitterGen::VarLenInst::buildRec(const DagInit *DI) {
+  auto Op = DI->getOperator()->getAsString();
----------------
ricky26 wrote:
> Not really anything worth changing now (mostly a note to myself if I come to write a disassembler generator for this):
> It might be worth coalescing neighbouring constant bits.
oh, right, the coalescing will only happen in the later stage ( `VarLenCodeEmitterGen::emitInstructionBaseValues`). I agree it might be worth it to do in VarLenInst if we want to share this class with the disassembler.


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

https://reviews.llvm.org/D115128