[PATCH] D44519: Add llvm-exegesis tool.

[Eugene Zelenko via Phabricator via llvm-commits]

Eugene.Zelenko added a comment.

Please run Clang-format and Clang-tidy modernize.

tools/llvm-exegesis/llvm-exegesis.rst should be moved to docs/Command. Please mention new tool in Release Notes.



================
Comment at: tools/llvm-exegesis/BenchmarkRunner.cpp:23
+
+BenchmarkRunner::~BenchmarkRunner() {}
+
----------------
Please use = default;


================
Comment at: tools/llvm-exegesis/InMemoryAssembler.cpp:36
+namespace {
+static constexpr const char kModuleID[] = "ExegesisInfoTest";
+static constexpr const char kFunctionID[] = "foo";
----------------
Please move static definitions out of anonymous namespace.


================
Comment at: tools/llvm-exegesis/InMemoryAssembler.cpp:40
+// Small utility function to add named passes.
+bool addPass(llvm::PassManagerBase &PM, llvm::StringRef PassName,
+             llvm::TargetPassConfig &TPC) {
----------------
static should be used instead of anonymous namespace for functions in LLVM. Same in other places.


================
Comment at: tools/llvm-exegesis/InMemoryAssembler.cpp:158
+  explicit TrackingSectionMemoryManager(uintptr_t *CodeSize)
+      : CodeSize(CodeSize) {}
+
----------------
Please use = default;


================
Comment at: tools/llvm-exegesis/InMemoryAssembler.cpp:197
+    : FunctionContext(std::move(Context)) {
+  //
+  fillMachineFunction(*FunctionContext.MF, Instructions);
----------------
Unnecessary comment.


================
Comment at: tools/llvm-exegesis/InstructionSnippetGenerator.cpp:49
+// Update the state of a Variable with an explicit operand.
+void updateExplicitOperandVariable(const llvm::MCRegisterInfo &RegInfo,
+                                   const llvm::MCInstrDesc &InstrInfo,
----------------
static should be used instead of anonymous namespace for functions in LLVM. Same in other places.


================
Comment at: tools/llvm-exegesis/Latency.cpp:26
+// TODO(courbet): Handle memory.
+bool isInvalidOperand(const llvm::MCOperandInfo &OpInfo) {
+  switch (OpInfo.OperandType) {
----------------
static should be used instead of anonymous namespace for functions in LLVM. Same in other places.


================
Comment at: tools/llvm-exegesis/Latency.cpp:42
+
+LatencyBenchmarkRunner::~LatencyBenchmarkRunner() {}
+
----------------
Please use = default;


================
Comment at: tools/llvm-exegesis/X86.cpp:15
+
+llvm::Error makeError(llvm::Twine Msg) {
+  return llvm::make_error<llvm::StringError>(Msg,
----------------
static should be used instead of anonymous namespace for functions in LLVM. Same in other places.


================
Comment at: tools/llvm-exegesis/llvm-exegesis.rst:16
+
+
+Given an LLVM opcode name and a benchmarking mode, :program:`llvm-exegesis`
----------------
Unnecessary empty line.


Repository:
  rL LLVM

https://reviews.llvm.org/D44519