[PATCH] D47764: [llvm-exegesis] Add instructions to BenchmarkResult Key.

[Clement Courbet via Phabricator via llvm-commits]

courbet added inline comments.


================
Comment at: tools/llvm-exegesis/lib/BenchmarkResult.h:31
 
+// This context is used when de/serializing InstructionBenchmark to guarantee
+// that Registers and Instructions are human readable and preserved accross
----------------
nit: move the class definition to the end of the header as this is more an artifact and therefore less important than the schema..


================
Comment at: tools/llvm-exegesis/lib/BenchmarkResult.h:54
+private:
+  std::unordered_map<unsigned, llvm::StringRef> InstrOpcodeToName;
+  std::unordered_map<unsigned, llvm::StringRef> RegNoToName;
----------------
Can you mention that we're not directly getting the name from the sched model to avoid creating a dependency to make testing easier ?


================
Comment at: tools/llvm-exegesis/llvm-exegesis.cpp:143
   Runner->run(GetOpcodeOrDie(State.getInstrInfo()), Filter, NumRepetitions)
-      .writeYamlOrDie(BenchmarkFile);
+      .writeYamlOrDie(Ctx, BenchmarkFile);
   exegesis::pfm::pfmTerminate();
----------------
`writeYamlOrDie(getBenchmarkResultContext(State), BenchmarkFile);` ?


================
Comment at: tools/llvm-exegesis/llvm-exegesis.cpp:175
   const std::vector<InstructionBenchmark> Points =
-      InstructionBenchmark::readYamlsOrDie(BenchmarkFile);
+      InstructionBenchmark::readYamlsOrDie(Ctx, BenchmarkFile);
   llvm::outs() << "Parsed " << Points.size() << " benchmark points\n";
----------------
ditto


Repository:
  rL LLVM

https://reviews.llvm.org/D47764