Addressing review comments

Author: AnastasiyaChernikova

llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp

@@ -14,10 +14,8 @@
 #include "RISCVInstrInfo.h"
 
 // include computeAvailableFeatures and computeRequiredFeatures.
-#define GET_COMPUTE_FEATURES
 #define GET_AVAILABLE_OPCODE_CHECKER
 #include "RISCVGenInstrInfo.inc"
-#undef GET_COMPUTE_FEATURES
 
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 
@@ -28,61 +26,6 @@ namespace exegesis {
 
 namespace {
 
-class ExegesisRISCVTarget : public ExegesisTarget {
-public:
-  ExegesisRISCVTarget();
-
-  MCRegister findRegisterByName(const StringRef RegName) const override;
-
-  bool matchesArch(Triple::ArchType Arch) const override;
-
-  std::vector<MCInst> setRegTo(const MCSubtargetInfo &STI, unsigned Reg,
-                               const APInt &Value) const override;
-
-  unsigned getDefaultLoopCounterRegister(const Triple &) const override;
-
-  void decrementLoopCounterAndJump(MachineBasicBlock &MBB,
-                                   MachineBasicBlock &TargetMBB,
-                                   const MCInstrInfo &MII,
-                                   unsigned LoopRegister) const override;
-
-  unsigned getScratchMemoryRegister(const Triple &TT) const override;
-
-  void fillMemoryOperands(InstructionTemplate &IT, unsigned Reg,
-                          unsigned Offset) const override;
-
-  ArrayRef<unsigned> getUnavailableRegisters() const override;
-
-  Error randomizeTargetMCOperand(const Instruction &Instr, const Variable &Var,
-                                 MCOperand &AssignedValue,
-                                 const BitVector &ForbiddenRegs) const override;
-
-  std::vector<InstructionTemplate>
-  generateInstructionVariants(const Instruction &Instr,
-                              unsigned MaxConfigsPerOpcode) const override;
-};
-
-ExegesisRISCVTarget::ExegesisRISCVTarget()
-    : ExegesisTarget(ArrayRef<CpuAndPfmCounters>{},
-                     RISCV_MC::isOpcodeAvailable) {}
-
-#define GET_REGISTER_MATCHER
-#include "RISCVGenAsmMatcher.inc"
-
-MCRegister
-ExegesisRISCVTarget::findRegisterByName(const StringRef RegName) const {
-  MCRegister Reg;
-  if ((Reg = MatchRegisterName(RegName)))
-    return Reg;
-  if ((Reg = MatchRegisterAltName(RegName)))
-    return Reg;
-  return RISCV::NoRegister;
-}
-
-bool ExegesisRISCVTarget::matchesArch(Triple::ArchType Arch) const {
-  return Arch == Triple::riscv32 || Arch == Triple::riscv64;
-}
-
 // Stores constant value to a general-purpose (integer) register.
 static std::vector<MCInst> loadIntReg(const MCSubtargetInfo &STI, unsigned Reg,
                                       const APInt &Value) {
@@ -156,6 +99,49 @@ static bool isVectorRegList(unsigned Reg) {
          RISCV::VRN8M1RegClass.contains(Reg);
 }
 
+class ExegesisRISCVTarget : public ExegesisTarget {
+public:
+  ExegesisRISCVTarget();
+
+  bool matchesArch(Triple::ArchType Arch) const override;
+
+  std::vector<MCInst> setRegTo(const MCSubtargetInfo &STI, unsigned Reg,
+                               const APInt &Value) const override;
+
+  unsigned getDefaultLoopCounterRegister(const Triple &) const override;
+
+  void decrementLoopCounterAndJump(MachineBasicBlock &MBB,
+                                   MachineBasicBlock &TargetMBB,
+                                   const MCInstrInfo &MII,
+                                   unsigned LoopRegister) const override;
+
+  unsigned getScratchMemoryRegister(const Triple &TT) const override;
+
+  void fillMemoryOperands(InstructionTemplate &IT, unsigned Reg,
+                          unsigned Offset) const override;
+
+  ArrayRef<unsigned> getUnavailableRegisters() const override;
+
+  Error randomizeTargetMCOperand(const Instruction &Instr, const Variable &Var,
+                                 MCOperand &AssignedValue,
+                                 const BitVector &ForbiddenRegs) const override;
+
+  std::vector<InstructionTemplate>
+  generateInstructionVariants(const Instruction &Instr,
+                              unsigned MaxConfigsPerOpcode) const override;
+};
+
+ExegesisRISCVTarget::ExegesisRISCVTarget()
+    : ExegesisTarget(ArrayRef<CpuAndPfmCounters>{},
+                     RISCV_MC::isOpcodeAvailable) {}
+
+#define GET_REGISTER_MATCHER
+#include "RISCVGenAsmMatcher.inc"
+
+bool ExegesisRISCVTarget::matchesArch(Triple::ArchType Arch) const {
+  return Arch == Triple::riscv32 || Arch == Triple::riscv64;
+}
+
 std::vector<MCInst> ExegesisRISCVTarget::setRegTo(const MCSubtargetInfo &STI,
                                                   unsigned Reg,
                                                   const APInt &Value) const {

llvm/tools/llvm-exegesis/llvm-exegesis.cpp

@@ -50,27 +50,15 @@
 namespace llvm {
 namespace exegesis {
 
-struct OpcodeNameParser
-    : public cl::parser<std::vector<std::pair<StringRef, StringRef>>> {
-  OpcodeNameParser(cl::Option &O)
-      : cl::parser<std::vector<std::pair<StringRef, StringRef>>>(O) {}
-  bool parse(cl::Option &O, StringRef ArgName, const StringRef ArgValue,
-             std::vector<std::pair<StringRef, StringRef>> &Val);
-};
-
 static cl::opt<int> OpcodeIndex(
     "opcode-index",
     cl::desc("opcode to measure, by index, or -1 to measure all opcodes"),
     cl::cat(BenchmarkOptions), cl::init(0));
 
-static cl::opt<std::vector<std::pair<StringRef, StringRef>>, false,
-               OpcodeNameParser>
+static cl::opt<std::string>
     OpcodeNames("opcode-name",
-                cl::desc("comma-separated list of opcodes to measure, "
-                         "each item is either opcode name ('OP') "
-                         "or opcode range ('OP1..OP2', ends are inclusive)"),
-                cl::cat(BenchmarkOptions),
-                cl::init(std::vector<std::pair<StringRef, StringRef>>()));
+                cl::desc("comma-separated list of opcodes to measure, by name"),
+                cl::cat(BenchmarkOptions), cl::init(""));
 
 static cl::opt<std::string> SnippetsFile("snippets-file",
                                          cl::desc("code snippets to measure"),
@@ -328,27 +316,6 @@ static bool isIgnoredOpcode(const LLVMState &State, unsigned Opcode) {
   return getIgnoredOpcodeReasonOrNull(State, Opcode) != nullptr;
 }
 
-bool OpcodeNameParser::parse(
-    cl::Option &O, StringRef ArgName, const StringRef OpcodeNames,
-    std::vector<std::pair<StringRef, StringRef>> &Val) {
-  SmallVector<StringRef, 2> Pieces;
-  StringRef(OpcodeNames)
-      .split(Pieces, ",", /*MaxSplit=*/-1, /*KeepEmpty=*/false);
-  for (const StringRef &OpcodeName : Pieces) {
-    size_t DotDotPos = OpcodeName.find("..");
-    if (DotDotPos == StringRef::npos) {
-      Val.emplace_back(OpcodeName, OpcodeName);
-      continue;
-    }
-    StringRef BeginOpcodeName = OpcodeName.substr(0, DotDotPos);
-    StringRef EndOpcodeName = OpcodeName.substr(DotDotPos + 2);
-    Val.emplace_back(BeginOpcodeName, EndOpcodeName);
-  }
-  if (Val.empty())
-    return O.error("No matching opcode names");
-  return false;
-}
-
 // Checks that only one of OpcodeNames, OpcodeIndex or SnippetsFile is provided,
 // and returns the opcode indices or {} if snippets should be read from
 // `SnippetsFile`.
@@ -388,34 +355,16 @@ static std::vector<unsigned> getOpcodesOrDie(const LLVMState &State) {
     return 0u;
   };
 
+  SmallVector<StringRef, 2> Pieces;
+  StringRef(OpcodeNames.getValue())
+      .split(Pieces, ",", /* MaxSplit */ -1, /* KeepEmpty */ false);
   std::vector<unsigned> Result;
-  for (const std::pair<StringRef, StringRef> &OpcodeName : OpcodeNames) {
-    if (OpcodeName.first == OpcodeName.second) {
-      if (unsigned Opcode = ResolveName(OpcodeName.first)) {
-        Result.push_back(Opcode);
-        continue;
-      } else {
-        ExitWithError(Twine("unknown opcode ").concat(OpcodeName.first));
-      }
-    } else {
-      StringRef BeginOpcodeName = OpcodeName.first;
-      unsigned BeginOpcode =
-          BeginOpcodeName.empty() ? 1 : ResolveName(BeginOpcodeName);
-      if (BeginOpcode == 0) {
-        ExitWithError(Twine("unknown opcode ").concat(BeginOpcodeName));
-      }
-      StringRef EndOpcodeName = OpcodeName.second;
-      unsigned EndOpcode = EndOpcodeName.empty()
-                               ? State.getInstrInfo().getNumOpcodes() - 1
-                               : ResolveName(EndOpcodeName);
-      if (EndOpcode == 0) {
-        ExitWithError(Twine("unknown opcode ").concat(EndOpcodeName));
-      }
-      for (unsigned I = BeginOpcode; I <= EndOpcode; ++I) {
-        if (!isIgnoredOpcode(State, I))
-          Result.push_back(I);
-      }
-    }
+  Result.reserve(Pieces.size());
+  for (const StringRef &OpcodeName : Pieces) {
+    if (unsigned Opcode = ResolveName(OpcodeName))
+      Result.push_back(Opcode);
+    else
+      ExitWithError(Twine("unknown opcode ").concat(OpcodeName));
   }
   return Result;
 }