[llvm-exegesis] Serializes instruction's operand in BenchmarkResult's key.

[gchatelet]

Summary

Follow up patch to https://reviews.llvm.org/D47764.

[gchatelet]

Updating D47785: [llvm-exegesis] Serializes instruction's operand in BenchmarkResult's key.

[gchatelet]

Updating D47785: [llvm-exegesis] Serializes instruction's operand in BenchmarkResult's key.

[courbet]

tools/llvm-exegesis/lib/BenchmarkResult.cpp

44 ↗

(On Diff #149990)

this won't work for arm/power.

47 ↗

(On Diff #149990)

Really ? :D
Then use a custom format in serialize(), else this will not work. You also need to make sure that you're using the ansi C locale.

[gchatelet]

Updating D47785: [llvm-exegesis] Serializes instruction's operand in BenchmarkResult's key.

tools/llvm-exegesis/lib/BenchmarkResult.cpp

44 ↗

(On Diff #149990)

As discussed, the registers are guaranteed to have a name (MCRegisterInfo::getName), these names are derived from the enums defined in `llvm/lib/Target/XXX/XXXGenRegisterInfo.inc.
Because they are valid identifier they have to start with a letter, LLVM convention defines enums in uppercase.

47 ↗

(On Diff #149990)

As discussed, I used an encoding scheme for numbers.

[courbet]

tools/llvm-exegesis/lib/BenchmarkResult.cpp

18 ↗

(On Diff #150116)

This should be
static constexpr const char kIntegerFormat[] = "i_0x%" PRId64 "x";

[gchatelet]

Updating D47785: [llvm-exegesis] Serializes instruction's operand in BenchmarkResult's key.

llvm/trunk/tools/llvm-exegesis/lib/BenchmarkResult.cpp

 #include "BenchmarkResult.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/raw_ostream.h"
 
+static constexpr const char kIntegerFormat[] = "i_0x%" PRId64 "x";
+static constexpr const char kDoubleFormat[] = "f_%la";
+
+static void serialize(const exegesis::BenchmarkResultContext &Context,
+                      const llvm::MCOperand &MCOperand, llvm::raw_ostream &OS) {
+  if (MCOperand.isReg()) {
+    OS << Context.getRegName(MCOperand.getReg());
+  } else if (MCOperand.isImm()) {
+    OS << llvm::format(kIntegerFormat, MCOperand.getImm());
+  } else if (MCOperand.isFPImm()) {
+    OS << llvm::format(kDoubleFormat, MCOperand.getFPImm());
+  } else {
+    OS << "INVALID";
+  }
+}
+
+static void serialize(const exegesis::BenchmarkResultContext &Context,
+                      const llvm::MCInst &MCInst, llvm::raw_ostream &OS) {
+  OS << Context.getInstrName(MCInst.getOpcode());
+  for (const auto &Op : MCInst) {
+    OS << ' ';
+    serialize(Context, Op, OS);
+  }
+}
+
+static llvm::MCOperand
+deserialize(const exegesis::BenchmarkResultContext &Context,
+            llvm::StringRef String) {
+  assert(!String.empty());
+  int64_t IntValue = 0;
+  double DoubleValue = 0;
+  if (sscanf(String.data(), kIntegerFormat, &IntValue) == 1)
+    return llvm::MCOperand::createImm(IntValue);
+  if (sscanf(String.data(), kDoubleFormat, &DoubleValue) == 1)
+    return llvm::MCOperand::createFPImm(DoubleValue);
+  if (unsigned RegNo = Context.getRegNo(String)) // Returns 0 if invalid.
+    return llvm::MCOperand::createReg(RegNo);
+  return {};
+}
+
+static llvm::StringRef
+deserialize(const exegesis::BenchmarkResultContext &Context,
+            llvm::StringRef String, llvm::MCInst &Value) {
+  llvm::SmallVector<llvm::StringRef, 8> Pieces;
+  String.split(Pieces, " ");
+  if (Pieces.empty())
+    return "Invalid Instruction";
+  bool ProcessOpcode = true;
+  for (llvm::StringRef Piece : Pieces) {
+    if (ProcessOpcode) {
+      ProcessOpcode = false;
+      Value.setOpcode(Context.getInstrOpcode(Piece));
+      if (Value.getOpcode() == 0)
+        return "Unknown Opcode Name";
+    } else {
+      Value.addOperand(deserialize(Context, Piece));
+    }
+  }
+  return {};
+}
+
+// YAML IO requires a mutable pointer to Context but we guarantee to not
+// modify it.
+static void *getUntypedContext(const exegesis::BenchmarkResultContext &Ctx) {
+  return const_cast<exegesis::BenchmarkResultContext *>(&Ctx);
+}
+
+static const exegesis::BenchmarkResultContext &getTypedContext(void *Ctx) {
+  assert(Ctx);
+  return *static_cast<const exegesis::BenchmarkResultContext *>(Ctx);
+}
+
 // Defining YAML traits for IO.
 namespace llvm {
 namespace yaml {
 
 // std::vector<llvm::MCInst> will be rendered as a list.
 template <> struct SequenceElementTraits<llvm::MCInst> {
   static const bool flow = false;
 };
 
 template <> struct ScalarTraits<llvm::MCInst> {
 
   static void output(const llvm::MCInst &Value, void *Ctx,
                      llvm::raw_ostream &Out) {
-    assert(Ctx);
-    auto *Context = static_cast<const exegesis::BenchmarkResultContext *>(Ctx);
-    const StringRef Name = Context->getInstrName(Value.getOpcode());
-    assert(!Name.empty());
-    Out << Name;
+    serialize(getTypedContext(Ctx), Value, Out);
   }
 
   static StringRef input(StringRef Scalar, void *Ctx, llvm::MCInst &Value) {
-    assert(Ctx);
-    auto *Context = static_cast<const exegesis::BenchmarkResultContext *>(Ctx);
-    const unsigned Opcode = Context->getInstrOpcode(Scalar);
-    if (Opcode == 0) {
-      return "Unable to parse instruction";
-    }
-    Value.setOpcode(Opcode);
-    return StringRef();
+    return deserialize(getTypedContext(Ctx), Scalar, Value);
   }
 
   static QuotingType mustQuote(StringRef) { return QuotingType::Single; }
 
   static const bool flow = true;
 };
 
 // std::vector<exegesis::Measure> will be rendered as a list.
unsigned BenchmarkResultContext::getInstrOpcode(llvm::StringRef Name) const {
   return 0;
 }
 
 template <typename ObjectOrList>
 static ObjectOrList readYamlOrDieCommon(const BenchmarkResultContext &Context,
                                         llvm::StringRef Filename) {
   std::unique_ptr<llvm::MemoryBuffer> MemBuffer = llvm::cantFail(
       llvm::errorOrToExpected(llvm::MemoryBuffer::getFile(Filename)));
-  // YAML IO requires a mutable pointer to Context but we guarantee to not
-  // modify it.
-  llvm::yaml::Input Yin(*MemBuffer,
-                        const_cast<BenchmarkResultContext *>(&Context));
+  llvm::yaml::Input Yin(*MemBuffer, getUntypedContext(Context));
   ObjectOrList Benchmark;
   Yin >> Benchmark;
   return Benchmark;
 }
 
 InstructionBenchmark
 InstructionBenchmark::readYamlOrDie(const BenchmarkResultContext &Context,
                                     llvm::StringRef Filename) {
   return readYamlOrDieCommon<InstructionBenchmark>(Context, Filename);
 }
 
 std::vector<InstructionBenchmark>
 InstructionBenchmark::readYamlsOrDie(const BenchmarkResultContext &Context,
                                      llvm::StringRef Filename) {
   return readYamlOrDieCommon<std::vector<InstructionBenchmark>>(Context,
                                                                 Filename);
 }
 
 void InstructionBenchmark::writeYamlTo(const BenchmarkResultContext &Context,
-                                       llvm::raw_ostream &S) {
-  // YAML IO requires a mutable pointer to Context but we guarantee to not
-  // modify it.
-  llvm::yaml::Output Yout(S, const_cast<BenchmarkResultContext *>(&Context));
+                                       llvm::raw_ostream &OS) {
+  llvm::yaml::Output Yout(OS, getUntypedContext(Context));
   Yout << *this;
 }
 
 void InstructionBenchmark::readYamlFrom(const BenchmarkResultContext &Context,
                                         llvm::StringRef InputContent) {
-  // YAML IO requires a mutable pointer to Context but we guarantee to not
-  // modify it.
-  llvm::yaml::Input Yin(InputContent,
-                        const_cast<BenchmarkResultContext *>(&Context));
+  llvm::yaml::Input Yin(InputContent, getUntypedContext(Context));
   Yin >> *this;
 }
 
 // FIXME: Change the API to let the caller handle errors.
 void InstructionBenchmark::writeYamlOrDie(const BenchmarkResultContext &Context,
                                           const llvm::StringRef Filename) {
   if (Filename == "-") {
     writeYamlTo(Context, llvm::outs());

llvm/trunk/tools/llvm-exegesis/lib/BenchmarkRunner.cpp

if (llvm::Error E = SnippetOrError.takeError()) {
     InstrBenchmark.Error = llvm::toString(std::move(E));
     return InstrBenchmark;
   }
   std::vector<llvm::MCInst> &Snippet = SnippetOrError.get();
   if (Snippet.empty()) {
     InstrBenchmark.Error = "Empty snippet";
     return InstrBenchmark;
   }
-
+  for (const auto &MCInst : Snippet) {
+    InstrBenchmark.Key.Instructions.push_back(MCInst);
+  }
   InfoStream << "Snippet:\n";
   for (const auto &MCInst : Snippet) {
     DumpMCInst(MCRegisterInfo, MCInstrInfo, MCInst, InfoStream);
     InfoStream << "\n";
   }
 
   std::vector<llvm::MCInst> Code;
   for (int I = 0; I < InstrBenchmark.NumRepetitions; ++I)

llvm/trunk/unittests/tools/llvm-exegesis/BenchmarkResultTest.cpp

 using ::testing::Property;
 
 namespace exegesis {
 
 bool operator==(const BenchmarkMeasure &A, const BenchmarkMeasure &B) {
   return std::tie(A.Key, A.Value) == std::tie(B.Key, B.Value);
 }
 
+static std::string Dump(const llvm::MCInst &McInst) {
+  std::string Buffer;
+  llvm::raw_string_ostream OS(Buffer);
+  McInst.print(OS);
+  return Buffer;
+}
+
 MATCHER(EqMCInst, "") {
-  return get<0>(arg).getOpcode() == get<1>(arg).getOpcode();
+  const std::string Lhs = Dump(get<0>(arg));
+  const std::string Rhs = Dump(get<1>(arg));
+  if (Lhs != Rhs) {
+    *result_listener << Lhs << " <=> " << Rhs;
+    return false;
+  }
+  return true;
 }
 
 namespace {
 
 static constexpr const unsigned kInstrId = 5;
 static constexpr const char kInstrName[] = "Instruction5";
+static constexpr const unsigned kReg1Id = 1;
+static constexpr const char kReg1Name[] = "Reg1";
+static constexpr const unsigned kReg2Id = 2;
+static constexpr const char kReg2Name[] = "Reg2";
 
 TEST(BenchmarkResultTest, WriteToAndReadFromDisk) {
   BenchmarkResultContext Ctx;
   Ctx.addInstrEntry(kInstrId, kInstrName);
+  Ctx.addRegEntry(kReg1Id, kReg1Name);
+  Ctx.addRegEntry(kReg2Id, kReg2Name);
 
   InstructionBenchmark ToDisk;
 
   ToDisk.Key.OpcodeName = "name";
-  ToDisk.Key.Instructions.push_back(llvm::MCInstBuilder(kInstrId));
+  ToDisk.Key.Instructions.push_back(llvm::MCInstBuilder(kInstrId)
+                                        .addReg(kReg1Id)
+                                        .addReg(kReg2Id)
+                                        .addImm(123)
+                                        .addFPImm(0.5));
   ToDisk.Key.Config = "config";
   ToDisk.Mode = InstructionBenchmark::Latency;
   ToDisk.CpuName = "cpu_name";
   ToDisk.LLVMTriple = "llvm_triple";
   ToDisk.NumRepetitions = 1;
   ToDisk.Measurements.push_back(BenchmarkMeasure{"a", 1, "debug a"});
   ToDisk.Measurements.push_back(BenchmarkMeasure{"b", 2, ""});
   ToDisk.Error = "error";