[TableGen][X86] Add Size field to X86MemOperand class

[Amir]

Summary

Set Size appropriately in operand definitions and query it for dumping memory
operand size table getMemOperandSize (follow-up use D126116) and
X86Disassembler::getMemOperandSize.

Excerpt from a produced getMemOperandSize table for X86:

static int getMemOperandSize(int OpType) {
  switch (OpType) {
  default: return 0;
  case OpTypes::i8mem:
  case OpTypes::i8mem_NOREX:
    return 8;

  case OpTypes::f16mem:
  case OpTypes::i16mem:
    return 16;

  case OpTypes::f32mem:
  case OpTypes::i32mem:
    return 32;
...

[skan]

llvm/utils/TableGen/InstrInfoEmitter.cpp

468

Could we add one field for class X86MemAsmOperand explicitly? I don't think regex is good way here...

[Amir]

Extend X86MemOperand with Size field.
Only process X86MemOperands in GET_INSTRINFO_MEM_OPERAND_SIZE.
Make i8mem_NOREX inherit from X86MemOperand.

[Amir]

llvm/utils/TableGen/InstrInfoEmitter.cpp

468

I've extended X86MemOperand class (I think that's the one you had in mind) with Size field.
It's being queried explicitly.

The alternative is to check something like

if (Op->getValueAsString("OperandType") != "OPERAND_MEMORY" ||
    Op->isValueUnset("Size"))
  continue;

to be able to cover operands other than X86MemOperand/targets other than X86.

[pengfei]

LGTM. The summary need to update too.

llvm/utils/TableGen/InstrInfoEmitter.cpp

25

This is not needed, right?

[craig.topper]

What is this going to be used for?

[skan]

llvm/lib/Target/X86/X86InstrInfo.td

397 ↗

(On Diff #437040)

Could you also simply the function getMemOperandSize in X86RecognizableInstr.cpp in this patch? You can return value of Size now.

[skan]

In D127787#3584289, @craig.topper wrote:

What is this going to be used for?

I think it is for D126116.

[Amir]

Remove Regex include

[Amir]

Use X86MemOperand->Size in X86Disassembler::getMemOperandSize.

[Amir]

llvm/lib/Target/X86/X86InstrInfo.td

397 ↗

(On Diff #437040)

Done. I'm not 100% sure it's an equivalent replacement – i.e. if all Operand's that have ParserMatchClass->Name = Mem* are actually X86MemOperand's. But the check-llvm test passed locally.

[Amir]

Rebase

[skan]

LGTM

llvm/utils/TableGen/InstrInfoEmitter.cpp

 #include "CodeGenTarget.h"
 #include "PredicateExpander.h"
 #include "SequenceToOffsetTable.h"
 #include "TableGenBackends.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include <cassert>
 #include <cstdint>
 #include <iterator>
 #include <map>
if (!NumberedInstructions.empty()) {
     OS << "  return OpcodeOperandTypes[Offsets[Opcode] + OpIdx];\n";
   } else {
     OS << "  llvm_unreachable(\"No instructions defined\");\n";
   }
   OS << "}\n";
   OS << "} // end namespace " << Namespace << "\n";
   OS << "} // end namespace llvm\n";
   OS << "#endif // GET_INSTRINFO_OPERAND_TYPE\n\n";
+
+  OS << "#ifdef GET_INSTRINFO_MEM_OPERAND_SIZE\n";
+  OS << "#undef GET_INSTRINFO_MEM_OPERAND_SIZE\n";
+  OS << "namespace llvm {\n";
+  OS << "namespace " << Namespace << " {\n";
+  OS << "LLVM_READONLY\n";
+  OS << "static int getMemOperandSize(int OpType) {\n";
+  OS << "  switch (OpType) {\n";
+  Regex MemSizeRegex("Mem([0-9]+)");
+  for (const Record *Op : Operands) {
+    assert(!Op->isValueUnset("ParserMatchClass"));
+    const Record *OperandClassDef = Op->getValueAsDef("ParserMatchClass");
+    // ParserMatchClass Name field, e.g. Mem64
+    StringRef Name = OperandClassDef->getValueAsString("Name");
+    SmallVector<StringRef> Matches;
+    if (!MemSizeRegex.match(Name, &Matches))
+      continue;
+    std::string Size = MemSizeRegex.sub("\\1", Matches[0]);
+    OS << "  case OpTypes::" << Op->getName() << ": return " << Size << ";\n";
+  }
+  OS << "  default: return 0;\n";
+  OS << "  }\n}\n";
+  OS << "} // end namespace " << Namespace << "\n";
+  OS << "} // end namespace llvm\n";
+  OS << "#endif // GET_INSTRINFO_MEM_OPERAND_SIZE\n\n";
 }
 
 void InstrInfoEmitter::emitLogicalOperandSizeMappings(
     raw_ostream &OS, StringRef Namespace,
     ArrayRef<const CodeGenInstruction *> NumberedInstructions) {
   std::map<std::vector<unsigned>, unsigned> LogicalOpSizeMap;
 
   std::map<unsigned, std::vector<std::string>> InstMap;