[PATCH] D136808: Refactor Tblgen DecoderEmitter to allow multiple language output

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

myhsu added a comment.

I only skimmed through the patch but I found this pretty interesting. To my best understanding, you're trying to add another layer of abstraction between disassembler generation logics and the code printer. Such that downstream projects like Capstone only need to maintain a single printer code (e.g. PrinterCapston.cpp) rather than hundreds of lines of code spanning across DecoderEmitter.cpp. If this is the case, I strongly encourage you to upstream most of the stuff here except PrinterCapstone.cpp.

> Other backends which emit code would need a "PrinterInterface" as well (e.g. AsmWriter, RegisterInfo, InstrInfo, SubtargetInfo).

We can try on the disassembler first and gradually refactor other components later in other patches (because this new printer interface can co-exist with the current code printing logics, right?)



================
Comment at: llvm/utils/TableGen/DecoderEmitter/CMakeLists.txt:6
+add_llvm_library(LLVMTableDecoderEmitter LINK_COMPONENTS DISABLE_LLVM_LINK_LLVM_DYLIB
+  "DecoderEmitter.cpp"
+  "Filter.cpp"
----------------
double quotes are not necessary here, please remove them for better readability


================
Comment at: llvm/utils/TableGen/DecoderEmitter/README.md:1
+# Decoder emitter
+
----------------
Kudos on this document!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136808