[PATCH] D54144: [llvm-exegesis] Correclty handle all X86 memory encoding formats.

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet added inline comments.


================
Comment at: tools/llvm-exegesis/lib/X86/Target.cpp:25
 
-// A chunk of instruction's operands that represents a single memory access.
-struct MemoryOperandRange {
-  MemoryOperandRange(llvm::ArrayRef<Operand> Operands) : Ops(Operands) {}
-
-  // Setup InstructionTemplate so the memory access represented by this object
-  // points to [reg] + offset.
-  void fillOrDie(InstructionTemplate &IT, unsigned Reg, unsigned Offset) {
-    switch (Ops.size()) {
-    case 5:
-      IT.getValueFor(Ops[0]) = llvm::MCOperand::createReg(Reg);    // BaseReg
-      IT.getValueFor(Ops[1]) = llvm::MCOperand::createImm(1);      // ScaleAmt
-      IT.getValueFor(Ops[2]) = llvm::MCOperand::createReg(0);      // IndexReg
-      IT.getValueFor(Ops[3]) = llvm::MCOperand::createImm(Offset); // Disp
-      IT.getValueFor(Ops[4]) = llvm::MCOperand::createReg(0);      // Segment
-      break;
-    default:
-      llvm::errs() << Ops.size() << "-op are not handled right now ("
-                   << IT.Instr.Name << ")\n";
-      llvm_unreachable("Invalid memory configuration");
-    }
-  }
-
-  // Returns whether Range can be filled.
-  static bool isValid(const MemoryOperandRange &Range) {
-    return Range.Ops.size() == 5;
-  }
-
-  // Returns whether Op is a valid memory operand.
-  static bool isMemoryOperand(const Operand &Op) {
-    return Op.isMemory() && Op.isExplicit();
-  }
-
-  llvm::ArrayRef<Operand> Ops;
-};
-
-// X86 memory access involve non constant number of operands, this function
-// extracts contiguous memory operands into MemoryOperandRange so it's easier to
-// check and fill.
-static std::vector<MemoryOperandRange>
-getMemoryOperandRanges(llvm::ArrayRef<Operand> Operands) {
-  std::vector<MemoryOperandRange> Result;
-  while (!Operands.empty()) {
-    Operands = Operands.drop_until(MemoryOperandRange::isMemoryOperand);
-    auto MemoryOps = Operands.take_while(MemoryOperandRange::isMemoryOperand);
-    if (!MemoryOps.empty())
-      Result.push_back(MemoryOps);
-    Operands = Operands.drop_front(MemoryOps.size());
+// Returns true if fillMemoryOrDie() handles this instruction.
+Error isInvalidMemoryInstr(const Instruction &Instr) {
----------------
I don't understand this comment


================
Comment at: tools/llvm-exegesis/lib/X86/Target.cpp:113
+  case X86II::AddRegFrm:
+    return (Instr.Description->mayLoad() || Instr.Description->mayStore())
+               ? make_error<BenchmarkFailure>(
----------------
I'd rather handle individual instructions rather than mayLoad/mayStore that may catch other instruction in the future.


Repository:
  rL LLVM

https://reviews.llvm.org/D54144