[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:118
+  // For manually mapping instructions that do not match by their encoding.
+  const std::vector<ManualMapEntry> ManualMapSet = {
+      { "ADD16ri_DB",       "ADD16mi",         NO_UNFOLD  },
----------------
aymanmus wrote:
> craig.topper wrote:
> > Why does this need to be vector? Why not a regular array or std::array?
> Defining an array as a class member requires stating it's size (ManualMapSet[18]), while std::vector doesn't. I thought this table might be dynamic and changed from time to time, so this way can be more friendly for future tuning.
> What do you think?
The number is required even if you put the array contents with it?


================
Comment at: utils/TableGen/X86FoldTablesEmitter.cpp:215
+  bool isExplicitAlign(const CodeGenInstruction *Inst) {
+    for (const std::string InstStr : ExplicitAlign) {
+      if (Inst->TheDef->getName().find(InstStr) != std::string::npos)
----------------
craig.topper wrote:
> This is a string copy. Did you omit an '&'?
Should those be String::npos too?


================
Comment at: utils/TableGen/X86FoldTablesEmitter.cpp:109
+  const StringRef ExplicitAlign[7] = {"MOVDQA",  "MOVAPS",  "MOVAPD",
+                                        "MOVNTPS", "MOVNTPD", "MOVNTDQ",
+                                        "MOVNTDQA"};
----------------
Fix indentation.


https://reviews.llvm.org/D32684