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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/utils/TableGen/VarLenCodeEmitterGen.cpp:103
+      ArrayRef<const CodeGenInstruction *> NumberedInstructions,
+      CodeGenTarget &Target, int HwMode = -1);
+
----------------
nit. Is the default for HwMode ever used? The call when HwModes is empty passes -1


================
Comment at: llvm/utils/TableGen/VarLenCodeEmitterGen.cpp:324
+  for (unsigned I = 0; I < Bits.getNumWords(); ++I, ++Index)
+    (I ? SS : SS.indent(4))
+        << "UINT64_C(" << utostr(Bits.getRawData()[I]) << "),";
----------------
Can the indent call be done before the loop?

Though I'm not sure about the indent. The NumBits and Index don't have a new line after them so we're not at the start of a line here.


================
Comment at: llvm/utils/TableGen/VarLenCodeEmitterGen.cpp:363
+
+    if (!VarLenInsts.count(EncodingDef))
+      PrintFatalError(EncodingDef, "VarLenInst not found for this record");
----------------
Use VarLenInsts.find to avoid doing two map lookups?


================
Comment at: llvm/utils/TableGen/VarLenCodeEmitterGen.cpp:423
+    Record *R, Record *EncodingDef, CodeGenTarget &Target) {
+  if (!VarLenInsts.count(EncodingDef))
+    PrintFatalError(EncodingDef, "Parsed encoding record not found");
----------------
Use VarLenInsts.find() instead of looking up the map twice?


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

https://reviews.llvm.org/D115128