[PATCH] D52366: [tblgen][disasm] Separate encodings from instructions
Daniel Sanders via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 31 10:42:51 PDT 2018
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
More information about the llvm-commits
mailing list