[PATCH] D52366: [tblgen][disasm] Separate encodings from instructions

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: utils/TableGen/FixedLenDecoderEmitter.cpp:94-95
+
+  EncodingAndInst(const Record *EncodingDef, const CodeGenInstruction *Inst)
+      : EncodingDef(EncodingDef), Inst(Inst) {}
+};
----------------
bogner wrote:
> Would simplifying by omitting this and just using brace-initialization be better?
It's needed for std::vector<EncodingAndInst>::emplace_back(). It eventually ends up failing on the placement new inside std::allocator::construct() without it


================
Comment at: utils/TableGen/FixedLenDecoderEmitter.cpp:420-422
+    if (EncodingDef != InstDef)
+      OS << EncodingDef->getName() << ":";
+    OS << InstDef->getName();
----------------
bogner wrote:
> Why not use the operator<< here if you're going to define it above anyway?
Well spotted. It's just that I didn't notice this one when I added the operator. I'll fix this


================
Comment at: utils/TableGen/FixedLenDecoderEmitter.cpp:851-853
+        if (EncodingDef != Def)
+          OS << EncodingDef->getName() << " => ";
+        OS << Def->getName() << "\n";
----------------
bogner wrote:
> Same here (unless using => instead of : is important for some reason?)
It's not hugely important. I used => here to indicate that we've matched the AlternateEncoding record but we're building the MCInst as if we'd matched the Instruction. I can switch this for the operator


Repository:
  rL LLVM

https://reviews.llvm.org/D52366