[PATCH] D32684: [X86] Adding new LLVM TableGen backend that generates the X86 backend memory folding tables.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: utils/TableGen/X86FoldTablesEmitter.cpp:76
+// Do not add these instructions to any of the folding tables.
+const std::vector<const char *> NoFoldSet = {
+    "TCRETURNri64",
----------------
Can this be an array now instead of a vector?


================
Comment at: utils/TableGen/X86FoldTablesEmitter.cpp:172
+
+      std::string result = "";
+
----------------
Do we need the temporary string? Can we just stream as we go?


================
Comment at: utils/TableGen/X86FoldTablesEmitter.cpp:228
+
+    for (X86FoldTableEntry E : Table)
+      OS << E;
----------------
This is a copy, can we just do

for (const X86FoldTableEntry &E : Table)


================
Comment at: utils/TableGen/X86FoldTablesEmitter.cpp:350
+
+  // Values from X86Local namespace defined in X86RecognizableInstr.cpp
+  return FormBitsNum >= X86Local::MRMDestMem && FormBitsNum <= X86Local::MRM7m;
----------------
Should this be X86RecognizableInstr.h?


================
Comment at: utils/TableGen/X86FoldTablesEmitter.cpp:562
+    Result.Alignment = Value;
+  } else if (!RegRec->isSubClassOf("VEX")) {
+    // Instructions with VEX encoding do not require alignment.
----------------
Can we use the OpEncBits field and compare against the encoding from X86Local?


================
Comment at: utils/TableGen/X86FoldTablesEmitter.cpp:588
+  // added to Table2Addr.
+  if (hasDefInList(MemRec, "SchedRW", "WriteRMW") && MemOutSize != RegOutSize &&
+      MemInSize == RegInSize) {
----------------
Is the best way we have to detect these instructions?


https://reviews.llvm.org/D32684